From 5724ef3bc488d0aab13c6abab6c82db625d6f77a Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 6 Oct 2026 10:35:12 -0400 Subject: [PATCH 01/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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 { From a426d782da243faae452282a2c9278bb25f88a76 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 10 Oct 2026 15:21:44 -0500 Subject: [PATCH 14/15] fix(agenthook): report Pi resumes and hook stderr; remove unused module A host that reopens a Pi session with pi --session needs to know the session was resumed. Pi reports startup for every first launch, even when it opens a saved session. Claude Code reports resume in the same case, so hosts got a different answer from Pi. A saved session file at startup means Pi opened an existing session, so the module now reports resume. A failed hook showed only its exit status in Pi. The command's own error text was lost, so a user had nothing to act on. Failures now end with the tail of the command's stderr. Reading stderr adds a risk: a background process that the hook starts can inherit the pipe and keep it open. The module stops waiting shortly after the command exits, and keeps reading the pipe so that process never blocks and Pi is not held open. After the last application uninstalled, Pi kept loading an empty module on every start. The file belongs only to kit, so Uninstall now deletes it. Result.Data is nil in that case, so callers that dump the planned config can tell a deletion from a rewrite. PI_CODING_AGENT_DIR also accepts file URLs, as Pi's own path handling does. Kit used to treat such a URL as a relative path and installed the module where Pi never looks. The Pi source links now point to a fixed commit, so their line ranges stay correct as Pi changes. Generated with Claude Code (claude-opus-5-5) Co-Authored-By: Claude Opus 5.5 --- agenthook/AGENTS.md | 5 +- agenthook/agenthook_test.go | 95 ++++++++++++++++++++++++++++++++----- agenthook/config.go | 17 +++++-- agenthook/doc.go | 6 ++- agenthook/pi.go | 52 +++++++++++++++++--- agenthook/pi_extension.js | 26 +++++----- agenthook/script.go | 7 ++- agenthook/script_runtime.js | 45 ++++++++++++++---- 8 files changed, 208 insertions(+), 45 deletions(-) diff --git a/agenthook/AGENTS.md b/agenthook/AGENTS.md index da05767e..f1011d96 100644 --- a/agenthook/AGENTS.md +++ b/agenthook/AGENTS.md @@ -32,7 +32,10 @@ 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. + on every write, delete the module when uninstall removes its last hook, and + spawn registered argv without a shell on every OS. The runtime stops waiting + for a command shortly after it exits even when a process it started still + holds stderr, and keeps draining that pipe without holding the harness open. The Pi module reports only from Pi's interactive terminal (`ctx.mode` is `tui`), so print, json, and rpc runs, subagents included, stay silent. It needs Pi 0.80.4 or later (`ctx.mode` 0.78.1, `agent_settled` 0.80.4). diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index 3f9e3fc8..f34df428 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -1,7 +1,9 @@ package agenthook import ( + "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -9,6 +11,7 @@ import ( "os/signal" "path/filepath" "runtime" + "strconv" "strings" "syscall" "testing" @@ -961,8 +964,9 @@ func TestConfigPathNormalizesPiAgentDirAsPiDoes(t *testing.T) { return elsewhere } tests := []struct { - env string - want string + env string + want string + fails bool }{ {env: "~", want: home}, {env: "~/pi-agent", want: filepath.Join(home, "pi-agent")}, @@ -971,6 +975,14 @@ 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"}, + // File URLs resolve as Node's fileURLToPath does, and fail where it throws. + { + env: "file:///C:/Users/me/pi%20agent", + want: pick(`C:\Users\me\pi agent`, "/C:/Users/me/pi agent"), + }, + {env: "file://localhost/srv/pi", want: "/srv/pi", fails: windows}, + {env: "file://server/share/pi", want: `\\server\share\pi`, fails: !windows}, + {env: "file:///srv/pi%2Fagent", fails: true}, } for _, tt := range tests { t.Run(tt.env, func(t *testing.T) { @@ -978,6 +990,10 @@ func TestConfigPathNormalizesPiAgentDirAsPiDoes(t *testing.T) { path, err := ConfigPath(AgentPi) + if tt.fails { + require.ErrorContains(t, err, "file URL") + return + } require.NoError(t, err) assert.Equal(t, filepath.Join(tt.want, "extensions", "agenthook.js"), path) }) @@ -1054,10 +1070,18 @@ func TestInstallPiKeepsOtherApplicationsCommands(t *testing.T) { assert.True(result.Changed) assert.Equal([]string{bCommand}, piCommands(piScriptHooks(t, path), "session_start")) + // Removing the last hook deletes the module so Pi stops loading it; a plan + // reports that without touching the file. + result, err = PlanUninstall(AgentPi, path, "--source b-hook") + require.NoError(err) + assert.True(result.Changed) + assert.Nil(result.Data) + assert.FileExists(path) + result, err = Uninstall(AgentPi, path, "--source b-hook") require.NoError(err) assert.True(result.Changed) - assert.Empty(piScriptHooks(t, path)) + assert.NoFileExists(path) result, err = Uninstall(AgentPi, filepath.Join(t.TempDir(), "missing.js"), "--source b-hook") require.NoError(err) @@ -1103,6 +1127,11 @@ func TestPlanInstallPiRequiresExecutableWithoutMatchers(t *testing.T) { } func TestPiExtensionHelper(t *testing.T) { + if os.Getenv("KIT_AGENTHOOK_PI_HELPER_LINGER") != "" { + // A background process the hook started, holding the hook's stderr open. + <-time.After(4 * time.Minute) + return + } out := os.Getenv("KIT_AGENTHOOK_PI_HELPER_OUT") if out == "" { return @@ -1120,9 +1149,25 @@ func TestPiExtensionHelper(t *testing.T) { _, err = file.Write(append(payload, '\n')) require.NoError(t, err) require.NoError(t, file.Close()) - if strings.Contains(string(payload), "agent_settled") { + switch fields["hook_event_name"] { + case "before_agent_start": + // Exit at once, leaving a process that inherits stderr. It must outlive + // this test, so it gets no cancellation; the test kills it by its PID, + // and it runs outside the test's directory so Windows can remove that. + linger := exec.CommandContext( + context.WithoutCancel(t.Context()), os.Args[0], "-test.run=^TestPiExtensionHelper$", + ) + linger.Env = append(os.Environ(), "KIT_AGENTHOOK_PI_HELPER_LINGER=1") + linger.Dir = os.TempDir() + linger.Stderr = os.Stderr + require.NoError(t, linger.Start()) + pid := []byte(strconv.Itoa(linger.Process.Pid)) + require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(out), "linger.pid"), pid, 0o600)) + require.NoError(t, linger.Process.Release()) + case "agent_settled": // Ignore SIGTERM and outlive the 1s hook timeout, so only a forced kill // with an unconditional deadline keeps the extension from waiting. + fmt.Fprintln(os.Stderr, "helper still settling") signal.Ignore(syscall.SIGTERM) <-time.After(4 * time.Minute) } @@ -1172,7 +1217,7 @@ await fire(pi, "session_start", { reason: "startup" }, ctx("a", fileA)); await fire(pi, "before_agent_start", { prompt: "one" }, ctx("a", fileA)); nothingSent("before the session file existed"); // The header line Pi's SessionManager writes first: -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/session-manager.ts#L1061-L1071 +// https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/session-manager.ts#L1063-L1070 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)); @@ -1188,6 +1233,10 @@ 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)); + +// pi --session: Pi reports startup for a saved session, which resumes it. +pi = runtime(); +await fire(pi, "session_start", { reason: "startup" }, ctx("a", fileA)); ` func TestPiExtensionReportsResumableSessions(t *testing.T) { @@ -1215,7 +1264,8 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { Marker: testMarker, Hooks: []Hook{ {Event: EventSessionStart}, - {Event: EventUserPromptSubmit}, + // Shorter than the background process's life, so waiting for stderr to close fails. + {Event: EventUserPromptSubmit, Timeout: 2 * time.Second}, {Event: EventSessionEnd}, {Event: EventStop, Timeout: time.Second}, }, @@ -1230,14 +1280,33 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { out := filepath.Join(dir, "payloads.jsonl") cmd := exec.CommandContext(t.Context(), node, driver, module, dir) cmd.Env = append(os.Environ(), "KIT_AGENTHOOK_PI_HELPER_OUT="+out) + t.Cleanup(func() { + pid, err := os.ReadFile(filepath.Join(dir, "linger.pid")) + if errors.Is(err, os.ErrNotExist) { + return + } + require.NoError(err) + id, err := strconv.Atoi(string(pid)) + require.NoError(err) + linger, err := os.FindProcess(id) + require.NoError(err) + if err := linger.Kill(); !errors.Is(err, os.ErrProcessDone) { + assert.NoError(err) + } + }) started := time.Now() output, err := cmd.CombinedOutput() require.NoError(err, string(output)) - // Node waits for a live child, so finishing well before the helper's sleep - // ends proves the kill; a loaded runner can spend tens of seconds on re-execs. - assert.Less(time.Since(started), 3*time.Minute, "the timed-out command was not killed") + // Node waits for a live child or a referenced pipe, so finishing well before + // the helpers' sleeps end proves the kill and that a background process + // holding stderr does not keep Pi waiting; a loaded runner can spend tens of + // seconds on re-execs. + assert.Less( + time.Since(started), 3*time.Minute, + "a hook process or its stderr kept the extension waiting", + ) payloads, err := os.ReadFile(out) require.NoError(err) type report struct { @@ -1273,14 +1342,18 @@ func TestPiExtensionReportsResumableSessions(t *testing.T) { {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"}, + {Event: "session_start", SessionID: "a", Transcript: fileA, Reason: "resume"}, }, reports) // A failed command is reported once its event's other commands have run. failed := "agenthook session_start commands failed: " + missing + " --source missing-hook: could not start" failures := strings.Split(strings.TrimSpace(string(output)), "\n") - require.Len(failures, 3, string(output)) + require.Len(failures, 4, string(output)) assert.Contains(failures[0], "context: "+failed) assert.Contains(failures[1], "agent_settled: agenthook agent_settled commands failed: ") - assert.Contains(failures[1], "timed out after 1s") + assert.Contains(failures[1], "timed out after 1s: helper still settling") assert.Contains(failures[2], "session_start: "+failed) + assert.Contains(failures[3], "session_start: "+failed) + // The prompt's command exited at once, though a process it started still holds stderr. + assert.NotContains(string(output), "before_agent_start commands failed") assert.NotContains(string(output), "TestPiExtensionHelper$ -- --source shared-agent-hook-test: could not start") } diff --git a/agenthook/config.go b/agenthook/config.go index e27ec1a3..e67cac9d 100644 --- a/agenthook/config.go +++ b/agenthook/config.go @@ -32,7 +32,9 @@ type Hook struct { // changes. On Windows, Claude Code hooks built from Executable are written in // exec form, which needs an executable such as an .exe rather than a .cmd or // .bat shim, and the marker matches the executable and arguments joined by -// spaces. Hooks defaults to every event supported by the selected profile. +// spaces. Pi hooks always use exec form on every OS: they require Executable, +// reject raw commands and matchers, and match the marker the same way. Hooks +// defaults to every event supported by the selected profile. type InstallOptions struct { ConfigPath string Executable string @@ -46,6 +48,8 @@ type InstallOptions struct { // Result reports the planned or completed config mutation. Data contains the // complete resulting config and can be used by command-line dump operations. +// Data is nil when an uninstall removes a kit-owned config file, such as Pi's +// extension module once no application's hooks remain. type Result struct { Agent Agent ConfigPath string @@ -152,13 +156,20 @@ func PlanUninstall(agent Agent, configPath, marker string) (Result, error) { } // Uninstall removes every command containing marker from the selected agent -// config while preserving hooks owned by other applications. Callers must -// serialize concurrent mutations of the same config path. +// config while preserving hooks owned by other applications. It deletes a +// kit-owned config file, such as Pi's extension module, once no hooks remain. +// Callers must serialize concurrent mutations of the same config path. func Uninstall(agent Agent, configPath, marker string) (Result, error) { result, err := PlanUninstall(agent, configPath, marker) if err != nil || !result.Changed { return result, err } + if result.Data == nil { + if err := os.Remove(result.ConfigPath); err != nil && !errors.Is(err, os.ErrNotExist) { + return Result{}, fmt.Errorf("remove agent hook config %s: %w", result.ConfigPath, err) + } + return result, nil + } if err := writeConfig(result.ConfigPath, result.Data); err != nil { if errors.Is(err, atomicfile.ErrPublished) { return result, err diff --git a/agenthook/doc.go b/agenthook/doc.go index 5a3e7b72..d733bf24 100644 --- a/agenthook/doc.go +++ b/agenthook/doc.go @@ -14,7 +14,11 @@ // 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 +// --no-session run reports nothing. Pi reports startup when it opens a saved +// session (pi --session or --continue), so the module reports that SessionStart +// with source resume. A failed command's error ends with the tail of its stderr. +// Uninstall deletes the module once no application's hooks remain. 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. // diff --git a/agenthook/pi.go b/agenthook/pi.go index 26a74ed4..4e026cc4 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -4,6 +4,7 @@ import ( _ "embed" "encoding/json" "fmt" + "net/url" "os" "path/filepath" "regexp" @@ -21,7 +22,7 @@ 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/earendil-works/pi/blob/main/packages/coding-agent/src/core/package-manager.ts#L603-L640 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/package-manager.ts#L592-L645 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. @@ -41,16 +42,16 @@ func piProfile() profileSpec { spec.responseFormat = responseObservational // session_start carries a reason that maps to source except for reload, and // the extension reports session_shutdown only for reasons that map to one: - // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L733-L741 - // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L802-L808 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/extensions/types.ts#L741-L747 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/extensions/types.ts#L810-L815 spec.sessionSourceRequirement = inputOptional spec.sessionEndReasonRequirement = inputRequired return spec } // piEventName maps Claude events to Pi extension events: -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L911-L922 -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/types.ts#L997-L1000 +// https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/extensions/types.ts#L919-L929 +// https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/extensions/types.ts#L1005-L1010 func piEventName(event Event) string { switch event { case EventSessionStart: @@ -70,8 +71,8 @@ func piEventName(event Event) string { var piWindowsShellPath = regexp.MustCompile(`(?i)^/(?:mnt/|cygdrive/)?([a-z])(?:/(.*))?$`) // piAgentDir follows Pi's normalizePath for PI_CODING_AGENT_DIR: Git Bash, -// MSYS, Cygwin, and WSL drive paths on Windows, then ~: -// https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/utils/paths.ts#L67-L101 +// MSYS, Cygwin, and WSL drive paths on Windows, then ~, then file URLs: +// https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/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, `\`) { @@ -91,9 +92,46 @@ func piAgentDir(dir string) (string, error) { } return filepath.Join(home, rest), nil } + if strings.HasPrefix(dir, "file://") { + return piFileURLPath(dir) + } return dir, nil } +// piFileURLPath converts a file URL as Node's fileURLToPath does, failing +// where it throws, since Pi then cannot start: +// https://github.com/nodejs/node/blob/v24.0.0/lib/internal/url.js#L1450-L1512 +func piFileURLPath(raw string) (string, error) { + parsed, err := url.Parse(raw) + if err != nil { + return "", fmt.Errorf("parse file URL %q: %w", raw, err) + } + escaped := strings.ToLower(parsed.EscapedPath()) + if strings.Contains(escaped, "%2f") || + (runtime.GOOS == "windows" && strings.Contains(escaped, "%5c")) { + return "", fmt.Errorf("file URL %q must not encode path separators", raw) + } + // The WHATWG URL parser Node uses drops a file URL's localhost host. + host := parsed.Host + if host == "localhost" { + host = "" + } + if runtime.GOOS != "windows" { + if host != "" { + return "", fmt.Errorf("file URL %q names host %q; it must name a local path", raw, host) + } + return parsed.Path, nil + } + path := filepath.FromSlash(parsed.Path) + if host != "" { + return `\\` + host + path, nil + } + if len(path) < 3 || path[2] != ':' || path[1]|0x20 < 'a' || path[1]|0x20 > 'z' { + return "", fmt.Errorf("file URL %q must name an absolute path with a drive letter", raw) + } + return path[1:], 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 diff --git a/agenthook/pi_extension.js b/agenthook/pi_extension.js index c5db5219..9b88dc36 100644 --- a/agenthook/pi_extension.js +++ b/agenthook/pi_extension.js @@ -1,10 +1,10 @@ import { existsSync } from "node:fs"; -// Pi extension API: https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/extensions.md +// Pi extension API: https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/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 +// https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/extensions/runner.ts#L1089-L1117 +// https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/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 @@ -13,7 +13,7 @@ export default function (pi) { // Events wait until the session file exists, so every reported ID resumes // with pi --session. Pi writes the file once the session holds a user // message, and never with --no-session: - // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/session-manager.ts#L1160-L1185 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/session-manager.ts#L1160-L1189 let heldStart = null; const heldPrompts = []; @@ -48,7 +48,7 @@ export default function (pi) { // 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 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/extensions/types.ts#L323-L335 const on = (name, handler) => pi.on(name, async (event, ctx) => { if (ctx.mode !== "tui") return; @@ -57,15 +57,19 @@ export default function (pi) { if (failures.length > 0) throw new Error(failures.join("; ")); }); + // Pi reports startup even when it opens a saved session (pi --session or + // --continue); a session file that already exists means a resume: + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/agent-session.ts#L509 on("session_start", async (event, ctx, failures) => { if (!persisting(ctx)) return; - heldStart = payloadFor("session_start", ctx, { reason: event.reason }); + const reason = event.reason === "startup" && saved(ctx) ? "resume" : event.reason; + heldStart = payloadFor("session_start", ctx, { reason }); await flush(ctx, failures); }); // before_agent_start runs before Pi appends the turn's user message, so a // first prompt waits for the next event: - // https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/agent-session.ts#L2060-L2109 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/agent-session.ts#L2059-L2111 on("before_agent_start", async (event, ctx, failures) => { if (!persisting(ctx)) return; if (typeof event.prompt === "string" && event.prompt !== "") { @@ -77,10 +81,10 @@ export default function (pi) { // 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 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/agent-session.ts#L1140-L1162 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/agent/src/agent-loop.ts#L117-L124 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/agent/src/agent-loop.ts#L388-L391 + // https://github.com/earendil-works/pi/blob/e37cdc420669e48b4df4bbce0fb2617fc3c71328/packages/coding-agent/src/core/sdk.ts#L436-L440 on("context", async (_event, ctx, failures) => { if (heldStart || heldPrompts.length > 0) await flush(ctx, failures); }); diff --git a/agenthook/script.go b/agenthook/script.go index 36ba9e54..453a03e1 100644 --- a/agenthook/script.go +++ b/agenthook/script.go @@ -23,7 +23,9 @@ const ( // 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. +// on every write, so a reinstall also upgrades them. An uninstall that removes +// the last hook returns nil data so the caller deletes the module, which the +// harness would otherwise keep loading. func planScriptConfig( spec profileSpec, path, marker string, @@ -52,6 +54,9 @@ func planScriptConfig( if err := applyNestedJSONHooks(root, path, marker, "", "", argv, hooks, uninstall); err != nil { return nil, false, err } + if remaining, _ := root["hooks"].(map[string]any); uninstall && len(remaining) == 0 { + return nil, true, nil + } encoded, err := marshalJSONConfig(root) if err != nil { return nil, false, fmt.Errorf("encode agent hook config %s: %w", path, err) diff --git a/agenthook/script_runtime.js b/agenthook/script_runtime.js index 4b5d05a9..c9865b51 100644 --- a/agenthook/script_runtime.js +++ b/agenthook/script_runtime.js @@ -1,35 +1,60 @@ import { spawn } from "node:child_process"; const defaultTimeoutSeconds = 60; +// A failure quotes at most this many characters from the end of the command's stderr. +const stderrTailLength = 2000; +// A background process the command starts can inherit stderr and hold it open, +// so the wait ends this long after the command itself exits. +const stderrGraceMs = 250; -// 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. +const exitFailure = (code, signal) => { + if (signal) return `killed by ${signal}`; + return code === 0 ? null : `exited with status ${code}`; +}; + +// Runs one argv without a shell in the payload's directory, writing the payload +// to stdin, and resolves to a failure description, ending in the tail of the +// command's stderr, or null. At its timeout the child is force-killed and the +// wait ends even if it has not exited, so a child that ignores SIGTERM cannot +// block later commands or the harness. function runCommand(handler, payload) { return new Promise((resolve) => { let child; try { // Pi changes ctx.cwd on a cross-project resume without changing the process directory. - child = spawn(handler.command, handler.args, { cwd: payload.cwd, stdio: ["pipe", "ignore", "ignore"] }); + child = spawn(handler.command, handler.args, { cwd: payload.cwd, stdio: ["pipe", "ignore", "pipe"] }); } catch (error) { resolve(`could not start: ${error.message}`); return; } + let stderr = ""; + let done = false; + let grace; const seconds = handler.timeout > 0 ? handler.timeout : defaultTimeoutSeconds; const timer = setTimeout(() => { child.kill("SIGKILL"); - resolve(`timed out after ${seconds}s`); + finish(`timed out after ${seconds}s`); }, seconds * 1000); const finish = (failure) => { + if (done) return; + done = true; clearTimeout(timer); - resolve(failure); + clearTimeout(grace); + // Keep draining stderr so a process still writing to it never blocks, + // without holding the harness open. + child.stderr.unref(); + const detail = stderr.trim(); + resolve(failure && detail ? `${failure}: ${detail}` : failure); }; + child.stderr.setEncoding("utf8"); + child.stderr.on("data", (chunk) => { + if (!done) stderr = (stderr + chunk).slice(-stderrTailLength); + }); child.once("error", (error) => finish(`could not start: ${error.message}`)); - child.once("close", (code, signal) => { - if (signal) finish(`killed by ${signal}`); - else finish(code === 0 ? null : `exited with status ${code}`); + child.once("exit", (code, signal) => { + grace = setTimeout(() => finish(exitFailure(code, signal)), stderrGraceMs); }); + child.once("close", (code, signal) => finish(exitFailure(code, signal))); child.stdin.on("error", () => {}); child.stdin.end(JSON.stringify(payload)); }); From e7900cc9b9fc8f6e27a6f0a40b05050a09dcd4f0 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 10 Oct 2026 16:18:55 -0500 Subject: [PATCH 15/15] fix(agenthook): keep Pi hooks that exit in time from timing out A hook that finished just before its timeout could still be reported as timed out. This happened when a background process it started kept its stderr open: the deadline stayed armed while the extension waited a short time for stderr to close. The deadline now stops when the command exits. No test covers this window. It needs a command to exit within 250ms of its deadline, and process start time on CI runners varies by more than that. Kit read some PI_CODING_AGENT_DIR file URLs differently from Pi. Node, which Pi runs on, lowercases the host, so file://LOCALHOST is a local path. Node also reads file://C:/ as drive C, where Go reads C: as a host. In those cases kit installed the module in a directory Pi never loads, or refused a value that Pi accepts. The Pi test no longer checks that each reported transcript exists. It ran after the test had created every saved file, so it could not catch an early report, and the exact report comparison already rejects any report for a session that was never saved. The helper's background process now starts in the test binary's directory instead of the system temp directory, which the usetesting lint rejects in tests. Generated with Claude Code (claude-opus-5-5) Co-Authored-By: Claude Opus 5.5 --- agenthook/agenthook_test.go | 14 ++++++++------ agenthook/pi.go | 10 +++++++--- agenthook/script_runtime.js | 2 ++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/agenthook/agenthook_test.go b/agenthook/agenthook_test.go index f34df428..6a1ab845 100644 --- a/agenthook/agenthook_test.go +++ b/agenthook/agenthook_test.go @@ -981,7 +981,11 @@ func TestConfigPathNormalizesPiAgentDirAsPiDoes(t *testing.T) { want: pick(`C:\Users\me\pi agent`, "/C:/Users/me/pi agent"), }, {env: "file://localhost/srv/pi", want: "/srv/pi", fails: windows}, + {env: "file://LOCALHOST/srv/pi", want: "/srv/pi", fails: windows}, + {env: "file://C:/Users/me/pi", want: pick(`C:\Users\me\pi`, "/C:/Users/me/pi")}, + {env: "file:///c|/Users/me/pi", want: pick(`c:\Users\me\pi`, "/c:/Users/me/pi")}, {env: "file://server/share/pi", want: `\\server\share\pi`, fails: !windows}, + {env: "file://SERVER/share/pi", want: `\\server\share\pi`, fails: !windows}, {env: "file:///srv/pi%2Fagent", fails: true}, } for _, tt := range tests { @@ -1152,13 +1156,14 @@ func TestPiExtensionHelper(t *testing.T) { switch fields["hook_event_name"] { case "before_agent_start": // Exit at once, leaving a process that inherits stderr. It must outlive - // this test, so it gets no cancellation; the test kills it by its PID, - // and it runs outside the test's directory so Windows can remove that. + // this test, so it gets no cancellation; the test kills it by its PID. + // It runs in the test binary's directory, outside the session's, so + // Windows can remove the session's directory. linger := exec.CommandContext( context.WithoutCancel(t.Context()), os.Args[0], "-test.run=^TestPiExtensionHelper$", ) linger.Env = append(os.Environ(), "KIT_AGENTHOOK_PI_HELPER_LINGER=1") - linger.Dir = os.TempDir() + linger.Dir = filepath.Dir(os.Args[0]) linger.Stderr = os.Stderr require.NoError(t, linger.Start()) pid := []byte(strconv.Itoa(linger.Process.Pid)) @@ -1333,9 +1338,6 @@ 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"}, diff --git a/agenthook/pi.go b/agenthook/pi.go index 4e026cc4..5a4e2ace 100644 --- a/agenthook/pi.go +++ b/agenthook/pi.go @@ -68,6 +68,8 @@ func piEventName(event Event) string { } } +var piFileURLDrive = regexp.MustCompile(`^file:///?([A-Za-z])[:|](/|$)`) + var piWindowsShellPath = regexp.MustCompile(`(?i)^/(?:mnt/|cygdrive/)?([a-z])(?:/(.*))?$`) // piAgentDir follows Pi's normalizePath for PI_CODING_AGENT_DIR: Git Bash, @@ -102,7 +104,9 @@ func piAgentDir(dir string) (string, error) { // where it throws, since Pi then cannot start: // https://github.com/nodejs/node/blob/v24.0.0/lib/internal/url.js#L1450-L1512 func piFileURLPath(raw string) (string, error) { - parsed, err := url.Parse(raw) + // The WHATWG URL parser Node uses reads a drive letter after file:// as the + // path's first segment and writes C| as C:, while Go reads C: as a host. + parsed, err := url.Parse(piFileURLDrive.ReplaceAllString(raw, "file:///$1:$2")) if err != nil { return "", fmt.Errorf("parse file URL %q: %w", raw, err) } @@ -111,8 +115,8 @@ func piFileURLPath(raw string) (string, error) { (runtime.GOOS == "windows" && strings.Contains(escaped, "%5c")) { return "", fmt.Errorf("file URL %q must not encode path separators", raw) } - // The WHATWG URL parser Node uses drops a file URL's localhost host. - host := parsed.Host + // WHATWG parsing also lowercases the host and drops localhost. + host := strings.ToLower(parsed.Host) if host == "localhost" { host = "" } diff --git a/agenthook/script_runtime.js b/agenthook/script_runtime.js index c9865b51..46007ef2 100644 --- a/agenthook/script_runtime.js +++ b/agenthook/script_runtime.js @@ -52,6 +52,8 @@ function runCommand(handler, payload) { }); child.once("error", (error) => finish(`could not start: ${error.message}`)); child.once("exit", (code, signal) => { + // The command finished in time, so only the stderr wait remains. + clearTimeout(timer); grace = setTimeout(() => finish(exitFailure(code, signal)), stderrGraceMs); }); child.once("close", (code, signal) => finish(exitFailure(code, signal)));