From 5724ef3bc488d0aab13c6abab6c82db625d6f77a Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 10:35:12 -0400 Subject: [PATCH 01/13] feat(agenthook): install Pi hooks as a generated extension --- agenthook/AGENTS.md | 3 + agenthook/agenthook_test.go | 246 ++++++++++++++++++++++++++++++++++-- agenthook/config.go | 21 ++- agenthook/doc.go | 4 +- agenthook/handler_test.go | 106 ++++++++++++++++ agenthook/json.go | 111 +++++++++------- agenthook/normalize.go | 5 + agenthook/pi.go | 110 ++++++++++++++++ agenthook/pi_extension.js | 24 ++++ agenthook/profile.go | 13 ++ agenthook/script.go | 85 +++++++++++++ agenthook/script_runtime.js | 43 +++++++ 12 files changed, 709 insertions(+), 62 deletions(-) create mode 100644 agenthook/pi.go create mode 100644 agenthook/pi_extension.js create mode 100644 agenthook/script.go create mode 100644 agenthook/script_runtime.js diff --git a/agenthook/AGENTS.md b/agenthook/AGENTS.md index 9477b481..82d0edf0 100644 --- a/agenthook/AGENTS.md +++ b/agenthook/AGENTS.md @@ -30,6 +30,9 @@ 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. - 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 898b3502..ba261991 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -3,7 +3,9 @@ package agenthook import ( "encoding/json" "fmt" + "io" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -24,7 +26,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 +35,7 @@ func TestProfilesExposeClaudeStyleEvents(t *testing.T) { AgentDroid, AgentGemini, AgentHermes, + AgentPi, AgentQwen, }, []Agent{ profiles[0].Agent, @@ -43,10 +46,12 @@ 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}, profiles[7].SupportedEvents) + assert.Contains(profiles[8].SupportedEvents, EventPermissionRequest) } func TestPlanInstallDefaultsToEveryProfileEvent(t *testing.T) { @@ -139,21 +144,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") + }) + } } } @@ -184,6 +191,7 @@ func TestConfigPathHonorsAgentHomes(t *testing.T) { {agent: AgentCopilot, env: "COPILOT_HOME", path: filepath.Join("hooks", "agenthook.json")}, {agent: AgentGemini, env: "GEMINI_CLI_HOME", path: filepath.Join(".gemini", "settings.json")}, {agent: AgentHermes, env: "HERMES_HOME", path: "config.yaml"}, + {agent: AgentPi, env: "PI_CODING_AGENT_DIR", path: filepath.Join("extensions", "agenthook.js")}, {agent: AgentQwen, env: "QWEN_HOME", path: "settings.json"}, } for _, tt := range tests { @@ -907,3 +915,213 @@ func TestWriteConfigRefusesLinkSwappedInForRegularConfig(t *testing.T) { require.NoError(err) assert.Equal(t, "other", string(data)) } + +func TestConfigPathExpandsPiAgentDirTilde(t *testing.T) { + home, err := os.UserHomeDir() + require.NoError(t, err) + t.Setenv("PI_CODING_AGENT_DIR", "~/pi-agent") + + path, err := ConfigPath(AgentPi) + + require.NoError(t, err) + assert.Equal(t, filepath.Join(home, "pi-agent", "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) { + _, err := Install(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: executable, + Arguments: []string{"agent-hook", "--source", source}, + Marker: "--source " + source, + }) + require.NoError(err) + } + + install("/opt/a", "a-hook") + install("/opt/b", "b-hook") + assert.Equal( + []string{"/opt/a agent-hook --source a-hook", "/opt/b agent-hook --source b-hook"}, + piCommands(piScriptHooks(t, path), "session_start"), + ) + + install("/moved/a", "a-hook") + hooks := piScriptHooks(t, path) + assert.Equal( + []string{"/opt/b agent-hook --source b-hook", "/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{"/opt/b agent-hook --source b-hook"}, + 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: EventSessionStart, Matcher: "startup"}}, + }) + 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) + 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") { + // Outlive the 1s hook timeout so the extension has to kill this process. + <-time.After(time.Minute) + } +} + +const piExtensionDriver = ` +import { pathToFileURL } from "node:url"; +const extension = await import(pathToFileURL(process.argv[2]).href); +const handlers = {}; +extension.default({ on: (name, handler) => { handlers[name] = handler; } }); +const ctx = (mode) => ({ + mode, + cwd: "/work", + sessionManager: { getSessionId: () => "pi-session-1", getSessionFile: () => "/sessions/1.jsonl" }, +}); +await handlers.session_start({ type: "session_start", reason: "startup" }, ctx("tui")); +await handlers.session_start({ type: "session_start", reason: "startup" }, ctx("json")); +await handlers.agent_settled({ type: "agent_settled" }, ctx("tui")); +` + +func TestPiExtensionRunsRegisteredCommand(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") + _, 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: 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) + cmd.Env = append(os.Environ(), "KIT_AGENTHOOK_PI_HELPER_OUT="+out) + + started := time.Now() + output, err := cmd.CombinedOutput() + + require.NoError(err, string(output)) + assert.Less(time.Since(started), 30*time.Second, "the timed-out command was not killed") + payloads, err := os.ReadFile(out) + require.NoError(err) + lines := strings.Split(strings.TrimSpace(string(payloads)), "\n") + require.Len(lines, 2) + assert.JSONEq(`{ + "hook_event_name":"session_start", + "session_id":"pi-session-1", + "transcript_path":"/sessions/1.jsonl", + "cwd":"/work", + "reason":"startup" +}`, lines[0]) + assert.Contains(lines[1], `"agent_settled"`) +} diff --git a/agenthook/config.go b/agenthook/config.go index d818f078..2c696097 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", @@ -205,6 +213,9 @@ func prepareInstall(agent Agent, opts InstallOptions) (profileSpec, string, []na return profileSpec{}, "", nil, errors.New("Hermes hook timeout must not exceed 300 seconds") } matcher := nativeMatcher(spec, strings.TrimSpace(hook.Matcher)) + if spec.format == formatScript && matcher != "" { + return profileSpec{}, "", nil, fmt.Errorf("%s hooks do not support matchers", spec.profile.DisplayName) + } if spec.format == formatHermesYAML && matcher != "" && hook.Event != EventPreToolUse && hook.Event != EventPostToolUse { return profileSpec{}, "", nil, errors.New("Hermes only supports matchers on PreToolUse and PostToolUse hooks") @@ -252,6 +263,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 cc9bc093..3ca8a3c5 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -6,7 +6,9 @@ // 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. // // 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 f86813a7..acc8748c 100644 --- a/agenthook/handler_test.go +++ b/agenthook/handler_test.go @@ -848,3 +848,109 @@ func TestHandleRejectsOversizedPayload(t *testing.T) { require.Error(t, err) assert.ErrorContains(t, err, "hook payload exceeds") } + +type piHandler struct { + NoopHandler + sessionStart *SessionStartInput + prompt *UserPromptSubmitInput + stop *StopInput + stopOutput StopOutput +} + +func (h *piHandler) SessionStart(_ context.Context, input SessionStartInput) (SessionStartOutput, error) { + h.sessionStart = &input + return SessionStartOutput{}, nil +} + +func (h *piHandler) UserPromptSubmit( + _ context.Context, + input UserPromptSubmitInput, +) (UserPromptSubmitOutput, error) { + h.prompt = &input + return UserPromptSubmitOutput{}, nil +} + +func (h *piHandler) Stop(_ context.Context, input StopInput) (StopOutput, error) { + h.stop = &input + return h.stopOutput, nil +} + +func TestHandleDispatchesPiEvents(t *testing.T) { + const common = `"session_id":"pi-1","cwd":"/work","transcript_path":"/s/1.jsonl"` + tests := []struct { + name string + payload string + check func(*testing.T, *piHandler) + }{ + { + name: "startup", payload: `"hook_event_name":"session_start","reason":"startup"`, + check: func(t *testing.T, h *piHandler) { + t.Helper() + require.NotNil(t, h.sessionStart) + assert.Equal(t, SessionSourceStartup, h.sessionStart.Source) + assert.Equal(t, "pi-1", h.sessionStart.SessionID) + }, + }, + { + name: "new", payload: `"hook_event_name":"session_start","reason":"new"`, + check: func(t *testing.T, h *piHandler) { + t.Helper() + require.NotNil(t, h.sessionStart) + assert.Equal(t, SessionSourceClear, h.sessionStart.Source) + }, + }, + { + name: "reload", payload: `"hook_event_name":"session_start","reason":"reload"`, + check: func(t *testing.T, h *piHandler) { + t.Helper() + require.NotNil(t, h.sessionStart) + assert.Empty(t, h.sessionStart.Source) + }, + }, + { + name: "prompt", payload: `"hook_event_name":"before_agent_start","prompt":"fix it"`, + check: func(t *testing.T, h *piHandler) { + t.Helper() + require.NotNil(t, h.prompt) + assert.Equal(t, "fix it", h.prompt.Prompt) + }, + }, + { + name: "settled", payload: `"hook_event_name":"agent_settled"`, + check: func(t *testing.T, h *piHandler) { + t.Helper() + require.NotNil(t, h.stop) + assert.Equal(t, EventStop, h.stop.HookEventName) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var output bytes.Buffer + handler := &piHandler{} + + err := Handle( + t.Context(), AgentPi, strings.NewReader("{"+common+","+tt.payload+"}"), + &output, handler, + ) + + require.NoError(t, err) + tt.check(t, handler) + assert.JSONEq(t, `{}`, output.String()) + }) + } +} + +func TestHandleRejectsPiControlOutput(t *testing.T) { + var output bytes.Buffer + handler := &piHandler{stopOutput: 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 eec1a007..a152bbb0 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 9b764e33..49cf77ca 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 := promotePiSource(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 00000000..b4051c38 --- /dev/null +++ b/agenthook/pi.go @@ -0,0 +1,110 @@ +package agenthook + +import ( + _ "embed" + "encoding/json" + "fmt" + "os" + "path/filepath" + "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/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/package-manager.ts#L603-L640 + ConfigFilename: filepath.Join("extensions", "agenthook.js"), + // SessionEnd is left out: Pi emits session_shutdown on SIGTERM and + // SIGHUP, so an application stopping Pi would erase the ID it resumes: + // https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/modes/interactive/interactive-mode.ts#L4258-L4270 + SupportedEvents: []Event{EventSessionStart, EventUserPromptSubmit, EventStop}, + }, + formatScript, + "", + func() (string, error) { return userDotDir(filepath.Join(".pi", "agent")) }, + ) + spec.configEnvDir = expandPiAgentDir + 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: + // https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/extensions/types.ts#L733-L741 + spec.sessionSourceRequirement = inputOptional + return spec +} + +// piEventName maps Claude events to Pi extension events: +// https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/extensions/types.ts#L911-L922 +// https://github.com/badlogic/pi-mono/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" + default: + return string(event) + } +} + +// expandPiAgentDir follows Pi's tilde expansion of PI_CODING_AGENT_DIR: +// https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/config.ts#L588-L611 +func expandPiAgentDir(dir string) (string, error) { + rest, ok := strings.CutPrefix(dir, "~") + if !ok || (rest != "" && rest[0] != '/' && rest[0] != filepath.Separator) { + return dir, nil + } + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(home, rest), nil +} + +// promotePiSource maps session_start's reason to Claude's source. Pi's /new +// starts a fresh session as Claude's /clear does; reload has no equivalent. +func promotePiSource(payload map[string]json.RawMessage) error { + if _, exists := payload["source"]; exists { + return nil + } + 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) + } + } + raw, ok := payload["reason"] + if event != "session_start" || !ok { + return nil + } + if err := json.Unmarshal(raw, &reason); err != nil { + return fmt.Errorf("field %q must be a string: %w", "reason", err) + } + source := map[string]SessionSource{ + "startup": SessionSourceStartup, + "resume": SessionSourceResume, + "fork": SessionSourceFork, + "new": SessionSourceClear, + }[reason] + if source == "" { + return nil + } + encoded, err := json.Marshal(source) + if err != nil { + return err + } + payload["source"] = encoded + return nil +} diff --git a/agenthook/pi_extension.js b/agenthook/pi_extension.js new file mode 100644 index 00000000..2d990d79 --- /dev/null +++ b/agenthook/pi_extension.js @@ -0,0 +1,24 @@ +// Pi extension API: https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/docs/extensions.md +export default function (pi) { + for (const name of Object.keys(config.hooks ?? {})) { + pi.on(name, async (event, ctx) => { + // 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/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/extensions/types.ts#L323-L335 + if (ctx.mode !== "tui") return; + 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; + if (typeof event.reason === "string") payload.reason = event.reason; + if (name === "before_agent_start") { + if (typeof event.prompt !== "string" || event.prompt === "") return; + payload.prompt = event.prompt; + } + await runHooks(name, payload); + }); + } +} diff --git a/agenthook/profile.go b/agenthook/profile.go index bc651037..07a478b0 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 00000000..83f0dbce --- /dev/null +++ b/agenthook/script.go @@ -0,0 +1,85 @@ +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. +func scriptBlock(data []byte, path string) ([]byte, error) { + text := string(data) + _, rest, foundBegin := strings.Cut(text, scriptBlockBegin) + block, _, foundEnd := strings.Cut(rest, scriptBlockEnd) + if !foundBegin || !foundEnd { + return nil, fmt.Errorf("agent hook config %s was not written by agenthook", path) + } + block = strings.TrimSpace(block) + 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 00000000..426f4bd4 --- /dev/null +++ b/agenthook/script_runtime.js @@ -0,0 +1,43 @@ +import { spawn } from "node:child_process"; + +const defaultTimeoutSeconds = 60; + +// Runs one argv without a shell, writing the payload to stdin. A child that +// outlives its timeout is killed; failures never reach the harness. +function runCommand(handler, payload) { + return new Promise((resolve) => { + let child; + try { + child = spawn(handler.command, Array.isArray(handler.args) ? handler.args : [], { + stdio: ["pipe", "ignore", "ignore"], + }); + } catch { + resolve(); + return; + } + const seconds = handler.timeout > 0 ? handler.timeout : defaultTimeoutSeconds; + const timer = setTimeout(() => child.kill(), seconds * 1000); + const done = () => { + clearTimeout(timer); + resolve(); + }; + child.once("error", done); + child.once("close", done); + if (child.stdin) { + child.stdin.on("error", () => {}); + child.stdin.end(JSON.stringify(payload)); + } + }); +} + +// Runs every registered command for a native event in order. +async function runHooks(event, payload) { + const entries = (config.hooks ?? {})[event]; + for (const entry of Array.isArray(entries) ? entries : []) { + for (const handler of Array.isArray(entry?.hooks) ? entry.hooks : []) { + if (handler?.type === "command" && typeof handler.command === "string") { + await runCommand(handler, payload); + } + } + } +} From c86b8468e15231270ccbdd3351e5f405ad038ce1 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 10:51:50 -0400 Subject: [PATCH 02/13] fix(agenthook): resume unsaved Pi sessions and retire replaced ones --- agentcli/README.md | 2 +- agentcli/agentcli_test.go | 4 +- agentcli/pi.go | 5 +- agenthook/AGENTS.md | 2 + agenthook/agenthook_test.go | 80 +++++++++++++++++++--- agenthook/config.go | 5 +- agenthook/doc.go | 3 +- agenthook/handler_test.go | 114 +++++++++++++++++-------------- agenthook/normalize.go | 2 +- agenthook/pi.go | 133 +++++++++++++++++++++++++----------- agenthook/pi_extension.js | 9 ++- agenthook/script.go | 22 ++++-- agenthook/script_runtime.js | 10 ++- 13 files changed, 274 insertions(+), 117 deletions(-) diff --git a/agentcli/README.md b/agentcli/README.md index b6126ecd..5d10b72f 100644 --- a/agentcli/README.md +++ b/agentcli/README.md @@ -77,7 +77,7 @@ autonomy and the built-in skills control; its other controls are `exec` flags. | Kiro | noninteractive | argument | `chat --resume-id ID` | text | low, medium, high, xhigh, maximum | | Kilo | noninteractive | stdin | `run --session ID` | text, JSONL | low, medium, high, xhigh, maximum | | Factory Droid | interactive, noninteractive | stdin, noninteractive only | `--resume ID`, `exec --session-id ID` | text, JSON, JSONL | low, medium, high, xhigh, maximum, noninteractive only | -| Pi | interactive, noninteractive | argument and `@file` | `--session ID` | text, JSONL | low, medium, high, xhigh, maximum | +| Pi | interactive, noninteractive | argument and `@file` | `--session-id ID` (Pi 0.76.0 or later) | text, JSONL | low, medium, high, xhigh, maximum | `ReasoningXHigh` and `ReasoningMaximum` are distinct. Adapters with a native `max` value, including Codex, map only `ReasoningMaximum` to it. Droid accepts diff --git a/agentcli/agentcli_test.go b/agentcli/agentcli_test.go index 0a7743a2..5bdf5318 100644 --- a/agentcli/agentcli_test.go +++ b/agentcli/agentcli_test.go @@ -85,7 +85,7 @@ func TestInteractiveResumePreservesConfiguredOptions(t *testing.T) { }{ {agentcli.Codex, agentcli.Command{Executable: "codex-custom", Options: []string{"--full-auto", "--profile", "team"}}, agentcli.Request{}, []string{"codex-custom", "--full-auto", "--profile", "team", "resume", "session-1"}}, {agentcli.Claude, agentcli.Command{Executable: "claude-custom", Options: []string{"--setting-sources", "project"}}, agentcli.Request{}, []string{"claude-custom", "--setting-sources", "project", "--resume", "session-1"}}, - {agentcli.Pi, agentcli.Command{Executable: "pi-custom", Options: []string{"--offline"}}, agentcli.Request{}, []string{"pi-custom", "--offline", "--session", "session-1"}}, + {agentcli.Pi, agentcli.Command{Executable: "pi-custom", Options: []string{"--offline"}}, agentcli.Request{}, []string{"pi-custom", "--offline", "--session-id", "session-1"}}, {agentcli.Copilot, agentcli.Command{Executable: "copilot-custom", Options: []string{"--add-dir", "shared"}}, agentcli.Request{}, []string{"copilot-custom", "--add-dir", "shared", "--resume=session-1"}}, {agentcli.Cursor, agentcli.Command{Executable: "cursor-agent", Options: []string{"--workspace", "repo"}}, agentcli.Request{}, []string{"cursor-agent", "--workspace", "repo", "--resume", "session-1"}}, {agentcli.Droid, agentcli.Command{Executable: "droid-custom", Options: []string{"--append-system-prompt", "Run tests."}}, agentcli.Request{Autonomy: agentcli.AutonomyLow, DisableSkills: true}, []string{"droid-custom", "--append-system-prompt", "Run tests.", "--resume", "session-1", "--auto", "low", "--disable-builtin-skills"}}, @@ -157,7 +157,7 @@ func TestConfiguredOptionsKeepTheirArityAndOrder(t *testing.T) { }{ {agentcli.Codex, agentcli.Command{Executable: "codex-custom", Options: []string{"--profile=team", "-c", "feature.test=true", "--add-dir", "-shared"}}, "", []string{"codex-custom", "--profile=team", "-c", "feature.test=true", "--add-dir", "-shared", "resume", "session-1"}}, {agentcli.Claude, agentcli.Command{Options: []string{"--setting-sources=project", "--plugin-dir", "one", "--plugin-dir", "-two"}}, "", []string{"claude", "--setting-sources=project", "--plugin-dir", "one", "--plugin-dir", "-two", "--resume", "session-1"}}, - {agentcli.Pi, agentcli.Command{Options: []string{"-ne", "--tui-mode", "fullscreen", "--offline"}}, "", []string{"pi", "-ne", "--tui-mode", "fullscreen", "--offline", "--session", "session-1"}}, + {agentcli.Pi, agentcli.Command{Options: []string{"-ne", "--tui-mode", "fullscreen", "--offline"}}, "", []string{"pi", "-ne", "--tui-mode", "fullscreen", "--offline", "--session-id", "session-1"}}, {agentcli.Kiro, agentcli.Command{Executable: "kiro-custom", Options: []string{"--wrap", "never"}}, agentcli.NonInteractive, []string{"kiro-custom", "chat", "--wrap", "never", "--no-interactive", "--resume-id", "session-1"}}, {agentcli.Droid, agentcli.Command{Executable: "droid-custom", Options: []string{"--append-system-prompt", "review only"}}, agentcli.NonInteractive, []string{"droid-custom", "exec", "--append-system-prompt", "review only", "--session-id", "session-1"}}, } diff --git a/agentcli/pi.go b/agentcli/pi.go index f857535c..c0ed19a3 100644 --- a/agentcli/pi.go +++ b/agentcli/pi.go @@ -82,7 +82,10 @@ func buildPi(a *adapter, sessionID string, request Request) (Invocation, error) args = append(args, "--mode", "json") } if sessionID != "" { - args = append(args, "--session", sessionID) + // --session-id opens the exact project-local session or creates it, so a + // session Pi has not saved yet still resumes; --session would fail: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/cli.md + args = append(args, "--session-id", sessionID) } if request.Provider != "" { args = append(args, "--provider", request.Provider) diff --git a/agenthook/AGENTS.md b/agenthook/AGENTS.md index 82d0edf0..ef050478 100644 --- a/agenthook/AGENTS.md +++ b/agenthook/AGENTS.md @@ -33,6 +33,8 @@ - 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. - 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 ba261991..57967297 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -6,9 +6,11 @@ import ( "io" "os" "os/exec" + "os/signal" "path/filepath" "runtime" "strings" + "syscall" "testing" "time" @@ -50,7 +52,10 @@ func TestProfilesExposeClaudeStyleEvents(t *testing.T) { }) assert.Contains(profiles[6].SupportedEvents, EventPreToolUse) assert.NotContains(profiles[6].SupportedEvents, EventNotification) - assert.Equal([]Event{EventSessionStart, EventUserPromptSubmit, EventStop}, profiles[7].SupportedEvents) + assert.Equal( + []Event{EventSessionStart, EventUserPromptSubmit, EventStop, EventSessionEnd}, + profiles[7].SupportedEvents, + ) assert.Contains(profiles[8].SupportedEvents, EventPermissionRequest) } @@ -916,15 +921,39 @@ func TestWriteConfigRefusesLinkSwappedInForRegularConfig(t *testing.T) { assert.Equal(t, "other", string(data)) } -func TestConfigPathExpandsPiAgentDirTilde(t *testing.T) { +func TestConfigPathNormalizesPiAgentDirAsPiDoes(t *testing.T) { home, err := os.UserHomeDir() require.NoError(t, err) - t.Setenv("PI_CODING_AGENT_DIR", "~/pi-agent") + 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"}, + {env: pick("file:///C:/pi/agent", "file:///pi/agent"), want: pick(`C:\pi\agent`, "/pi/agent")}, + } + for _, tt := range tests { + t.Run(tt.env, func(t *testing.T) { + t.Setenv("PI_CODING_AGENT_DIR", tt.env) - path, err := ConfigPath(AgentPi) + path, err := ConfigPath(AgentPi) - require.NoError(t, err) - assert.Equal(t, filepath.Join(home, "pi-agent", "extensions", "agenthook.js"), path) + 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. @@ -1039,11 +1068,36 @@ func TestPlanInstallPiRequiresExecutableWithoutMatchers(t *testing.T) { Executable: "/opt/hook", Arguments: []string{"--source", "shared-agent-hook-test"}, Marker: testMarker, - Hooks: []Hook{{Event: EventSessionStart, Matcher: "startup"}}, + Hooks: []Hook{{Event: EventStop, Matcher: ToolBash}}, }) require.ErrorContains(t, err, "do not support matchers") } +func TestInstallPiKeepsArgumentThatLooksLikeBlockMarker(t *testing.T) { + require := require.New(t) + path := filepath.Join(t.TempDir(), "agenthook.js") + opts := InstallOptions{ + ConfigPath: path, + Executable: "/opt/hook", + Arguments: []string{scriptBlockEnd, "--source", "shared-agent-hook-test"}, + Marker: testMarker, + } + + _, err := Install(AgentPi, opts) + require.NoError(err) + result, err := Install(AgentPi, opts) + require.NoError(err) + assert.False(t, result.Changed) + assert.Equal(t, + []string{"/opt/hook " + scriptBlockEnd + " " + testMarker}, + piCommands(piScriptHooks(t, path), "session_start"), + ) + + _, err = Uninstall(AgentPi, path, testMarker) + require.NoError(err) + assert.Empty(t, piScriptHooks(t, path)) +} + func TestPiExtensionHelper(t *testing.T) { out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") if out == "" { @@ -1057,7 +1111,9 @@ func TestPiExtensionHelper(t *testing.T) { require.NoError(t, err) require.NoError(t, file.Close()) if strings.Contains(string(payload), "agent_settled") { - // Outlive the 1s hook timeout so the extension has to kill this process. + // 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(time.Minute) } } @@ -1074,6 +1130,8 @@ const ctx = (mode) => ({ }); await handlers.session_start({ type: "session_start", reason: "startup" }, ctx("tui")); await handlers.session_start({ type: "session_start", reason: "startup" }, ctx("json")); +await handlers.session_shutdown({ type: "session_shutdown", reason: "quit" }, ctx("tui")); +await handlers.session_shutdown({ type: "session_shutdown", reason: "new" }, ctx("tui")); await handlers.agent_settled({ type: "agent_settled" }, ctx("tui")); ` @@ -1093,6 +1151,7 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { Marker: testMarker, Hooks: []Hook{ {Event: EventSessionStart}, + {Event: EventSessionEnd}, {Event: EventStop, Timeout: time.Second}, }, }) @@ -1115,7 +1174,7 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { payloads, err := os.ReadFile(out) require.NoError(err) lines := strings.Split(strings.TrimSpace(string(payloads)), "\n") - require.Len(lines, 2) + require.Len(lines, 3) assert.JSONEq(`{ "hook_event_name":"session_start", "session_id":"pi-session-1", @@ -1123,5 +1182,6 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { "cwd":"/work", "reason":"startup" }`, lines[0]) - assert.Contains(lines[1], `"agent_settled"`) + assert.Contains(lines[1], `"reason":"new"`) + assert.Contains(lines[2], `"agent_settled"`) } diff --git a/agenthook/config.go b/agenthook/config.go index 2c696097..0205034c 100644 --- a/agenthook/config.go +++ b/agenthook/config.go @@ -212,10 +212,11 @@ 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") } - matcher := nativeMatcher(spec, strings.TrimSpace(hook.Matcher)) - if spec.format == formatScript && matcher != "" { + // Check the caller's matcher: Bash translates to Pi's empty shell tool. + 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 { return profileSpec{}, "", nil, errors.New("Hermes only supports matchers on PreToolUse and PostToolUse hooks") diff --git a/agenthook/doc.go b/agenthook/doc.go index 3ca8a3c5..43473da4 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -8,7 +8,8 @@ // Profiles are provided for Claude Code, Codex, GitHub Copilot CLI, Cursor, // 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. +// runs the registered commands. That module reports only from Pi's interactive +// terminal, so print, json, and rpc runs, subagents included, stay silent. // // 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 acc8748c..f2da86c6 100644 --- a/agenthook/handler_test.go +++ b/agenthook/handler_test.go @@ -849,42 +849,16 @@ func TestHandleRejectsOversizedPayload(t *testing.T) { assert.ErrorContains(t, err, "hook payload exceeds") } -type piHandler struct { - NoopHandler - sessionStart *SessionStartInput - prompt *UserPromptSubmitInput - stop *StopInput - stopOutput StopOutput -} - -func (h *piHandler) SessionStart(_ context.Context, input SessionStartInput) (SessionStartOutput, error) { - h.sessionStart = &input - return SessionStartOutput{}, nil -} - -func (h *piHandler) UserPromptSubmit( - _ context.Context, - input UserPromptSubmitInput, -) (UserPromptSubmitOutput, error) { - h.prompt = &input - return UserPromptSubmitOutput{}, nil -} - -func (h *piHandler) Stop(_ context.Context, input StopInput) (StopOutput, error) { - h.stop = &input - return h.stopOutput, nil -} - func TestHandleDispatchesPiEvents(t *testing.T) { const common = `"session_id":"pi-1","cwd":"/work","transcript_path":"/s/1.jsonl"` tests := []struct { name string payload string - check func(*testing.T, *piHandler) + check func(*testing.T, *lifecycleHandler) }{ { name: "startup", payload: `"hook_event_name":"session_start","reason":"startup"`, - check: func(t *testing.T, h *piHandler) { + check: func(t *testing.T, h *lifecycleHandler) { t.Helper() require.NotNil(t, h.sessionStart) assert.Equal(t, SessionSourceStartup, h.sessionStart.Source) @@ -892,8 +866,8 @@ func TestHandleDispatchesPiEvents(t *testing.T) { }, }, { - name: "new", payload: `"hook_event_name":"session_start","reason":"new"`, - check: func(t *testing.T, h *piHandler) { + name: "new session", payload: `"hook_event_name":"session_start","reason":"new"`, + check: func(t *testing.T, h *lifecycleHandler) { t.Helper() require.NotNil(t, h.sessionStart) assert.Equal(t, SessionSourceClear, h.sessionStart.Source) @@ -901,33 +875,48 @@ func TestHandleDispatchesPiEvents(t *testing.T) { }, { name: "reload", payload: `"hook_event_name":"session_start","reason":"reload"`, - check: func(t *testing.T, h *piHandler) { + check: func(t *testing.T, h *lifecycleHandler) { t.Helper() require.NotNil(t, h.sessionStart) assert.Empty(t, h.sessionStart.Source) }, }, { - name: "prompt", payload: `"hook_event_name":"before_agent_start","prompt":"fix it"`, - check: func(t *testing.T, h *piHandler) { + name: "replaced by new", payload: `"hook_event_name":"session_shutdown","reason":"new"`, + check: func(t *testing.T, h *lifecycleHandler) { + t.Helper() + require.NotNil(t, h.sessionEnd) + assert.Equal(t, SessionEndClear, h.sessionEnd.Reason) + }, + }, + { + name: "replaced by resume", payload: `"hook_event_name":"session_shutdown","reason":"resume"`, + check: func(t *testing.T, h *lifecycleHandler) { t.Helper() - require.NotNil(t, h.prompt) - assert.Equal(t, "fix it", h.prompt.Prompt) + require.NotNil(t, h.sessionEnd) + assert.Equal(t, SessionEndResume, h.sessionEnd.Reason) }, }, { - name: "settled", payload: `"hook_event_name":"agent_settled"`, - check: func(t *testing.T, h *piHandler) { + name: "replaced by fork", payload: `"hook_event_name":"session_shutdown","reason":"fork"`, + check: func(t *testing.T, h *lifecycleHandler) { t.Helper() - require.NotNil(t, h.stop) - assert.Equal(t, EventStop, h.stop.HookEventName) + require.NotNil(t, h.sessionEnd) + assert.Equal(t, SessionEndOther, h.sessionEnd.Reason) + }, + }, + { + name: "prompt", payload: `"hook_event_name":"before_agent_start","prompt":"fix it"`, + check: func(t *testing.T, h *lifecycleHandler) { + t.Helper() + assert.Nil(t, h.sessionStart) }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var output bytes.Buffer - handler := &piHandler{} + handler := &lifecycleHandler{} err := Handle( t.Context(), AgentPi, strings.NewReader("{"+common+","+tt.payload+"}"), @@ -941,16 +930,41 @@ func TestHandleDispatchesPiEvents(t *testing.T) { } } -func TestHandleRejectsPiControlOutput(t *testing.T) { - var output bytes.Buffer - handler := &piHandler{stopOutput: StopOutput{Decision: DecisionBlock, Reason: "work remains"}} +func TestHandleRejectsUnmappedPiEvents(t *testing.T) { + tests := []struct { + name string + payload string + handler Handler + want string + }{ + { + name: "quit keeps the session", + payload: `"hook_event_name":"session_shutdown","reason":"quit"`, + handler: &lifecycleHandler{}, want: "SessionEnd input missing reason", + }, + { + name: "empty prompt", + payload: `"hook_event_name":"before_agent_start","prompt":""`, + handler: &lifecycleHandler{}, want: "UserPromptSubmit input missing prompt", + }, + { + name: "stop control output", + payload: `"hook_event_name":"agent_settled"`, + handler: stopHandler{output: StopOutput{Decision: DecisionBlock, Reason: "work remains"}}, + want: "does not support Stop control output", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var output bytes.Buffer - err := Handle( - t.Context(), AgentPi, - strings.NewReader(`{"session_id":"pi-1","hook_event_name":"agent_settled"}`), - &output, handler, - ) + err := Handle( + t.Context(), AgentPi, strings.NewReader(`{"session_id":"pi-1",`+tt.payload+"}"), + &output, tt.handler, + ) - require.ErrorContains(t, err, "does not support Stop control output") - assert.Empty(t, output.String()) + require.ErrorContains(t, err, tt.want) + assert.Empty(t, output.String()) + }) + } } diff --git a/agenthook/normalize.go b/agenthook/normalize.go index 49cf77ca..57edefa1 100644 --- a/agenthook/normalize.go +++ b/agenthook/normalize.go @@ -48,7 +48,7 @@ func normalize(agent Agent, input io.Reader) ([]byte, error) { } } if agent == AgentPi { - if err := promotePiSource(payload); err != nil { + if err := promotePiReason(payload); err != nil { return nil, fmt.Errorf("normalize Pi hook payload: %w", err) } } diff --git a/agenthook/pi.go b/agenthook/pi.go index b4051c38..1c39160c 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -4,8 +4,11 @@ import ( _ "embed" "encoding/json" "fmt" + "net/url" "os" "path/filepath" + "regexp" + "runtime" "strings" ) @@ -19,32 +22,34 @@ func piProfile() profileSpec { 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/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/package-manager.ts#L603-L640 + // 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 is left out: Pi emits session_shutdown on SIGTERM and - // SIGHUP, so an application stopping Pi would erase the ID it resumes: - // https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/modes/interactive/interactive-mode.ts#L4258-L4270 - SupportedEvents: []Event{EventSessionStart, EventUserPromptSubmit, EventStop}, + SupportedEvents: []Event{ + EventSessionStart, EventUserPromptSubmit, EventStop, EventSessionEnd, + }, }, formatScript, "", func() (string, error) { return userDotDir(filepath.Join(".pi", "agent")) }, ) - spec.configEnvDir = expandPiAgentDir + 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: - // https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/extensions/types.ts#L733-L741 + // 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/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/extensions/types.ts#L911-L922 -// https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/extensions/types.ts#L997-L1000 +// 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: @@ -54,57 +59,109 @@ func piEventName(event Event) string { 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) } } -// expandPiAgentDir follows Pi's tilde expansion of PI_CODING_AGENT_DIR: -// https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/src/config.ts#L588-L611 -func expandPiAgentDir(dir string) (string, error) { - rest, ok := strings.CutPrefix(dir, "~") - if !ok || (rest != "" && rest[0] != '/' && rest[0] != filepath.Separator) { - return dir, nil +var piWindowsShellPath = regexp.MustCompile(`(?i)^/(?:mnt/|cygdrive/)?([a-z])(?:/(.*))?$`) + +// piAgentDir mirrors Pi's normalizePath for PI_CODING_AGENT_DIR: Git Bash, +// MSYS, Cygwin, and WSL drive paths on Windows, then ~, then file: URLs: +// 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 } - home, err := os.UserHomeDir() + if strings.HasPrefix(dir, "file://") { + return fileURLPath(dir) + } + return dir, nil +} + +// fileURLPath follows Node's fileURLToPath for the cases Pi accepts. +func fileURLPath(raw string) (string, error) { + parsed, err := url.Parse(raw) if err != nil { return "", err } - return filepath.Join(home, rest), nil + remote := parsed.Host != "" && parsed.Host != "localhost" + if runtime.GOOS != "windows" { + if remote { + return "", fmt.Errorf("file URL host must be localhost or empty: %s", raw) + } + return parsed.Path, nil + } + if remote { + return `\\` + parsed.Host + filepath.FromSlash(parsed.Path), nil + } + return filepath.FromSlash(strings.TrimPrefix(parsed.Path, "/")), nil } -// promotePiSource maps session_start's reason to Claude's source. Pi's /new -// starts a fresh session as Claude's /clear does; reload has no equivalent. -func promotePiSource(payload map[string]json.RawMessage) error { - if _, exists := payload["source"]; exists { - return 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. A shutdown for quit or reload loses its +// reason so Handle refuses it rather than retiring a resumable session. +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) } } - raw, ok := payload["reason"] - if event != "session_start" || !ok { - return nil + 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) + } } - 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] + if value == "" { + delete(payload, "reason") + } + default: + return nil } - source := map[string]SessionSource{ - "startup": SessionSourceStartup, - "resume": SessionSourceResume, - "fork": SessionSourceFork, - "new": SessionSourceClear, - }[reason] - if source == "" { + if value == "" { return nil } - encoded, err := json.Marshal(source) + encoded, err := json.Marshal(value) if err != nil { return err } - payload["source"] = encoded + payload[field] = encoded return nil } diff --git a/agenthook/pi_extension.js b/agenthook/pi_extension.js index 2d990d79..77220ab7 100644 --- a/agenthook/pi_extension.js +++ b/agenthook/pi_extension.js @@ -1,11 +1,16 @@ -// Pi extension API: https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/docs/extensions.md +// Pi extension API: https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/extensions.md +const replacedSessionReasons = ["new", "resume", "fork"]; + export default function (pi) { for (const name of Object.keys(config.hooks ?? {})) { pi.on(name, async (event, ctx) => { // 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/badlogic/pi-mono/blob/main/packages/coding-agent/src/core/extensions/types.ts#L323-L335 + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L323-L335 if (ctx.mode !== "tui") return; + // Report a shutdown only when another session replaces this one; quit + // and reload keep the session resumable. + if (name === "session_shutdown" && !replacedSessionReasons.includes(event.reason)) return; const payload = { hook_event_name: name, session_id: ctx.sessionManager.getSessionId(), diff --git a/agenthook/script.go b/agenthook/script.go index 83f0dbce..36ba9e54 100644 --- a/agenthook/script.go +++ b/agenthook/script.go @@ -68,16 +68,26 @@ func planScriptConfig( // 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) { - text := string(data) - _, rest, foundBegin := strings.Cut(text, scriptBlockBegin) - block, _, foundEnd := strings.Cut(rest, scriptBlockEnd) - if !foundBegin || !foundEnd { + 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(block) + block := strings.TrimSpace(strings.Join(lines[begin+1:end], "\n")) block, foundDecl := strings.CutPrefix(block, scriptConfigDecl) - block, foundEnd = strings.CutSuffix(block, ";") + block, foundEnd := strings.CutSuffix(block, ";") if !foundDecl || !foundEnd { return nil, fmt.Errorf("agent hook config %s has a malformed registration block", path) } diff --git a/agenthook/script_runtime.js b/agenthook/script_runtime.js index 426f4bd4..c1e88f4a 100644 --- a/agenthook/script_runtime.js +++ b/agenthook/script_runtime.js @@ -2,8 +2,9 @@ import { spawn } from "node:child_process"; const defaultTimeoutSeconds = 60; -// Runs one argv without a shell, writing the payload to stdin. A child that -// outlives its timeout is killed; failures never reach the harness. +// Runs one argv without a shell, writing the payload to stdin. 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; @@ -16,7 +17,10 @@ function runCommand(handler, payload) { return; } const seconds = handler.timeout > 0 ? handler.timeout : defaultTimeoutSeconds; - const timer = setTimeout(() => child.kill(), seconds * 1000); + const timer = setTimeout(() => { + child.kill("SIGKILL"); + resolve(); + }, seconds * 1000); const done = () => { clearTimeout(timer); resolve(); From 02c156e721327805ae961ee6f4624efec0f60ba1 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 11:08:41 -0400 Subject: [PATCH 03/13] fix(agenthook): surface Pi hook failures and refuse directories Pi will not load --- agentcli/README.md | 2 +- agentcli/agentcli_test.go | 18 +++++++++++ agentcli/pi.go | 18 ++++++++--- agenthook/AGENTS.md | 3 +- agenthook/agenthook_test.go | 61 ++++++++++++++++++++++++++++++++++--- agenthook/doc.go | 3 +- agenthook/handler_test.go | 5 --- agenthook/pi.go | 58 +++++++++++++++++++++++++++++++---- agenthook/pi_extension.js | 4 +++ agenthook/profile.go | 1 + agenthook/script.go | 5 +++ agenthook/script_runtime.js | 37 ++++++++++++++-------- 12 files changed, 180 insertions(+), 35 deletions(-) diff --git a/agentcli/README.md b/agentcli/README.md index 5d10b72f..5fdfc2c7 100644 --- a/agentcli/README.md +++ b/agentcli/README.md @@ -77,7 +77,7 @@ autonomy and the built-in skills control; its other controls are `exec` flags. | Kiro | noninteractive | argument | `chat --resume-id ID` | text | low, medium, high, xhigh, maximum | | Kilo | noninteractive | stdin | `run --session ID` | text, JSONL | low, medium, high, xhigh, maximum | | Factory Droid | interactive, noninteractive | stdin, noninteractive only | `--resume ID`, `exec --session-id ID` | text, JSON, JSONL | low, medium, high, xhigh, maximum, noninteractive only | -| Pi | interactive, noninteractive | argument and `@file` | `--session-id ID` (Pi 0.76.0 or later) | text, JSONL | low, medium, high, xhigh, maximum | +| Pi | interactive, noninteractive | argument and `@file` | `--session-id ID` (Pi 0.76.0 or later), `--session PATH` for a path or `.jsonl` file | text, JSONL | low, medium, high, xhigh, maximum | `ReasoningXHigh` and `ReasoningMaximum` are distinct. Adapters with a native `max` value, including Codex, map only `ReasoningMaximum` to it. Droid accepts diff --git a/agentcli/agentcli_test.go b/agentcli/agentcli_test.go index 5bdf5318..19286ee2 100644 --- a/agentcli/agentcli_test.go +++ b/agentcli/agentcli_test.go @@ -75,6 +75,24 @@ func TestInvocationContracts(t *testing.T) { } } +func TestPiResumeSendsPathsToSessionAndIDsToSessionID(t *testing.T) { + t.Parallel() + tests := []struct { + session string + flag string + }{ + {session: "019a-session", flag: "--session-id"}, + {session: "/home/me/.pi/agent/sessions/1.jsonl", flag: "--session"}, + {session: `C:\Users\me\.pi\agent\sessions\1.jsonl`, flag: "--session"}, + {session: "1.jsonl", flag: "--session"}, + } + for _, test := range tests { + got, err := mustAgent(t, agentcli.Pi, agentcli.Command{}).Resume(test.session, agentcli.Request{}) + require.NoError(t, err) + assert.Equal(t, []string{"pi", test.flag, test.session}, got.Argv) + } +} + func TestInteractiveResumePreservesConfiguredOptions(t *testing.T) { t.Parallel() tests := []struct { diff --git a/agentcli/pi.go b/agentcli/pi.go index c0ed19a3..5156856f 100644 --- a/agentcli/pi.go +++ b/agentcli/pi.go @@ -82,10 +82,7 @@ func buildPi(a *adapter, sessionID string, request Request) (Invocation, error) args = append(args, "--mode", "json") } if sessionID != "" { - // --session-id opens the exact project-local session or creates it, so a - // session Pi has not saved yet still resumes; --session would fail: - // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/cli.md - args = append(args, "--session-id", sessionID) + args = append(args, piSessionFlag(sessionID), sessionID) } if request.Provider != "" { args = append(args, "--provider", request.Provider) @@ -131,3 +128,16 @@ func validatePiRequest(mode Mode, request Request) error { } return nil } + +// piSessionFlag follows Pi's own test for a session path. A path goes to +// --session, which rejects nothing; an ID goes to --session-id, which opens the +// exact project-local session or creates it, so a session Pi has not saved yet +// still resumes: +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/main.ts#L252-L256 +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/cli.md +func piSessionFlag(session string) string { + if strings.ContainsAny(session, "/\\") || strings.HasSuffix(session, ".jsonl") { + return "--session" + } + return "--session-id" +} diff --git a/agenthook/AGENTS.md b/agenthook/AGENTS.md index ef050478..da05767e 100644 --- a/agenthook/AGENTS.md +++ b/agenthook/AGENTS.md @@ -34,7 +34,8 @@ 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. + `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 57967297..ab6201a4 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -943,6 +943,7 @@ func TestConfigPathNormalizesPiAgentDirAsPiDoes(t *testing.T) { {env: "/cygdrive/e", want: pick(`E:\`, "/cygdrive/e")}, {env: "//server/share", want: "//server/share"}, {env: pick("file:///C:/pi/agent", "file:///pi/agent"), want: pick(`C:\pi\agent`, "/pi/agent")}, + {env: pick("file://LOCALHOST/C:/pi/agent", "file://LOCALHOST/pi/agent"), want: pick(`C:\pi\agent`, "/pi/agent")}, } for _, tt := range tests { t.Run(tt.env, func(t *testing.T) { @@ -1098,6 +1099,33 @@ func TestInstallPiKeepsArgumentThatLooksLikeBlockMarker(t *testing.T) { assert.Empty(t, piScriptHooks(t, path)) } +func TestInstallPiRefusesDirectoryPiLoadsOnlyEntriesFrom(t *testing.T) { + tests := map[string]map[string]string{ + "index.js": {"index.js": "export default () => {};\n"}, + "index.ts": {"index.ts": "export default () => {};\n"}, + "package.json": {"package.json": `{"pi":{"extensions":["main.js"]}}`, "main.js": ""}, + } + for name, files := range tests { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + for file, content := range files { + require.NoError(t, os.WriteFile(filepath.Join(dir, file), []byte(content), 0o600)) + } + path := filepath.Join(dir, "agenthook.js") + + _, err := Install(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: "/opt/hook", + Arguments: []string{"--source", "shared-agent-hook-test"}, + Marker: testMarker, + }) + + require.ErrorContains(t, err, "would never load agenthook.js") + assert.NoFileExists(t, path) + }) + } +} + func TestPiExtensionHelper(t *testing.T) { out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") if out == "" { @@ -1128,11 +1156,18 @@ const ctx = (mode) => ({ cwd: "/work", sessionManager: { getSessionId: () => "pi-session-1", getSessionFile: () => "/sessions/1.jsonl" }, }); -await handlers.session_start({ type: "session_start", reason: "startup" }, ctx("tui")); -await handlers.session_start({ type: "session_start", reason: "startup" }, ctx("json")); -await handlers.session_shutdown({ type: "session_shutdown", reason: "quit" }, ctx("tui")); -await handlers.session_shutdown({ type: "session_shutdown", reason: "new" }, ctx("tui")); -await handlers.agent_settled({ type: "agent_settled" }, ctx("tui")); +const fire = async (name, event, mode) => { + try { + await handlers[name](event, ctx(mode)); + } catch (error) { + console.log(error.message); + } +}; +await fire("session_start", { type: "session_start", reason: "startup" }, "tui"); +await fire("session_start", { type: "session_start", reason: "startup" }, "json"); +await fire("session_shutdown", { type: "session_shutdown", reason: "quit" }, "tui"); +await fire("session_shutdown", { type: "session_shutdown", reason: "new" }, "tui"); +await fire("agent_settled", { type: "agent_settled" }, "tui"); ` func TestPiExtensionRunsRegisteredCommand(t *testing.T) { @@ -1144,6 +1179,15 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { } 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], @@ -1184,4 +1228,11 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { }`, lines[0]) assert.Contains(lines[1], `"reason":"new"`) assert.Contains(lines[2], `"agent_settled"`) + // A failed command is reported once its event's other commands have run. + failures := strings.Split(strings.TrimSpace(string(output)), "\n") + require.Len(failures, 2, string(output)) + assert.Contains(failures[0], "agenthook session_start commands failed: "+missing+" --source missing-hook: could not start") + assert.NotContains(failures[0], "TestPiExtensionHelper") + assert.Contains(failures[1], "agenthook agent_settled commands failed: ") + assert.Contains(failures[1], "timed out after 1s") } diff --git a/agenthook/doc.go b/agenthook/doc.go index 43473da4..c04f08d4 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -9,7 +9,8 @@ // 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. +// terminal, so print, json, and rpc runs, subagents included, stay silent. 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 f2da86c6..b0f96315 100644 --- a/agenthook/handler_test.go +++ b/agenthook/handler_test.go @@ -937,11 +937,6 @@ func TestHandleRejectsUnmappedPiEvents(t *testing.T) { handler Handler want string }{ - { - name: "quit keeps the session", - payload: `"hook_event_name":"session_shutdown","reason":"quit"`, - handler: &lifecycleHandler{}, want: "SessionEnd input missing reason", - }, { name: "empty prompt", payload: `"hook_event_name":"before_agent_start","prompt":""`, diff --git a/agenthook/pi.go b/agenthook/pi.go index 1c39160c..cf63c20d 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -35,6 +35,7 @@ func piProfile() profileSpec { spec.configEnvDir = piAgentDir spec.eventName = piEventName spec.script = piExtension + spec.checkScriptLoads = piExtensionLoads // Pi extension handlers run in-process; the generated extension ignores // command output, so control decisions have nowhere to go. spec.responseFormat = responseObservational @@ -102,7 +103,7 @@ func fileURLPath(raw string) (string, error) { if err != nil { return "", err } - remote := parsed.Host != "" && parsed.Host != "localhost" + remote := parsed.Host != "" && !strings.EqualFold(parsed.Host, "localhost") if runtime.GOOS != "windows" { if remote { return "", fmt.Errorf("file URL host must be localhost or empty: %s", raw) @@ -118,8 +119,8 @@ func fileURLPath(raw string) (string, error) { // 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. A shutdown for quit or reload loses its -// reason so Handle refuses it rather than retiring a resumable session. +// 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 { @@ -149,9 +150,6 @@ func promotePiReason(payload map[string]json.RawMessage) error { "resume": string(SessionEndResume), "fork": string(SessionEndOther), }[reason] - if value == "" { - delete(payload, "reason") - } default: return nil } @@ -165,3 +163,51 @@ func promotePiReason(payload map[string]json.RawMessage) error { payload[field] = encoded return nil } + +// piExtensionLoads refuses a directory whose package.json pi.extensions or +// index.ts/index.js makes Pi load only those entries, which would leave the +// generated extension silently unloaded: +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/package-manager.ts#L563-L601 +func piExtensionLoads(path string) error { + dir := filepath.Dir(path) + var entries []string + if data, err := os.ReadFile(filepath.Join(dir, "package.json")); err == nil { + var manifest struct { + Pi struct { + Extensions []string `json:"extensions"` + } `json:"pi"` + } + if json.Unmarshal(data, &manifest) == nil { + for _, entry := range manifest.Pi.Extensions { + resolved := entry + if !filepath.IsAbs(resolved) { + resolved = filepath.Join(dir, entry) + } + if _, err := os.Stat(resolved); err == nil { + entries = append(entries, resolved) + } + } + } + } + if len(entries) == 0 { + for _, name := range []string{"index.ts", "index.js"} { + if _, err := os.Stat(filepath.Join(dir, name)); err == nil { + entries = []string{filepath.Join(dir, name)} + break + } + } + } + for _, entry := range entries { + if filepath.Clean(entry) == filepath.Clean(path) { + return nil + } + } + if len(entries) > 0 { + return fmt.Errorf( + "Pi loads only %s from %s, so it would never load %s; "+ + "add it to package.json pi.extensions or install to another directory", + strings.Join(entries, ", "), dir, filepath.Base(path), + ) + } + return nil +} diff --git a/agenthook/pi_extension.js b/agenthook/pi_extension.js index 77220ab7..c917f48e 100644 --- a/agenthook/pi_extension.js +++ b/agenthook/pi_extension.js @@ -1,4 +1,8 @@ // 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"]; export default function (pi) { diff --git a/agenthook/profile.go b/agenthook/profile.go index 07a478b0..9e93c64e 100644 --- a/agenthook/profile.go +++ b/agenthook/profile.go @@ -115,6 +115,7 @@ type profileSpec struct { sessionSourceRequirement inputRequirement sessionEndReasonRequirement inputRequirement script string + checkScriptLoads func(path string) error } var profileOrder = []Agent{ diff --git a/agenthook/script.go b/agenthook/script.go index 36ba9e54..f12ed837 100644 --- a/agenthook/script.go +++ b/agenthook/script.go @@ -39,6 +39,11 @@ func planScriptConfig( if !exists && uninstall { return nil, false, nil } + if !uninstall && spec.checkScriptLoads != nil { + if err := spec.checkScriptLoads(path); err != nil { + return nil, false, err + } + } root := map[string]any{} if exists { block, err := scriptBlock(existing, path) diff --git a/agenthook/script_runtime.js b/agenthook/script_runtime.js index c1e88f4a..0d68ba1d 100644 --- a/agenthook/script_runtime.js +++ b/agenthook/script_runtime.js @@ -2,9 +2,10 @@ import { spawn } from "node:child_process"; const defaultTimeoutSeconds = 60; -// Runs one argv without a shell, writing the payload to stdin. 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. +// Runs one argv without a shell, 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; @@ -12,21 +13,24 @@ function runCommand(handler, payload) { child = spawn(handler.command, Array.isArray(handler.args) ? handler.args : [], { stdio: ["pipe", "ignore", "ignore"], }); - } catch { - resolve(); + } catch (error) { + resolve(`could not start: ${error.message}`); return; } const seconds = handler.timeout > 0 ? handler.timeout : defaultTimeoutSeconds; const timer = setTimeout(() => { child.kill("SIGKILL"); - resolve(); + resolve(`timed out after ${seconds}s`); }, seconds * 1000); - const done = () => { + const finish = (failure) => { clearTimeout(timer); - resolve(); + resolve(failure); }; - child.once("error", done); - child.once("close", done); + 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}`); + }); if (child.stdin) { child.stdin.on("error", () => {}); child.stdin.end(JSON.stringify(payload)); @@ -34,14 +38,23 @@ function runCommand(handler, payload) { }); } -// Runs every registered command for a native event in order. +// 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 = []; const entries = (config.hooks ?? {})[event]; for (const entry of Array.isArray(entries) ? entries : []) { for (const handler of Array.isArray(entry?.hooks) ? entry.hooks : []) { if (handler?.type === "command" && typeof handler.command === "string") { - await runCommand(handler, payload); + const failure = await runCommand(handler, payload); + if (failure) { + const argv = [handler.command, ...(Array.isArray(handler.args) ? handler.args : [])]; + failures.push(`${argv.join(" ")}: ${failure}`); + } } } } + if (failures.length > 0) { + throw new Error(`agenthook ${event} commands failed: ${failures.join("; ")}`); + } } From e8263d49f3c2e0f7e381211f703e2811ee80fe8e Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 11:26:53 -0400 Subject: [PATCH 04/13] fix(agenthook): keep Pi resume on --session and drop the extra path handling --- agentcli/README.md | 2 +- agentcli/agentcli_test.go | 22 +---------- agentcli/pi.go | 15 +------ agenthook/agenthook_test.go | 29 -------------- agenthook/doc.go | 5 ++- agenthook/handler_test.go | 14 ++++++- agenthook/pi.go | 78 ++----------------------------------- agenthook/profile.go | 1 - agenthook/script.go | 5 --- 9 files changed, 24 insertions(+), 147 deletions(-) diff --git a/agentcli/README.md b/agentcli/README.md index 5fdfc2c7..b6126ecd 100644 --- a/agentcli/README.md +++ b/agentcli/README.md @@ -77,7 +77,7 @@ autonomy and the built-in skills control; its other controls are `exec` flags. | Kiro | noninteractive | argument | `chat --resume-id ID` | text | low, medium, high, xhigh, maximum | | Kilo | noninteractive | stdin | `run --session ID` | text, JSONL | low, medium, high, xhigh, maximum | | Factory Droid | interactive, noninteractive | stdin, noninteractive only | `--resume ID`, `exec --session-id ID` | text, JSON, JSONL | low, medium, high, xhigh, maximum, noninteractive only | -| Pi | interactive, noninteractive | argument and `@file` | `--session-id ID` (Pi 0.76.0 or later), `--session PATH` for a path or `.jsonl` file | text, JSONL | low, medium, high, xhigh, maximum | +| Pi | interactive, noninteractive | argument and `@file` | `--session ID` | text, JSONL | low, medium, high, xhigh, maximum | `ReasoningXHigh` and `ReasoningMaximum` are distinct. Adapters with a native `max` value, including Codex, map only `ReasoningMaximum` to it. Droid accepts diff --git a/agentcli/agentcli_test.go b/agentcli/agentcli_test.go index 19286ee2..0a7743a2 100644 --- a/agentcli/agentcli_test.go +++ b/agentcli/agentcli_test.go @@ -75,24 +75,6 @@ func TestInvocationContracts(t *testing.T) { } } -func TestPiResumeSendsPathsToSessionAndIDsToSessionID(t *testing.T) { - t.Parallel() - tests := []struct { - session string - flag string - }{ - {session: "019a-session", flag: "--session-id"}, - {session: "/home/me/.pi/agent/sessions/1.jsonl", flag: "--session"}, - {session: `C:\Users\me\.pi\agent\sessions\1.jsonl`, flag: "--session"}, - {session: "1.jsonl", flag: "--session"}, - } - for _, test := range tests { - got, err := mustAgent(t, agentcli.Pi, agentcli.Command{}).Resume(test.session, agentcli.Request{}) - require.NoError(t, err) - assert.Equal(t, []string{"pi", test.flag, test.session}, got.Argv) - } -} - func TestInteractiveResumePreservesConfiguredOptions(t *testing.T) { t.Parallel() tests := []struct { @@ -103,7 +85,7 @@ func TestInteractiveResumePreservesConfiguredOptions(t *testing.T) { }{ {agentcli.Codex, agentcli.Command{Executable: "codex-custom", Options: []string{"--full-auto", "--profile", "team"}}, agentcli.Request{}, []string{"codex-custom", "--full-auto", "--profile", "team", "resume", "session-1"}}, {agentcli.Claude, agentcli.Command{Executable: "claude-custom", Options: []string{"--setting-sources", "project"}}, agentcli.Request{}, []string{"claude-custom", "--setting-sources", "project", "--resume", "session-1"}}, - {agentcli.Pi, agentcli.Command{Executable: "pi-custom", Options: []string{"--offline"}}, agentcli.Request{}, []string{"pi-custom", "--offline", "--session-id", "session-1"}}, + {agentcli.Pi, agentcli.Command{Executable: "pi-custom", Options: []string{"--offline"}}, agentcli.Request{}, []string{"pi-custom", "--offline", "--session", "session-1"}}, {agentcli.Copilot, agentcli.Command{Executable: "copilot-custom", Options: []string{"--add-dir", "shared"}}, agentcli.Request{}, []string{"copilot-custom", "--add-dir", "shared", "--resume=session-1"}}, {agentcli.Cursor, agentcli.Command{Executable: "cursor-agent", Options: []string{"--workspace", "repo"}}, agentcli.Request{}, []string{"cursor-agent", "--workspace", "repo", "--resume", "session-1"}}, {agentcli.Droid, agentcli.Command{Executable: "droid-custom", Options: []string{"--append-system-prompt", "Run tests."}}, agentcli.Request{Autonomy: agentcli.AutonomyLow, DisableSkills: true}, []string{"droid-custom", "--append-system-prompt", "Run tests.", "--resume", "session-1", "--auto", "low", "--disable-builtin-skills"}}, @@ -175,7 +157,7 @@ func TestConfiguredOptionsKeepTheirArityAndOrder(t *testing.T) { }{ {agentcli.Codex, agentcli.Command{Executable: "codex-custom", Options: []string{"--profile=team", "-c", "feature.test=true", "--add-dir", "-shared"}}, "", []string{"codex-custom", "--profile=team", "-c", "feature.test=true", "--add-dir", "-shared", "resume", "session-1"}}, {agentcli.Claude, agentcli.Command{Options: []string{"--setting-sources=project", "--plugin-dir", "one", "--plugin-dir", "-two"}}, "", []string{"claude", "--setting-sources=project", "--plugin-dir", "one", "--plugin-dir", "-two", "--resume", "session-1"}}, - {agentcli.Pi, agentcli.Command{Options: []string{"-ne", "--tui-mode", "fullscreen", "--offline"}}, "", []string{"pi", "-ne", "--tui-mode", "fullscreen", "--offline", "--session-id", "session-1"}}, + {agentcli.Pi, agentcli.Command{Options: []string{"-ne", "--tui-mode", "fullscreen", "--offline"}}, "", []string{"pi", "-ne", "--tui-mode", "fullscreen", "--offline", "--session", "session-1"}}, {agentcli.Kiro, agentcli.Command{Executable: "kiro-custom", Options: []string{"--wrap", "never"}}, agentcli.NonInteractive, []string{"kiro-custom", "chat", "--wrap", "never", "--no-interactive", "--resume-id", "session-1"}}, {agentcli.Droid, agentcli.Command{Executable: "droid-custom", Options: []string{"--append-system-prompt", "review only"}}, agentcli.NonInteractive, []string{"droid-custom", "exec", "--append-system-prompt", "review only", "--session-id", "session-1"}}, } diff --git a/agentcli/pi.go b/agentcli/pi.go index 5156856f..f857535c 100644 --- a/agentcli/pi.go +++ b/agentcli/pi.go @@ -82,7 +82,7 @@ func buildPi(a *adapter, sessionID string, request Request) (Invocation, error) args = append(args, "--mode", "json") } if sessionID != "" { - args = append(args, piSessionFlag(sessionID), sessionID) + args = append(args, "--session", sessionID) } if request.Provider != "" { args = append(args, "--provider", request.Provider) @@ -128,16 +128,3 @@ func validatePiRequest(mode Mode, request Request) error { } return nil } - -// piSessionFlag follows Pi's own test for a session path. A path goes to -// --session, which rejects nothing; an ID goes to --session-id, which opens the -// exact project-local session or creates it, so a session Pi has not saved yet -// still resumes: -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/main.ts#L252-L256 -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/cli.md -func piSessionFlag(session string) string { - if strings.ContainsAny(session, "/\\") || strings.HasSuffix(session, ".jsonl") { - return "--session" - } - return "--session-id" -} diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index ab6201a4..878e6cdb 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -942,8 +942,6 @@ func TestConfigPathNormalizesPiAgentDirAsPiDoes(t *testing.T) { {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"}, - {env: pick("file:///C:/pi/agent", "file:///pi/agent"), want: pick(`C:\pi\agent`, "/pi/agent")}, - {env: pick("file://LOCALHOST/C:/pi/agent", "file://LOCALHOST/pi/agent"), want: pick(`C:\pi\agent`, "/pi/agent")}, } for _, tt := range tests { t.Run(tt.env, func(t *testing.T) { @@ -1099,33 +1097,6 @@ func TestInstallPiKeepsArgumentThatLooksLikeBlockMarker(t *testing.T) { assert.Empty(t, piScriptHooks(t, path)) } -func TestInstallPiRefusesDirectoryPiLoadsOnlyEntriesFrom(t *testing.T) { - tests := map[string]map[string]string{ - "index.js": {"index.js": "export default () => {};\n"}, - "index.ts": {"index.ts": "export default () => {};\n"}, - "package.json": {"package.json": `{"pi":{"extensions":["main.js"]}}`, "main.js": ""}, - } - for name, files := range tests { - t.Run(name, func(t *testing.T) { - dir := t.TempDir() - for file, content := range files { - require.NoError(t, os.WriteFile(filepath.Join(dir, file), []byte(content), 0o600)) - } - path := filepath.Join(dir, "agenthook.js") - - _, err := Install(AgentPi, InstallOptions{ - ConfigPath: path, - Executable: "/opt/hook", - Arguments: []string{"--source", "shared-agent-hook-test"}, - Marker: testMarker, - }) - - require.ErrorContains(t, err, "would never load agenthook.js") - assert.NoFileExists(t, path) - }) - } -} - func TestPiExtensionHelper(t *testing.T) { out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") if out == "" { diff --git a/agenthook/doc.go b/agenthook/doc.go index c04f08d4..7014197d 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -9,8 +9,9 @@ // 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. It -// needs Pi 0.80.4 or later. +// 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. 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 b0f96315..5f8a90da 100644 --- a/agenthook/handler_test.go +++ b/agenthook/handler_test.go @@ -586,9 +586,18 @@ func TestHandleRejectsMissingRequiredEventFields(t *testing.T) { type lifecycleHandler struct { NoopHandler sessionStart *SessionStartInput + prompt *UserPromptSubmitInput sessionEnd *SessionEndInput } +func (h *lifecycleHandler) UserPromptSubmit( + _ context.Context, + input UserPromptSubmitInput, +) (UserPromptSubmitOutput, error) { + h.prompt = &input + return UserPromptSubmitOutput{}, nil +} + func (h *lifecycleHandler) SessionStart( _ context.Context, input SessionStartInput, @@ -909,7 +918,10 @@ func TestHandleDispatchesPiEvents(t *testing.T) { name: "prompt", payload: `"hook_event_name":"before_agent_start","prompt":"fix it"`, check: func(t *testing.T, h *lifecycleHandler) { t.Helper() - assert.Nil(t, h.sessionStart) + require.NotNil(t, h.prompt) + assert.Equal(t, "fix it", h.prompt.Prompt) + assert.Equal(t, "pi-1", h.prompt.SessionID) + assert.Equal(t, EventUserPromptSubmit, h.prompt.HookEventName) }, }, } diff --git a/agenthook/pi.go b/agenthook/pi.go index cf63c20d..26a74ed4 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -4,7 +4,6 @@ import ( _ "embed" "encoding/json" "fmt" - "net/url" "os" "path/filepath" "regexp" @@ -24,6 +23,8 @@ func piProfile() profileSpec { // 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, }, @@ -35,7 +36,6 @@ func piProfile() profileSpec { spec.configEnvDir = piAgentDir spec.eventName = piEventName spec.script = piExtension - spec.checkScriptLoads = piExtensionLoads // Pi extension handlers run in-process; the generated extension ignores // command output, so control decisions have nowhere to go. spec.responseFormat = responseObservational @@ -69,8 +69,8 @@ func piEventName(event Event) string { var piWindowsShellPath = regexp.MustCompile(`(?i)^/(?:mnt/|cygdrive/)?([a-z])(?:/(.*))?$`) -// piAgentDir mirrors Pi's normalizePath for PI_CODING_AGENT_DIR: Git Bash, -// MSYS, Cygwin, and WSL drive paths on Windows, then ~, then file: URLs: +// 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, "/") && @@ -91,31 +91,9 @@ func piAgentDir(dir string) (string, error) { } return filepath.Join(home, rest), nil } - if strings.HasPrefix(dir, "file://") { - return fileURLPath(dir) - } return dir, nil } -// fileURLPath follows Node's fileURLToPath for the cases Pi accepts. -func fileURLPath(raw string) (string, error) { - parsed, err := url.Parse(raw) - if err != nil { - return "", err - } - remote := parsed.Host != "" && !strings.EqualFold(parsed.Host, "localhost") - if runtime.GOOS != "windows" { - if remote { - return "", fmt.Errorf("file URL host must be localhost or empty: %s", raw) - } - return parsed.Path, nil - } - if remote { - return `\\` + parsed.Host + filepath.FromSlash(parsed.Path), nil - } - return filepath.FromSlash(strings.TrimPrefix(parsed.Path, "/")), 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 @@ -163,51 +141,3 @@ func promotePiReason(payload map[string]json.RawMessage) error { payload[field] = encoded return nil } - -// piExtensionLoads refuses a directory whose package.json pi.extensions or -// index.ts/index.js makes Pi load only those entries, which would leave the -// generated extension silently unloaded: -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/package-manager.ts#L563-L601 -func piExtensionLoads(path string) error { - dir := filepath.Dir(path) - var entries []string - if data, err := os.ReadFile(filepath.Join(dir, "package.json")); err == nil { - var manifest struct { - Pi struct { - Extensions []string `json:"extensions"` - } `json:"pi"` - } - if json.Unmarshal(data, &manifest) == nil { - for _, entry := range manifest.Pi.Extensions { - resolved := entry - if !filepath.IsAbs(resolved) { - resolved = filepath.Join(dir, entry) - } - if _, err := os.Stat(resolved); err == nil { - entries = append(entries, resolved) - } - } - } - } - if len(entries) == 0 { - for _, name := range []string{"index.ts", "index.js"} { - if _, err := os.Stat(filepath.Join(dir, name)); err == nil { - entries = []string{filepath.Join(dir, name)} - break - } - } - } - for _, entry := range entries { - if filepath.Clean(entry) == filepath.Clean(path) { - return nil - } - } - if len(entries) > 0 { - return fmt.Errorf( - "Pi loads only %s from %s, so it would never load %s; "+ - "add it to package.json pi.extensions or install to another directory", - strings.Join(entries, ", "), dir, filepath.Base(path), - ) - } - return nil -} diff --git a/agenthook/profile.go b/agenthook/profile.go index 9e93c64e..07a478b0 100644 --- a/agenthook/profile.go +++ b/agenthook/profile.go @@ -115,7 +115,6 @@ type profileSpec struct { sessionSourceRequirement inputRequirement sessionEndReasonRequirement inputRequirement script string - checkScriptLoads func(path string) error } var profileOrder = []Agent{ diff --git a/agenthook/script.go b/agenthook/script.go index f12ed837..36ba9e54 100644 --- a/agenthook/script.go +++ b/agenthook/script.go @@ -39,11 +39,6 @@ func planScriptConfig( if !exists && uninstall { return nil, false, nil } - if !uninstall && spec.checkScriptLoads != nil { - if err := spec.checkScriptLoads(path); err != nil { - return nil, false, err - } - } root := map[string]any{} if exists { block, err := scriptBlock(existing, path) From d9ca022a587b707d5c77838ed3fd96e08385436c Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 11:40:50 -0400 Subject: [PATCH 05/13] fix(agenthook): report a Pi session only once its ID resumes --- agenthook/agenthook_test.go | 92 +++++++++++++++++++++++++------------ agenthook/doc.go | 6 ++- agenthook/pi_extension.js | 87 +++++++++++++++++++++++++---------- agenthook/script_runtime.js | 24 +++------- 4 files changed, 138 insertions(+), 71 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index 878e6cdb..b1cf665b 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1118,30 +1118,51 @@ func TestPiExtensionHelper(t *testing.T) { } const piExtensionDriver = ` +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; import { pathToFileURL } from "node:url"; const extension = await import(pathToFileURL(process.argv[2]).href); -const handlers = {}; -extension.default({ on: (name, handler) => { handlers[name] = handler; } }); -const ctx = (mode) => ({ +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: "/work", - sessionManager: { getSessionId: () => "pi-session-1", getSessionFile: () => "/sessions/1.jsonl" }, + sessionManager: { getSessionId: () => id, getSessionFile: () => file }, }); -const fire = async (name, event, mode) => { +const fire = async (pi, name, event, context) => { try { - await handlers[name](event, ctx(mode)); + await pi[name]({ type: name, ...event }, context); } catch (error) { - console.log(error.message); + console.log(name + ": " + error.message); } }; -await fire("session_start", { type: "session_start", reason: "startup" }, "tui"); -await fire("session_start", { type: "session_start", reason: "startup" }, "json"); -await fire("session_shutdown", { type: "session_shutdown", reason: "quit" }, "tui"); -await fire("session_shutdown", { type: "session_shutdown", reason: "new" }, "tui"); -await fire("agent_settled", { type: "agent_settled" }, "tui"); + +// Fresh start: Pi has no session file yet, so SessionStart waits for the first prompt. +let pi = runtime(); +await fire(pi, "session_start", { reason: "startup" }, ctx("a", fileA)); +await fire(pi, "before_agent_start", { prompt: "one" }, ctx("a", fileA)); +writeFileSync(fileA, "{}\n"); +await fire(pi, "agent_settled", {}, ctx("a", fileA)); + +// /new: the replaced session ends; the new one never gets a prompt, so its replacement 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 TestPiExtensionRunsRegisteredCommand(t *testing.T) { +func TestPiExtensionReportsResumableSessions(t *testing.T) { assert := assert.New(t) require := require.New(t) node, err := exec.LookPath("node") @@ -1166,6 +1187,7 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { Marker: testMarker, Hooks: []Hook{ {Event: EventSessionStart}, + {Event: EventUserPromptSubmit}, {Event: EventSessionEnd}, {Event: EventStop, Timeout: time.Second}, }, @@ -1178,7 +1200,7 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { 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) + cmd := exec.CommandContext(t.Context(), node, driver, module, dir) cmd.Env = append(os.Environ(), "KIT_AGENTHOOK_PI_HELPER_OUT="+out) started := time.Now() @@ -1188,22 +1210,34 @@ func TestPiExtensionRunsRegisteredCommand(t *testing.T) { assert.Less(time.Since(started), 30*time.Second, "the timed-out command was not killed") payloads, err := os.ReadFile(out) require.NoError(err) - lines := strings.Split(strings.TrimSpace(string(payloads)), "\n") - require.Len(lines, 3) - assert.JSONEq(`{ - "hook_event_name":"session_start", - "session_id":"pi-session-1", - "transcript_path":"/sessions/1.jsonl", - "cwd":"/work", - "reason":"startup" -}`, lines[0]) - assert.Contains(lines[1], `"reason":"new"`) - assert.Contains(lines[2], `"agent_settled"`) + 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"` + } + var reports []report + for line := range strings.Lines(strings.TrimSpace(string(payloads))) { + var r report + require.NoError(json.Unmarshal([]byte(line), &r)) + reports = append(reports, r) + } + fileA := filepath.Join(dir, "a.jsonl") + 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, 2, string(output)) - assert.Contains(failures[0], "agenthook session_start commands failed: "+missing+" --source missing-hook: could not start") - assert.NotContains(failures[0], "TestPiExtensionHelper") - assert.Contains(failures[1], "agenthook agent_settled commands failed: ") + require.Len(failures, 3, string(output)) + assert.Contains(failures[0], "before_agent_start: "+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/doc.go b/agenthook/doc.go index 7014197d..26b87292 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -11,7 +11,11 @@ // 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. It needs Pi 0.80.4 or later. +// resume, or fork), never on quit or reload. A new session reports its +// SessionStart with its first prompt, once Pi has saved the session so its ID +// resumes. 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/pi_extension.js b/agenthook/pi_extension.js index c917f48e..35a5cf0f 100644 --- a/agenthook/pi_extension.js +++ b/agenthook/pi_extension.js @@ -1,3 +1,5 @@ +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: @@ -5,29 +7,66 @@ // 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) { - for (const name of Object.keys(config.hooks ?? {})) { - pi.on(name, async (event, ctx) => { - // 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 - if (ctx.mode !== "tui") return; - // Report a shutdown only when another session replaces this one; quit - // and reload keep the session resumable. - if (name === "session_shutdown" && !replacedSessionReasons.includes(event.reason)) return; - 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; - if (typeof event.reason === "string") payload.reason = event.reason; - if (name === "before_agent_start") { - if (typeof event.prompt !== "string" || event.prompt === "") return; - payload.prompt = event.prompt; - } - await runHooks(name, payload); - }); - } + // A SessionStart held until the session file exists, so a reported ID always + // resumes. Pi writes the file once the session holds a user message: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/session-manager.ts#L1160-L1185 + let held = null; + let reported = false; + + const payloadFor = (name, ctx) => { + 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; + }; + + // 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, (event, ctx) => (ctx.mode === "tui" ? handler(event, ctx) : undefined)); + + on("session_start", async (event, ctx) => { + const payload = { ...payloadFor("session_start", ctx), reason: event.reason }; + if (!payload.transcript_path || !existsSync(payload.transcript_path)) { + held = payload; + return; + } + held = null; + reported = true; + await runHooks("session_start", payload); + }); + + // before_agent_start runs just before Pi persists the turn's user message: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/agent-session.ts#L2060-L2109 + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/agent-session.ts#L1154-L1161 + on("before_agent_start", async (event, ctx) => { + const failures = []; + if (held && held.session_id === ctx.sessionManager.getSessionId()) { + const start = held; + held = null; + reported = true; + await runHooks("session_start", start).catch((error) => failures.push(error.message)); + } + if (typeof event.prompt === "string" && event.prompt !== "") { + const payload = { ...payloadFor("before_agent_start", ctx), prompt: event.prompt }; + await runHooks("before_agent_start", payload).catch((error) => failures.push(error.message)); + } + if (failures.length > 0) throw new Error(failures.join("; ")); + }); + + on("agent_settled", (_event, ctx) => (reported ? runHooks("agent_settled", payloadFor("agent_settled", ctx)) : undefined)); + + // Report a shutdown only for a reported session that another session + // replaces; quit and reload keep the session resumable. + on("session_shutdown", async (event, ctx) => { + if (!reported || !replacedSessionReasons.includes(event.reason)) return; + await runHooks("session_shutdown", { ...payloadFor("session_shutdown", ctx), reason: event.reason }); + }); } diff --git a/agenthook/script_runtime.js b/agenthook/script_runtime.js index 0d68ba1d..09b61268 100644 --- a/agenthook/script_runtime.js +++ b/agenthook/script_runtime.js @@ -10,9 +10,7 @@ function runCommand(handler, payload) { return new Promise((resolve) => { let child; try { - child = spawn(handler.command, Array.isArray(handler.args) ? handler.args : [], { - stdio: ["pipe", "ignore", "ignore"], - }); + child = spawn(handler.command, handler.args, { stdio: ["pipe", "ignore", "ignore"] }); } catch (error) { resolve(`could not start: ${error.message}`); return; @@ -31,10 +29,8 @@ function runCommand(handler, payload) { if (signal) finish(`killed by ${signal}`); else finish(code === 0 ? null : `exited with status ${code}`); }); - if (child.stdin) { - child.stdin.on("error", () => {}); - child.stdin.end(JSON.stringify(payload)); - } + child.stdin.on("error", () => {}); + child.stdin.end(JSON.stringify(payload)); }); } @@ -42,16 +38,10 @@ function runCommand(handler, payload) { // error naming each command that failed so the harness reports it. async function runHooks(event, payload) { const failures = []; - const entries = (config.hooks ?? {})[event]; - for (const entry of Array.isArray(entries) ? entries : []) { - for (const handler of Array.isArray(entry?.hooks) ? entry.hooks : []) { - if (handler?.type === "command" && typeof handler.command === "string") { - const failure = await runCommand(handler, payload); - if (failure) { - const argv = [handler.command, ...(Array.isArray(handler.args) ? handler.args : [])]; - failures.push(`${argv.join(" ")}: ${failure}`); - } - } + 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) { From 4753693f3166bbf503545832c56d267ba3231027 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 11:51:53 -0400 Subject: [PATCH 06/13] fix(agenthook): report Pi events only once the session file exists --- agenthook/agenthook_test.go | 26 +++++++-- agenthook/doc.go | 11 ++-- agenthook/pi_extension.js | 102 ++++++++++++++++++++++-------------- 3 files changed, 90 insertions(+), 49 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index b1cf665b..9a90ccf3 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1118,7 +1118,7 @@ func TestPiExtensionHelper(t *testing.T) { } const piExtensionDriver = ` -import { writeFileSync } from "node:fs"; +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); @@ -1141,15 +1141,30 @@ const fire = async (pi, name, event, context) => { console.log(name + ": " + error.message); } }; +const nothingSent = (when) => { + if (existsSync(process.env.KIT_AGENTHOOK_PI_HELPER_OUT)) console.log("sent " + when); +}; -// Fresh start: Pi has no session file yet, so SessionStart waits for the first prompt. +// --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"); writeFileSync(fileA, "{}\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 never gets a prompt, so its replacement sends nothing. +// /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)); @@ -1224,6 +1239,9 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { 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"}, @@ -1235,7 +1253,7 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { 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], "before_agent_start: "+failed) + 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) diff --git a/agenthook/doc.go b/agenthook/doc.go index 26b87292..5a3e7b72 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -11,11 +11,12 @@ // 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. A new session reports its -// SessionStart with its first prompt, once Pi has saved the session so its ID -// resumes. 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. +// 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/pi_extension.js b/agenthook/pi_extension.js index 35a5cf0f..89feb0c9 100644 --- a/agenthook/pi_extension.js +++ b/agenthook/pi_extension.js @@ -10,63 +10,85 @@ 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) { - // A SessionStart held until the session file exists, so a reported ID always - // resumes. Pi writes the file once the session holds a user message: + // 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 held = null; - let reported = false; + let heldStart = null; + const heldPrompts = []; - const payloadFor = (name, ctx) => { - const payload = { - hook_event_name: name, - session_id: ctx.sessionManager.getSessionId(), - cwd: ctx.cwd, - }; + 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; + 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, (event, ctx) => (ctx.mode === "tui" ? handler(event, ctx) : undefined)); + 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) => { - const payload = { ...payloadFor("session_start", ctx), reason: event.reason }; - if (!payload.transcript_path || !existsSync(payload.transcript_path)) { - held = payload; - return; - } - held = null; - reported = true; - await runHooks("session_start", payload); + on("session_start", async (event, ctx, failures) => { + heldStart = payloadFor("session_start", ctx, { reason: event.reason }); + await flush(ctx, failures); }); - // before_agent_start runs just before Pi persists the turn's user message: + // 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 - // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/agent-session.ts#L1154-L1161 - on("before_agent_start", async (event, ctx) => { - const failures = []; - if (held && held.session_id === ctx.sessionManager.getSessionId()) { - const start = held; - held = null; - reported = true; - await runHooks("session_start", start).catch((error) => failures.push(error.message)); - } + on("before_agent_start", async (event, ctx, failures) => { if (typeof event.prompt === "string" && event.prompt !== "") { - const payload = { ...payloadFor("before_agent_start", ctx), prompt: event.prompt }; - await runHooks("before_agent_start", payload).catch((error) => failures.push(error.message)); + heldPrompts.push(payloadFor("before_agent_start", ctx, { prompt: event.prompt })); } - if (failures.length > 0) throw new Error(failures.join("; ")); + await flush(ctx, failures); }); - on("agent_settled", (_event, ctx) => (reported ? runHooks("agent_settled", payloadFor("agent_settled", ctx)) : undefined)); + // 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 for a reported session that another session - // replaces; quit and reload keep the session resumable. - on("session_shutdown", async (event, ctx) => { - if (!reported || !replacedSessionReasons.includes(event.reason)) return; - await runHooks("session_shutdown", { ...payloadFor("session_shutdown", ctx), reason: event.reason }); + // 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 })); }); } From 211f6be66aca3dca566f43bc8d0ed3580bd698f9 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 12:19:12 -0400 Subject: [PATCH 07/13] fix(agenthook): hold nothing when Pi is not persisting the session --- agenthook/agenthook_test.go | 4 +++- agenthook/pi_extension.js | 4 ++++ agentmcp/agentmcp_test.go | 5 ++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index 9a90ccf3..14a2ca70 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1160,7 +1160,9 @@ 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"); -writeFileSync(fileA, "{}\n"); +// 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)); diff --git a/agenthook/pi_extension.js b/agenthook/pi_extension.js index 89feb0c9..c5db5219 100644 --- a/agenthook/pi_extension.js +++ b/agenthook/pi_extension.js @@ -17,6 +17,8 @@ export default function (pi) { 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); @@ -56,6 +58,7 @@ export default function (pi) { }); on("session_start", async (event, ctx, failures) => { + if (!persisting(ctx)) return; heldStart = payloadFor("session_start", ctx, { reason: event.reason }); await flush(ctx, failures); }); @@ -64,6 +67,7 @@ export default function (pi) { // 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 })); } diff --git a/agentmcp/agentmcp_test.go b/agentmcp/agentmcp_test.go index b41258c0..172e3dda 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)) From 8139bc262e0d0c88cca3f971a13289e55439ea85 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 12:48:09 -0400 Subject: [PATCH 08/13] test(agenthook): fold Pi cases into the shared normalize and install tests --- agenthook/agenthook_test.go | 77 +++++++++--------- agenthook/handler_test.go | 152 ++++++++---------------------------- 2 files changed, 74 insertions(+), 155 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index 14a2ca70..d0aee5e1 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -196,7 +196,6 @@ func TestConfigPathHonorsAgentHomes(t *testing.T) { {agent: AgentCopilot, env: "COPILOT_HOME", path: filepath.Join("hooks", "agenthook.json")}, {agent: AgentGemini, env: "GEMINI_CLI_HOME", path: filepath.Join(".gemini", "settings.json")}, {agent: AgentHermes, env: "HERMES_HOME", path: "config.yaml"}, - {agent: AgentPi, env: "PI_CODING_AGENT_DIR", path: filepath.Join("extensions", "agenthook.js")}, {agent: AgentQwen, env: "QWEN_HOME", path: "settings.json"}, } for _, tt := range tests { @@ -767,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, @@ -991,27 +1020,31 @@ 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) { - _, err := Install(AgentPi, InstallOptions{ + install := func(executable, source string, extra ...string) Result { + result, err := Install(AgentPi, InstallOptions{ ConfigPath: path, Executable: executable, - Arguments: []string{"agent-hook", "--source", source}, + 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") + install("/opt/b", "b-hook", scriptBlockEnd) assert.Equal( - []string{"/opt/a agent-hook --source a-hook", "/opt/b agent-hook --source b-hook"}, + []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{"/opt/b agent-hook --source b-hook", "/moved/a agent-hook --source a-hook"}, + []string{bCommand, "/moved/a agent-hook --source a-hook"}, piCommands(hooks, "agent_settled"), ) assert.Len(piCommands(hooks, "before_agent_start"), 2) @@ -1019,10 +1052,7 @@ func TestInstallPiKeepsOtherApplicationsCommands(t *testing.T) { result, err := Uninstall(AgentPi, path, "--source a-hook") require.NoError(err) assert.True(result.Changed) - assert.Equal( - []string{"/opt/b agent-hook --source b-hook"}, - piCommands(piScriptHooks(t, path), "session_start"), - ) + assert.Equal([]string{bCommand}, piCommands(piScriptHooks(t, path), "session_start")) result, err = Uninstall(AgentPi, path, "--source b-hook") require.NoError(err) @@ -1072,31 +1102,6 @@ func TestPlanInstallPiRequiresExecutableWithoutMatchers(t *testing.T) { require.ErrorContains(t, err, "do not support matchers") } -func TestInstallPiKeepsArgumentThatLooksLikeBlockMarker(t *testing.T) { - require := require.New(t) - path := filepath.Join(t.TempDir(), "agenthook.js") - opts := InstallOptions{ - ConfigPath: path, - Executable: "/opt/hook", - Arguments: []string{scriptBlockEnd, "--source", "shared-agent-hook-test"}, - Marker: testMarker, - } - - _, err := Install(AgentPi, opts) - require.NoError(err) - result, err := Install(AgentPi, opts) - require.NoError(err) - assert.False(t, result.Changed) - assert.Equal(t, - []string{"/opt/hook " + scriptBlockEnd + " " + testMarker}, - piCommands(piScriptHooks(t, path), "session_start"), - ) - - _, err = Uninstall(AgentPi, path, testMarker) - require.NoError(err) - assert.Empty(t, piScriptHooks(t, path)) -} - func TestPiExtensionHelper(t *testing.T) { out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") if out == "" { diff --git a/agenthook/handler_test.go b/agenthook/handler_test.go index 5f8a90da..d253f02b 100644 --- a/agenthook/handler_test.go +++ b/agenthook/handler_test.go @@ -586,18 +586,9 @@ func TestHandleRejectsMissingRequiredEventFields(t *testing.T) { type lifecycleHandler struct { NoopHandler sessionStart *SessionStartInput - prompt *UserPromptSubmitInput sessionEnd *SessionEndInput } -func (h *lifecycleHandler) UserPromptSubmit( - _ context.Context, - input UserPromptSubmitInput, -) (UserPromptSubmitOutput, error) { - h.prompt = &input - return UserPromptSubmitOutput{}, nil -} - func (h *lifecycleHandler) SessionStart( _ context.Context, input SessionStartInput, @@ -636,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, @@ -858,120 +859,33 @@ func TestHandleRejectsOversizedPayload(t *testing.T) { assert.ErrorContains(t, err, "hook payload exceeds") } -func TestHandleDispatchesPiEvents(t *testing.T) { - const common = `"session_id":"pi-1","cwd":"/work","transcript_path":"/s/1.jsonl"` - tests := []struct { - name string - payload string - check func(*testing.T, *lifecycleHandler) - }{ - { - name: "startup", payload: `"hook_event_name":"session_start","reason":"startup"`, - check: func(t *testing.T, h *lifecycleHandler) { - t.Helper() - require.NotNil(t, h.sessionStart) - assert.Equal(t, SessionSourceStartup, h.sessionStart.Source) - assert.Equal(t, "pi-1", h.sessionStart.SessionID) - }, - }, - { - name: "new session", payload: `"hook_event_name":"session_start","reason":"new"`, - check: func(t *testing.T, h *lifecycleHandler) { - t.Helper() - require.NotNil(t, h.sessionStart) - assert.Equal(t, SessionSourceClear, h.sessionStart.Source) - }, - }, - { - name: "reload", payload: `"hook_event_name":"session_start","reason":"reload"`, - check: func(t *testing.T, h *lifecycleHandler) { - t.Helper() - require.NotNil(t, h.sessionStart) - assert.Empty(t, h.sessionStart.Source) - }, - }, - { - name: "replaced by new", payload: `"hook_event_name":"session_shutdown","reason":"new"`, - check: func(t *testing.T, h *lifecycleHandler) { - t.Helper() - require.NotNil(t, h.sessionEnd) - assert.Equal(t, SessionEndClear, h.sessionEnd.Reason) - }, - }, - { - name: "replaced by resume", payload: `"hook_event_name":"session_shutdown","reason":"resume"`, - check: func(t *testing.T, h *lifecycleHandler) { - t.Helper() - require.NotNil(t, h.sessionEnd) - assert.Equal(t, SessionEndResume, h.sessionEnd.Reason) - }, - }, - { - name: "replaced by fork", payload: `"hook_event_name":"session_shutdown","reason":"fork"`, - check: func(t *testing.T, h *lifecycleHandler) { - t.Helper() - require.NotNil(t, h.sessionEnd) - assert.Equal(t, SessionEndOther, h.sessionEnd.Reason) - }, - }, - { - name: "prompt", payload: `"hook_event_name":"before_agent_start","prompt":"fix it"`, - check: func(t *testing.T, h *lifecycleHandler) { - t.Helper() - require.NotNil(t, h.prompt) - assert.Equal(t, "fix it", h.prompt.Prompt) - assert.Equal(t, "pi-1", h.prompt.SessionID) - assert.Equal(t, EventUserPromptSubmit, h.prompt.HookEventName) - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var output bytes.Buffer - handler := &lifecycleHandler{} +func TestHandleDispatchesPiSessionEnd(t *testing.T) { + var output bytes.Buffer + handler := &lifecycleHandler{} - err := Handle( - t.Context(), AgentPi, strings.NewReader("{"+common+","+tt.payload+"}"), - &output, handler, - ) + err := Handle( + t.Context(), AgentPi, + strings.NewReader(`{"session_id":"pi-1","hook_event_name":"session_shutdown","reason":"new"}`), + &output, handler, + ) - require.NoError(t, err) - tt.check(t, handler) - assert.JSONEq(t, `{}`, output.String()) - }) - } + 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 TestHandleRejectsUnmappedPiEvents(t *testing.T) { - tests := []struct { - name string - payload string - handler Handler - want string - }{ - { - name: "empty prompt", - payload: `"hook_event_name":"before_agent_start","prompt":""`, - handler: &lifecycleHandler{}, want: "UserPromptSubmit input missing prompt", - }, - { - name: "stop control output", - payload: `"hook_event_name":"agent_settled"`, - handler: stopHandler{output: StopOutput{Decision: DecisionBlock, Reason: "work remains"}}, - want: "does not support Stop control output", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var output bytes.Buffer +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",`+tt.payload+"}"), - &output, tt.handler, - ) + err := Handle( + t.Context(), AgentPi, + strings.NewReader(`{"session_id":"pi-1","hook_event_name":"agent_settled"}`), + &output, handler, + ) - require.ErrorContains(t, err, tt.want) - assert.Empty(t, output.String()) - }) - } + require.ErrorContains(t, err, "does not support Stop control output") + assert.Empty(t, output.String()) } From 662e0cdfb786688aa8a4665bd77d1c075902cb6b Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 12:58:40 -0400 Subject: [PATCH 09/13] fix(agenthook): refuse a Pi extensions directory that would skip the hook module --- agenthook/agenthook_test.go | 44 ++++++++++++++++++++++++++++++ agenthook/doc.go | 5 ++-- agenthook/pi.go | 54 +++++++++++++++++++++++++++++++++++++ agenthook/profile.go | 1 + agenthook/script.go | 5 ++++ 5 files changed, 107 insertions(+), 2 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index d0aee5e1..00ef560f 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1102,6 +1102,50 @@ func TestPlanInstallPiRequiresExecutableWithoutMatchers(t *testing.T) { require.ErrorContains(t, err, "do not support matchers") } +func TestInstallPiRefusesDirectoryThatSkipsTheModule(t *testing.T) { + tests := []struct { + name string + files map[string]string + blocker string + }{ + {name: "index.js", files: map[string]string{"index.js": ""}, blocker: "index.js"}, + {name: "index.ts", files: map[string]string{"index.ts": ""}, blocker: "index.ts"}, + { + name: "manifest without the module", + files: map[string]string{"package.json": `{"pi":{"extensions":["main.js"]}}`, "main.js": ""}, + blocker: "package.json", + }, + { + name: "manifest listing the module before it exists", + files: map[string]string{"package.json": `{"pi":{"extensions":["agenthook.js"]}}`, "index.js": ""}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + for name, content := range tt.files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600)) + } + path := filepath.Join(dir, "agenthook.js") + + _, err := Install(AgentPi, InstallOptions{ + ConfigPath: path, + Executable: "/opt/hook", + Arguments: []string{"--source", "shared-agent-hook-test"}, + Marker: testMarker, + }) + + if tt.blocker == "" { + require.NoError(t, err) + assert.FileExists(t, path) + return + } + require.ErrorContains(t, err, "Pi loads only "+tt.blocker) + assert.NoFileExists(t, path) + }) + } +} + func TestPiExtensionHelper(t *testing.T) { out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") if out == "" { diff --git a/agenthook/doc.go b/agenthook/doc.go index 5a3e7b72..7c1f8784 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -14,8 +14,9 @@ // 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. +// --no-session run reports nothing. Install refuses an extensions directory +// with a root index.ts or index.js, or a package.json pi.extensions list that +// leaves the module out, since Pi would then never load it. // It needs Pi 0.80.4 or later. // // Applications identify their hooks with a stable marker embedded in the diff --git a/agenthook/pi.go b/agenthook/pi.go index 26a74ed4..7e40eb0c 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -36,6 +36,7 @@ func piProfile() profileSpec { spec.configEnvDir = piAgentDir spec.eventName = piEventName spec.script = piExtension + spec.checkScriptLoads = piExtensionLoads // Pi extension handlers run in-process; the generated extension ignores // command output, so control decisions have nowhere to go. spec.responseFormat = responseObservational @@ -141,3 +142,56 @@ func promotePiReason(payload map[string]json.RawMessage) error { payload[field] = encoded return nil } + +// piExtensionLoads refuses an extensions directory where Pi would load only a +// package.json pi.extensions list or a root index.ts or index.js, which would +// leave the generated module silently unloaded. A manifest entry counts when +// its file exists or it names the module about to be written: +// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/package-manager.ts#L563-L601 +func piExtensionLoads(path string) error { + dir := filepath.Dir(path) + manifestPath := filepath.Join(dir, "package.json") + if data, err := os.ReadFile(manifestPath); err == nil { + var manifest struct { + Pi struct { + Extensions []string `json:"extensions"` + } `json:"pi"` + } + if json.Unmarshal(data, &manifest) == nil { + listed := false + for _, entry := range manifest.Pi.Extensions { + resolved := entry + if !filepath.IsAbs(resolved) { + resolved = filepath.Join(dir, entry) + } + if filepath.Clean(resolved) == filepath.Clean(path) { + return nil + } + if _, err := os.Stat(resolved); err == nil { + listed = true + } + } + if listed { + return piExtensionBlocked(manifestPath, path) + } + } + } + for _, name := range []string{"index.ts", "index.js"} { + if index := filepath.Join(dir, name); fileExists(index) { + return piExtensionBlocked(index, path) + } + } + return nil +} + +func piExtensionBlocked(blocker, path string) error { + return fmt.Errorf( + "Pi loads only %s from %s, so it would never load %s; list %s in package.json pi.extensions or remove %s", + filepath.Base(blocker), filepath.Dir(path), filepath.Base(path), filepath.Base(path), blocker, + ) +} + +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/agenthook/profile.go b/agenthook/profile.go index 07a478b0..9e93c64e 100644 --- a/agenthook/profile.go +++ b/agenthook/profile.go @@ -115,6 +115,7 @@ type profileSpec struct { sessionSourceRequirement inputRequirement sessionEndReasonRequirement inputRequirement script string + checkScriptLoads func(path string) error } var profileOrder = []Agent{ diff --git a/agenthook/script.go b/agenthook/script.go index 36ba9e54..f12ed837 100644 --- a/agenthook/script.go +++ b/agenthook/script.go @@ -39,6 +39,11 @@ func planScriptConfig( if !exists && uninstall { return nil, false, nil } + if !uninstall && spec.checkScriptLoads != nil { + if err := spec.checkScriptLoads(path); err != nil { + return nil, false, err + } + } root := map[string]any{} if exists { block, err := scriptBlock(existing, path) From 43921050ed0dab40cc405869386d59b1dc6a6c26 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 13:07:24 -0400 Subject: [PATCH 10/13] fix(agenthook): run Pi hooks in the session's directory --- agenthook/agenthook_test.go | 18 +++++++++++++++++- agenthook/config.go | 2 +- agenthook/pi.go | 4 ++++ agenthook/script_runtime.js | 5 +++-- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index 00ef560f..d67ff487 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1115,6 +1115,11 @@ func TestInstallPiRefusesDirectoryThatSkipsTheModule(t *testing.T) { files: map[string]string{"package.json": `{"pi":{"extensions":["main.js"]}}`, "main.js": ""}, blocker: "package.json", }, + { + name: "manifest with a byte order mark", + files: map[string]string{"package.json": "\ufeff" + `{"pi":{"extensions":["main.js"]}}`, "main.js": ""}, + blocker: "package.json", + }, { name: "manifest listing the module before it exists", files: map[string]string{"package.json": `{"pi":{"extensions":["agenthook.js"]}}`, "index.js": ""}, @@ -1153,6 +1158,12 @@ func TestPiExtensionHelper(t *testing.T) { } 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')) @@ -1180,7 +1191,7 @@ const runtime = () => { }; const ctx = (id, file, mode = "tui") => ({ mode, - cwd: "/work", + cwd: process.argv[3], sessionManager: { getSessionId: () => id, getSessionFile: () => file }, }); const fire = async (pi, name, event, context) => { @@ -1282,11 +1293,16 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { Transcript string `json:"transcript_path"` Reason string `json:"reason"` Prompt string `json:"prompt"` + Cwd string `json:"cwd"` + HelperCwd string `json:"helper_cwd"` } 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) + assert.Equal(r.Cwd, r.HelperCwd, "the hook ran outside the session's directory") + r.Cwd, r.HelperCwd = "", "" reports = append(reports, r) } fileA := filepath.Join(dir, "a.jsonl") diff --git a/agenthook/config.go b/agenthook/config.go index 0205034c..e27ec1a3 100644 --- a/agenthook/config.go +++ b/agenthook/config.go @@ -212,7 +212,7 @@ 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") } - // Check the caller's matcher: Bash translates to Pi's empty shell tool. + // 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) } diff --git a/agenthook/pi.go b/agenthook/pi.go index 7e40eb0c..4be63f66 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -1,6 +1,7 @@ package agenthook import ( + "bytes" _ "embed" "encoding/json" "fmt" @@ -157,6 +158,9 @@ func piExtensionLoads(path string) error { Extensions []string `json:"extensions"` } `json:"pi"` } + // Pi strips a UTF-8 BOM before parsing: + // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/pi-manifest.ts#L19 + data = bytes.TrimPrefix(data, []byte("\xef\xbb\xbf")) if json.Unmarshal(data, &manifest) == nil { listed := false for _, entry := range manifest.Pi.Extensions { diff --git a/agenthook/script_runtime.js b/agenthook/script_runtime.js index 09b61268..4b5d05a9 100644 --- a/agenthook/script_runtime.js +++ b/agenthook/script_runtime.js @@ -2,7 +2,7 @@ import { spawn } from "node:child_process"; const defaultTimeoutSeconds = 60; -// Runs one argv without a shell, writing the payload to stdin, and resolves to +// 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. @@ -10,7 +10,8 @@ function runCommand(handler, payload) { return new Promise((resolve) => { let child; try { - child = spawn(handler.command, handler.args, { stdio: ["pipe", "ignore", "ignore"] }); + // 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; From 59f9cacbaaee969543fc69d0092df9dad21dcd18 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 13:38:06 -0400 Subject: [PATCH 11/13] fix(agenthook): stop refusing Pi extensions directories so other profiles still install --- agenthook/agenthook_test.go | 49 ------------------------------- agenthook/doc.go | 5 ++-- agenthook/pi.go | 58 ------------------------------------- agenthook/profile.go | 1 - agenthook/script.go | 5 ---- 5 files changed, 2 insertions(+), 116 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index d67ff487..ef6852c6 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1102,55 +1102,6 @@ func TestPlanInstallPiRequiresExecutableWithoutMatchers(t *testing.T) { require.ErrorContains(t, err, "do not support matchers") } -func TestInstallPiRefusesDirectoryThatSkipsTheModule(t *testing.T) { - tests := []struct { - name string - files map[string]string - blocker string - }{ - {name: "index.js", files: map[string]string{"index.js": ""}, blocker: "index.js"}, - {name: "index.ts", files: map[string]string{"index.ts": ""}, blocker: "index.ts"}, - { - name: "manifest without the module", - files: map[string]string{"package.json": `{"pi":{"extensions":["main.js"]}}`, "main.js": ""}, - blocker: "package.json", - }, - { - name: "manifest with a byte order mark", - files: map[string]string{"package.json": "\ufeff" + `{"pi":{"extensions":["main.js"]}}`, "main.js": ""}, - blocker: "package.json", - }, - { - name: "manifest listing the module before it exists", - files: map[string]string{"package.json": `{"pi":{"extensions":["agenthook.js"]}}`, "index.js": ""}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - dir := t.TempDir() - for name, content := range tt.files { - require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600)) - } - path := filepath.Join(dir, "agenthook.js") - - _, err := Install(AgentPi, InstallOptions{ - ConfigPath: path, - Executable: "/opt/hook", - Arguments: []string{"--source", "shared-agent-hook-test"}, - Marker: testMarker, - }) - - if tt.blocker == "" { - require.NoError(t, err) - assert.FileExists(t, path) - return - } - require.ErrorContains(t, err, "Pi loads only "+tt.blocker) - assert.NoFileExists(t, path) - }) - } -} - func TestPiExtensionHelper(t *testing.T) { out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") if out == "" { diff --git a/agenthook/doc.go b/agenthook/doc.go index 7c1f8784..5a3e7b72 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -14,9 +14,8 @@ // 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. Install refuses an extensions directory -// with a root index.ts or index.js, or a package.json pi.extensions list that -// leaves the module out, since Pi would then never load it. +// --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 diff --git a/agenthook/pi.go b/agenthook/pi.go index 4be63f66..26a74ed4 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -1,7 +1,6 @@ package agenthook import ( - "bytes" _ "embed" "encoding/json" "fmt" @@ -37,7 +36,6 @@ func piProfile() profileSpec { spec.configEnvDir = piAgentDir spec.eventName = piEventName spec.script = piExtension - spec.checkScriptLoads = piExtensionLoads // Pi extension handlers run in-process; the generated extension ignores // command output, so control decisions have nowhere to go. spec.responseFormat = responseObservational @@ -143,59 +141,3 @@ func promotePiReason(payload map[string]json.RawMessage) error { payload[field] = encoded return nil } - -// piExtensionLoads refuses an extensions directory where Pi would load only a -// package.json pi.extensions list or a root index.ts or index.js, which would -// leave the generated module silently unloaded. A manifest entry counts when -// its file exists or it names the module about to be written: -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/package-manager.ts#L563-L601 -func piExtensionLoads(path string) error { - dir := filepath.Dir(path) - manifestPath := filepath.Join(dir, "package.json") - if data, err := os.ReadFile(manifestPath); err == nil { - var manifest struct { - Pi struct { - Extensions []string `json:"extensions"` - } `json:"pi"` - } - // Pi strips a UTF-8 BOM before parsing: - // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/pi-manifest.ts#L19 - data = bytes.TrimPrefix(data, []byte("\xef\xbb\xbf")) - if json.Unmarshal(data, &manifest) == nil { - listed := false - for _, entry := range manifest.Pi.Extensions { - resolved := entry - if !filepath.IsAbs(resolved) { - resolved = filepath.Join(dir, entry) - } - if filepath.Clean(resolved) == filepath.Clean(path) { - return nil - } - if _, err := os.Stat(resolved); err == nil { - listed = true - } - } - if listed { - return piExtensionBlocked(manifestPath, path) - } - } - } - for _, name := range []string{"index.ts", "index.js"} { - if index := filepath.Join(dir, name); fileExists(index) { - return piExtensionBlocked(index, path) - } - } - return nil -} - -func piExtensionBlocked(blocker, path string) error { - return fmt.Errorf( - "Pi loads only %s from %s, so it would never load %s; list %s in package.json pi.extensions or remove %s", - filepath.Base(blocker), filepath.Dir(path), filepath.Base(path), filepath.Base(path), blocker, - ) -} - -func fileExists(path string) bool { - _, err := os.Stat(path) - return err == nil -} diff --git a/agenthook/profile.go b/agenthook/profile.go index 9e93c64e..07a478b0 100644 --- a/agenthook/profile.go +++ b/agenthook/profile.go @@ -115,7 +115,6 @@ type profileSpec struct { sessionSourceRequirement inputRequirement sessionEndReasonRequirement inputRequirement script string - checkScriptLoads func(path string) error } var profileOrder = []Agent{ diff --git a/agenthook/script.go b/agenthook/script.go index f12ed837..36ba9e54 100644 --- a/agenthook/script.go +++ b/agenthook/script.go @@ -39,11 +39,6 @@ func planScriptConfig( if !exists && uninstall { return nil, false, nil } - if !uninstall && spec.checkScriptLoads != nil { - if err := spec.checkScriptLoads(path); err != nil { - return nil, false, err - } - } root := map[string]any{} if exists { block, err := scriptBlock(existing, path) From 33bcfd23ff8ab823ded6745a565f576fb30861ff Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 13:50:34 -0400 Subject: [PATCH 12/13] test(agenthook): compare Pi hook directories after resolving symlinks --- agenthook/agenthook_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index ef6852c6..c587cf78 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1247,12 +1247,17 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { 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) - assert.Equal(r.Cwd, r.HelperCwd, "the hook ran outside the session's directory") + 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) } From 01221045721fa91a293be8dd0cc00035dc23cbf3 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 14:12:45 -0400 Subject: [PATCH 13/13] test(agenthook): bound the Pi kill check by the helper's sleep, not runner speed --- agenthook/agenthook_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index c587cf78..3f9e3fc8 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1124,7 +1124,7 @@ func TestPiExtensionHelper(t *testing.T) { // 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(time.Minute) + <-time.After(4 * time.Minute) } } @@ -1235,7 +1235,9 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { output, err := cmd.CombinedOutput() require.NoError(err, string(output)) - assert.Less(time.Since(started), 30*time.Second, "the timed-out command was not killed") + // 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 {