From bc41540261bd5543f0f6d595d28e1eae2d386010 Mon Sep 17 00:00:00 2001 From: berce <75228263+bercedev@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:50:46 +0300 Subject: [PATCH 1/4] feat(trace): skip Claude Code and Cursor hooks when the Chainloop CLI is missing The generated hook commands called chainloop directly, so opening a repository with the hooks installed but no CLI on PATH failed every hook with exit 127. They are now wrapped in a command -v guard that exits 0 when the CLI is missing and execs it otherwise, as the Codex hooks do. Re-running chainloop trace init upgrades unguarded entries. Signed-off-by: berce <75228263+bercedev@users.noreply.github.com> --- app/cli/internal/trace/claude/hooks.go | 19 +++- app/cli/internal/trace/claude/hooks_test.go | 98 ++++++++++++++++++++- app/cli/internal/trace/cursor/hooks.go | 4 +- app/cli/internal/trace/cursor/hooks_test.go | 92 ++++++++++++++++++- app/cli/internal/trace/provider.go | 8 ++ 5 files changed, 214 insertions(+), 7 deletions(-) diff --git a/app/cli/internal/trace/claude/hooks.go b/app/cli/internal/trace/claude/hooks.go index e30138e17..fd11292e0 100644 --- a/app/cli/internal/trace/claude/hooks.go +++ b/app/cli/internal/trace/claude/hooks.go @@ -241,10 +241,11 @@ func addHookEntry(hooks map[string]any, event, command, matcher string) bool { if !entryContainsChainloopHook(entry) { continue } - // Chainloop entry exists — check if it needs updating (e.g., matcher changed) + // Chainloop entry exists — check if it needs updating (e.g., matcher + // changed, or an entry written before the missing-CLI guard) m, _ := entry.(map[string]any) existingMatcher, _ := m["matcher"].(string) - if existingMatcher == matcher { + if existingMatcher == matcher && chainloopHookCommand(m) == trace.GuardHookCommand(command) { return false // already up to date } // Replace the entry with updated version @@ -258,12 +259,24 @@ func addHookEntry(hooks map[string]any, event, command, matcher string) bool { return true } +// chainloopHookCommand returns the command of the Chainloop handler in a hook group. +func chainloopHookCommand(entry map[string]any) string { + handlers, _ := entry["hooks"].([]any) + for _, h := range handlers { + handler, _ := h.(map[string]any) + if cmd, _ := handler["command"].(string); strings.Contains(cmd, hookMarker) { + return cmd + } + } + return "" +} + func buildHookEntry(command, matcher string) map[string]any { entry := map[string]any{ "hooks": []any{ map[string]any{ "type": "command", - "command": command, + "command": trace.GuardHookCommand(command), }, }, } diff --git a/app/cli/internal/trace/claude/hooks_test.go b/app/cli/internal/trace/claude/hooks_test.go index 3a7155f7e..32dd2d240 100644 --- a/app/cli/internal/trace/claude/hooks_test.go +++ b/app/cli/internal/trace/claude/hooks_test.go @@ -20,9 +20,13 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "path/filepath" + "runtime" + "strings" "testing" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -456,5 +460,97 @@ func assertHookCommand(t *testing.T, hooks map[string]any, event, expectedCmd st hook := innerHooks[0].(map[string]any) assert.Equal(t, "command", hook["type"]) - assert.Equal(t, expectedCmd, hook["command"]) + assert.Equal(t, trace.GuardHookCommand(expectedCmd), hook["command"]) +} + +// The hooks are committed to the repository, so they also run for anyone who +// opens it without the chainloop CLI installed (#3687). +func TestInstalledHooksWithoutCLI(t *testing.T) { + shell := shellOnlyPath(t) + root := t.TempDir() + require.NoError(t, New().InstallHooks(root)) + + for _, event := range hookEvents { + t.Run(event.event+"/"+event.command, func(t *testing.T) { + cmd := exec.CommandContext(t.Context(), shell.sh, "-c", trace.GuardHookCommand(event.command)) //nolint:gosec // run the installed hook to check its shell behaviour + cmd.Env = append(os.Environ(), "PATH="+shell.bin) + cmd.Stdin = strings.NewReader(`{"session_id":"test"}`) + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) + assert.Empty(t, out) + }) + } + assertInstalledCommandsGuarded(t, root) +} + +func TestInstalledHookPreservesIOAndExitStatus(t *testing.T) { + shell := shellOnlyPath(t) + stub := "#!/bin/sh\nIFS= read -r payload\nprintf '%s\\n%s\\n' \"$*\" \"$payload\"\nexit \"$HOOK_TEST_EXIT_CODE\"\n" + require.NoError(t, os.WriteFile(filepath.Join(shell.bin, "chainloop"), []byte(stub), 0700)) //nolint:gosec // executable stub CLI in a temporary test directory + + for _, code := range []string{"0", "37"} { + t.Run("exit_"+code, func(t *testing.T) { + cmd := exec.CommandContext(t.Context(), shell.sh, "-c", trace.GuardHookCommand("chainloop trace hook claude post-tool-use")) //nolint:gosec // run the installed hook against a local stub CLI + cmd.Env = append(os.Environ(), "PATH="+shell.bin, "HOOK_TEST_EXIT_CODE="+code) + cmd.Stdin = strings.NewReader("{\"session_id\":\"test\"}\n") + out, err := cmd.Output() + if code == "0" { + require.NoError(t, err) + } else { + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + assert.Equal(t, 37, exitErr.ExitCode()) + } + assert.Equal(t, "trace hook claude post-tool-use"+"\n{\"session_id\":\"test\"}\n", string(out)) + }) + } +} + +type shellPath struct{ sh, bin string } + +// shellOnlyPath returns a PATH directory holding only sh, so chainloop is missing. +func shellOnlyPath(t *testing.T) shellPath { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the guard is a POSIX sh command") + } + sh, err := exec.LookPath("sh") + require.NoError(t, err) + bin := t.TempDir() + require.NoError(t, os.Symlink(sh, filepath.Join(bin, "sh"))) + return shellPath{sh: sh, bin: bin} +} + +func assertInstalledCommandsGuarded(t *testing.T, root string) { + t.Helper() + data, err := os.ReadFile(filepath.Join(root, settingsFile)) + require.NoError(t, err) + assert.Equal(t, len(hookEvents), strings.Count(string(data), "command -v chainloop")) +} + +func TestInstallUpgradesUnguardedHooks(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, settingsFile) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700)) + legacy := `{"hooks":{"PostToolUse":[{"matcher":"` + postHookToolMatcher + `","hooks":[{"type":"command","command":"chainloop trace hook claude post-tool-use"}]},{"hooks":[{"type":"command","command":"echo user-hook"}]}]}}` + require.NoError(t, os.WriteFile(path, []byte(legacy), 0600)) + + require.NoError(t, New().InstallHooks(root)) + first, err := os.ReadFile(path) + require.NoError(t, err) + var settings struct { + Hooks map[string][]map[string]any `json:"hooks"` + } + require.NoError(t, json.Unmarshal(first, &settings)) + postToolUse := settings.Hooks[eventPostToolUse] + require.Len(t, postToolUse, 2, "the chainloop entry is replaced, not duplicated") + assert.Equal(t, trace.GuardHookCommand("chainloop trace hook claude post-tool-use"), chainloopHookCommand(postToolUse[0])) + assert.NotContains(t, string(first), `"command": "chainloop trace hook claude post-tool-use"`) + assert.Contains(t, string(first), "echo user-hook") + + // Idempotent once upgraded. + require.NoError(t, New().InstallHooks(root)) + second, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, string(first), string(second)) } diff --git a/app/cli/internal/trace/cursor/hooks.go b/app/cli/internal/trace/cursor/hooks.go index 2246bbef4..cdac42268 100644 --- a/app/cli/internal/trace/cursor/hooks.go +++ b/app/cli/internal/trace/cursor/hooks.go @@ -289,7 +289,7 @@ func addHookEntry(hooks map[string]any, h cursorHookEvent) bool { m, _ := entry.(map[string]any) existingCmd, _ := m["command"].(string) existingMatcher, _ := m["matcher"].(string) - if existingCmd == h.command && existingMatcher == h.matcher { + if existingCmd == trace.GuardHookCommand(h.command) && existingMatcher == h.matcher { return false } entries[i] = buildHookEntry(h) @@ -331,7 +331,7 @@ func removeHookEntry(hooks map[string]any, event string) bool { func buildHookEntry(h cursorHookEvent) map[string]any { entry := map[string]any{ - "command": h.command, + "command": trace.GuardHookCommand(h.command), "timeout": defaultHookTimeoutSeconds, } if h.matcher != "" { diff --git a/app/cli/internal/trace/cursor/hooks_test.go b/app/cli/internal/trace/cursor/hooks_test.go index 80bb4afad..95b9c7fa9 100644 --- a/app/cli/internal/trace/cursor/hooks_test.go +++ b/app/cli/internal/trace/cursor/hooks_test.go @@ -19,9 +19,13 @@ import ( "bytes" "encoding/json" "os" + "os/exec" "path/filepath" + "runtime" + "strings" "testing" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -275,7 +279,7 @@ func TestInstallHooksShellToolHooks(t *testing.T) { entries, _ := hooks[event].([]any) require.Len(t, entries, 1, "entries for %s", event) entry, _ := entries[0].(map[string]any) - assert.Equal(t, command, entry["command"], "command for %s", event) + assert.Equal(t, trace.GuardHookCommand(command), entry["command"], "command for %s", event) assert.Equal(t, shellToolName, entry["matcher"], "matcher for %s", event) } @@ -382,3 +386,89 @@ func readInstalledHooks(t *testing.T, repoRoot string) map[string]any { return hooks } + +// The hooks are committed to the repository, so they also run for anyone who +// opens it without the chainloop CLI installed (#3687). +func TestInstalledHooksWithoutCLI(t *testing.T) { + shell := shellOnlyPath(t) + root := t.TempDir() + require.NoError(t, New().InstallHooks(root)) + + for _, event := range cursorHookEvents { + t.Run(event.event+"/"+event.command, func(t *testing.T) { + cmd := exec.CommandContext(t.Context(), shell.sh, "-c", trace.GuardHookCommand(event.command)) //nolint:gosec // run the installed hook to check its shell behaviour + cmd.Env = append(os.Environ(), "PATH="+shell.bin) + cmd.Stdin = strings.NewReader(`{"session_id":"test"}`) + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) + assert.Empty(t, out) + }) + } + assertInstalledCommandsGuarded(t, root) +} + +func TestInstalledHookPreservesIOAndExitStatus(t *testing.T) { + shell := shellOnlyPath(t) + stub := "#!/bin/sh\nIFS= read -r payload\nprintf '%s\\n%s\\n' \"$*\" \"$payload\"\nexit \"$HOOK_TEST_EXIT_CODE\"\n" + require.NoError(t, os.WriteFile(filepath.Join(shell.bin, "chainloop"), []byte(stub), 0700)) //nolint:gosec // executable stub CLI in a temporary test directory + + for _, code := range []string{"0", "37"} { + t.Run("exit_"+code, func(t *testing.T) { + cmd := exec.CommandContext(t.Context(), shell.sh, "-c", trace.GuardHookCommand("chainloop trace hook cursor post-tool-use")) //nolint:gosec // run the installed hook against a local stub CLI + cmd.Env = append(os.Environ(), "PATH="+shell.bin, "HOOK_TEST_EXIT_CODE="+code) + cmd.Stdin = strings.NewReader("{\"session_id\":\"test\"}\n") + out, err := cmd.Output() + if code == "0" { + require.NoError(t, err) + } else { + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + assert.Equal(t, 37, exitErr.ExitCode()) + } + assert.Equal(t, "trace hook cursor post-tool-use"+"\n{\"session_id\":\"test\"}\n", string(out)) + }) + } +} + +type shellPath struct{ sh, bin string } + +// shellOnlyPath returns a PATH directory holding only sh, so chainloop is missing. +func shellOnlyPath(t *testing.T) shellPath { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the guard is a POSIX sh command") + } + sh, err := exec.LookPath("sh") + require.NoError(t, err) + bin := t.TempDir() + require.NoError(t, os.Symlink(sh, filepath.Join(bin, "sh"))) + return shellPath{sh: sh, bin: bin} +} + +func assertInstalledCommandsGuarded(t *testing.T, root string) { + t.Helper() + data, err := os.ReadFile(filepath.Join(root, settingsFile)) + require.NoError(t, err) + assert.Equal(t, len(cursorHookEvents), strings.Count(string(data), "command -v chainloop")) +} + +func TestInstallUpgradesUnguardedHooks(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, settingsFile) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700)) + legacy := `{"version":1,"hooks":{"postToolUse":[{"command":"./my-hook.sh"},{"command":"chainloop trace hook cursor post-tool-use","matcher":"` + shellToolName + `"}]}}` + require.NoError(t, os.WriteFile(path, []byte(legacy), 0600)) + + require.NoError(t, New().InstallHooks(root)) + first, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, 1, strings.Count(string(first), "exec chainloop trace hook cursor post-tool-use")) + assert.NotContains(t, string(first), `"command": "chainloop trace hook cursor post-tool-use"`) + assert.Contains(t, string(first), "./my-hook.sh") + + // Idempotent once upgraded. + require.NoError(t, New().InstallHooks(root)) + second, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, string(first), string(second)) +} diff --git a/app/cli/internal/trace/provider.go b/app/cli/internal/trace/provider.go index 490af29a5..ba4cbdb60 100644 --- a/app/cli/internal/trace/provider.go +++ b/app/cli/internal/trace/provider.go @@ -395,3 +395,11 @@ type ParseOpts struct { // both the providers registry and the action package can reference it // without a package import. const DefaultProviderName = "claude-code" + +// GuardHookCommand wraps an agent hook command so that a repository with the +// hooks installed keeps working when the chainloop CLI is not on PATH: the +// hook exits 0 instead of failing with "command not found" (127). When the +// CLI is present, exec hands it stdin, stdout and the exit status unchanged. +func GuardHookCommand(command string) string { + return "sh -c 'if ! command -v chainloop >/dev/null 2>&1; then exit 0; fi; exec " + command + "'" +} From d0772442eed01153280be0e452cf622b03bedc47 Mon Sep 17 00:00:00 2001 From: berce <75228263+bercedev@users.noreply.github.com> Date: Thu, 22 Oct 2026 12:02:22 +0300 Subject: [PATCH 2/4] fix(trace): answer Cursor's preToolUse and keep user handlers when upgrading hooks Cursor blocks the shell command when a preToolUse hook writes no valid response, so the missing-CLI guard of that hook now prints the same {"permission":"allow"} as AllowToolUse (new GuardHookCommandWithOutput). Upgrading an unguarded Claude hook replaced its whole group and dropped any user handler in it; the Chainloop handler is now updated in place, or moved to a group of its own when its matcher changed. Signed-off-by: berce <75228263+bercedev@users.noreply.github.com> --- app/cli/internal/trace/claude/hooks.go | 36 ++++++++++++++++-- app/cli/internal/trace/claude/hooks_test.go | 41 +++++++++++++++++++++ app/cli/internal/trace/cursor/hooks.go | 19 ++++++++-- app/cli/internal/trace/cursor/hooks_test.go | 19 +++++++--- app/cli/internal/trace/cursor/provider.go | 9 +++-- app/cli/internal/trace/provider.go | 9 +++++ 6 files changed, 119 insertions(+), 14 deletions(-) diff --git a/app/cli/internal/trace/claude/hooks.go b/app/cli/internal/trace/claude/hooks.go index fd11292e0..e8a30a0ee 100644 --- a/app/cli/internal/trace/claude/hooks.go +++ b/app/cli/internal/trace/claude/hooks.go @@ -245,11 +245,34 @@ func addHookEntry(hooks map[string]any, event, command, matcher string) bool { // changed, or an entry written before the missing-CLI guard) m, _ := entry.(map[string]any) existingMatcher, _ := m["matcher"].(string) - if existingMatcher == matcher && chainloopHookCommand(m) == trace.GuardHookCommand(command) { + guarded := trace.GuardHookCommand(command) + if existingMatcher == matcher && chainloopHookCommand(m) == guarded { return false // already up to date } - // Replace the entry with updated version - entries[i] = buildHookEntry(command, matcher) + // The group can also hold the user's own handlers: keep them. + handlers, _ := m["hooks"].([]any) + var others []any + for _, h := range handlers { + if !handlerIsChainloop(h) { + others = append(others, h) + } + } + switch { + case len(others) == 0: + entries[i] = buildHookEntry(command, matcher) + case existingMatcher == matcher: + // Only the command is out of date: update the Chainloop handler in place. + for _, h := range handlers { + if handlerIsChainloop(h) { + h.(map[string]any)["command"] = guarded + } + } + default: + // The matcher changed: leave the user's handlers under their matcher + // and give the Chainloop handler a group of its own. + m["hooks"] = others + entries = append(entries, buildHookEntry(command, matcher)) + } hooks[event] = entries return true } @@ -259,6 +282,13 @@ func addHookEntry(hooks map[string]any, event, command, matcher string) bool { return true } +// handlerIsChainloop reports whether a handler in a hook group runs the Chainloop CLI. +func handlerIsChainloop(h any) bool { + handler, _ := h.(map[string]any) + cmd, _ := handler["command"].(string) + return strings.Contains(cmd, hookMarker) +} + // chainloopHookCommand returns the command of the Chainloop handler in a hook group. func chainloopHookCommand(entry map[string]any) string { handlers, _ := entry["hooks"].([]any) diff --git a/app/cli/internal/trace/claude/hooks_test.go b/app/cli/internal/trace/claude/hooks_test.go index 32dd2d240..669d3f0c1 100644 --- a/app/cli/internal/trace/claude/hooks_test.go +++ b/app/cli/internal/trace/claude/hooks_test.go @@ -528,6 +528,47 @@ func assertInstalledCommandsGuarded(t *testing.T, root string) { assert.Equal(t, len(hookEvents), strings.Count(string(data), "command -v chainloop")) } +func TestInstallUpgradeKeepsUserHandlersInTheSameGroup(t *testing.T) { + const userHandler = `{"type":"command","command":"echo user-hook"}` + unguarded := `{"type":"command","command":"chainloop trace hook claude post-tool-use"}` + + t.Run("same matcher", func(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, settingsFile) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700)) + legacy := `{"hooks":{"PostToolUse":[{"matcher":"` + postHookToolMatcher + `","hooks":[` + userHandler + `,` + unguarded + `]}]}}` + require.NoError(t, os.WriteFile(path, []byte(legacy), 0600)) + + require.NoError(t, New().InstallHooks(root)) + postToolUse := readSettings(t, root)["hooks"].(map[string]any)[eventPostToolUse].([]any) + require.Len(t, postToolUse, 1) + group := postToolUse[0].(map[string]any) + assert.Equal(t, postHookToolMatcher, group["matcher"]) + handlers := group["hooks"].([]any) + require.Len(t, handlers, 2) + assert.Equal(t, "echo user-hook", handlers[0].(map[string]any)["command"]) + assert.Equal(t, trace.GuardHookCommand("chainloop trace hook claude post-tool-use"), handlers[1].(map[string]any)["command"]) + }) + + t.Run("changed matcher", func(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, settingsFile) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700)) + legacy := `{"hooks":{"PostToolUse":[{"matcher":"Bash","hooks":[` + userHandler + `,` + unguarded + `]}]}}` + require.NoError(t, os.WriteFile(path, []byte(legacy), 0600)) + + require.NoError(t, New().InstallHooks(root)) + postToolUse := readSettings(t, root)["hooks"].(map[string]any)[eventPostToolUse].([]any) + require.Len(t, postToolUse, 2) + user := postToolUse[0].(map[string]any) + assert.Equal(t, "Bash", user["matcher"], "the user's handler keeps its matcher") + assert.Equal(t, []any{map[string]any{"type": "command", "command": "echo user-hook"}}, user["hooks"]) + chainloop := postToolUse[1].(map[string]any) + assert.Equal(t, postHookToolMatcher, chainloop["matcher"]) + assert.Equal(t, trace.GuardHookCommand("chainloop trace hook claude post-tool-use"), chainloopHookCommand(chainloop)) + }) +} + func TestInstallUpgradesUnguardedHooks(t *testing.T) { root := t.TempDir() path := filepath.Join(root, settingsFile) diff --git a/app/cli/internal/trace/cursor/hooks.go b/app/cli/internal/trace/cursor/hooks.go index cdac42268..781497a14 100644 --- a/app/cli/internal/trace/cursor/hooks.go +++ b/app/cli/internal/trace/cursor/hooks.go @@ -62,6 +62,17 @@ type cursorHookEvent struct { // matcher limits a tool hook to the tools whose name it matches. Empty // for hooks that are not tool hooks. matcher string + // missingCLIOutput is what the hook prints when the chainloop CLI is not + // installed. Empty for hooks whose output Cursor does not require. + missingCLIOutput string +} + +// guardedCommand is the hook command as installed, wrapped in the missing-CLI guard. +func (h cursorHookEvent) guardedCommand() string { + if h.missingCLIOutput != "" { + return trace.GuardHookCommandWithOutput(h.command, h.missingCLIOutput) + } + return trace.GuardHookCommand(h.command) } // cursorHookEvents is the full list of hooks installed by InstallHooks. @@ -69,7 +80,9 @@ var cursorHookEvents = []cursorHookEvent{ {event: eventSessionStart, command: "chainloop trace hook cursor session-start"}, {event: eventSessionEnd, command: "chainloop trace hook cursor session-end"}, {event: eventAfterFileEdit, command: "chainloop trace hook cursor after-file-edit"}, - {event: eventPreToolUse, command: "chainloop trace hook cursor pre-tool-use", matcher: shellToolName}, + // Cursor blocks the shell command when preToolUse writes no valid response, + // so the guard answers for a missing CLI the way AllowToolUse does. + {event: eventPreToolUse, command: "chainloop trace hook cursor pre-tool-use", matcher: shellToolName, missingCLIOutput: allowToolUseResponse}, {event: eventPostToolUse, command: "chainloop trace hook cursor post-tool-use", matcher: shellToolName}, } @@ -289,7 +302,7 @@ func addHookEntry(hooks map[string]any, h cursorHookEvent) bool { m, _ := entry.(map[string]any) existingCmd, _ := m["command"].(string) existingMatcher, _ := m["matcher"].(string) - if existingCmd == trace.GuardHookCommand(h.command) && existingMatcher == h.matcher { + if existingCmd == h.guardedCommand() && existingMatcher == h.matcher { return false } entries[i] = buildHookEntry(h) @@ -331,7 +344,7 @@ func removeHookEntry(hooks map[string]any, event string) bool { func buildHookEntry(h cursorHookEvent) map[string]any { entry := map[string]any{ - "command": trace.GuardHookCommand(h.command), + "command": h.guardedCommand(), "timeout": defaultHookTimeoutSeconds, } if h.matcher != "" { diff --git a/app/cli/internal/trace/cursor/hooks_test.go b/app/cli/internal/trace/cursor/hooks_test.go index 95b9c7fa9..198060415 100644 --- a/app/cli/internal/trace/cursor/hooks_test.go +++ b/app/cli/internal/trace/cursor/hooks_test.go @@ -273,13 +273,13 @@ func TestInstallHooksShellToolHooks(t *testing.T) { hooks := readInstalledHooks(t, repoRoot) for event, command := range map[string]string{ - eventPreToolUse: "chainloop trace hook cursor pre-tool-use", - eventPostToolUse: "chainloop trace hook cursor post-tool-use", + eventPreToolUse: trace.GuardHookCommandWithOutput("chainloop trace hook cursor pre-tool-use", allowToolUseResponse), + eventPostToolUse: trace.GuardHookCommand("chainloop trace hook cursor post-tool-use"), } { entries, _ := hooks[event].([]any) require.Len(t, entries, 1, "entries for %s", event) entry, _ := entries[0].(map[string]any) - assert.Equal(t, trace.GuardHookCommand(command), entry["command"], "command for %s", event) + assert.Equal(t, command, entry["command"], "command for %s", event) assert.Equal(t, shellToolName, entry["matcher"], "matcher for %s", event) } @@ -396,12 +396,21 @@ func TestInstalledHooksWithoutCLI(t *testing.T) { for _, event := range cursorHookEvents { t.Run(event.event+"/"+event.command, func(t *testing.T) { - cmd := exec.CommandContext(t.Context(), shell.sh, "-c", trace.GuardHookCommand(event.command)) //nolint:gosec // run the installed hook to check its shell behaviour + cmd := exec.CommandContext(t.Context(), shell.sh, "-c", event.guardedCommand()) //nolint:gosec // run the installed hook to check its shell behaviour cmd.Env = append(os.Environ(), "PATH="+shell.bin) cmd.Stdin = strings.NewReader(`{"session_id":"test"}`) out, err := cmd.CombinedOutput() require.NoError(t, err, string(out)) - assert.Empty(t, out) + if event.event != eventPreToolUse { + assert.Empty(t, out) + return + } + // Cursor blocks the shell command unless preToolUse answers. + var resp struct { + Permission string `json:"permission"` + } + require.NoError(t, json.Unmarshal(out, &resp), string(out)) + assert.Equal(t, "allow", resp.Permission) }) } assertInstalledCommandsGuarded(t, root) diff --git a/app/cli/internal/trace/cursor/provider.go b/app/cli/internal/trace/cursor/provider.go index 01c64839c..dcd7b6e0b 100644 --- a/app/cli/internal/trace/cursor/provider.go +++ b/app/cli/internal/trace/cursor/provider.go @@ -113,14 +113,17 @@ func (p *Provider) AnnouncePromptSubmit(_ string) error { return nil } +// allowToolUseResponse is the preToolUse hook response that lets the tool +// call proceed. The missing-CLI guard of the installed hook prints it too. +const allowToolUseResponse = `{"permission":"allow"}` + // AllowToolUse writes the preToolUse hook response that lets the tool call // proceed. Cursor blocks the call when a preToolUse hook writes a response // that is not valid JSON or does not match the schema, so the pre hook writes // this response whatever the trace does. func (p *Provider) AllowToolUse() error { - return json.NewEncoder(os.Stdout).Encode(struct { - Permission string `json:"permission"` - }{Permission: "allow"}) + _, err := fmt.Fprintln(os.Stdout, allowToolUseResponse) + return err } // AnnounceToUser is unsupported for Cursor: the postToolUse response that diff --git a/app/cli/internal/trace/provider.go b/app/cli/internal/trace/provider.go index ba4cbdb60..720d037ba 100644 --- a/app/cli/internal/trace/provider.go +++ b/app/cli/internal/trace/provider.go @@ -21,6 +21,7 @@ import ( "errors" "io" "sort" + "strconv" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/pointer" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/skill" @@ -403,3 +404,11 @@ const DefaultProviderName = "claude-code" func GuardHookCommand(command string) string { return "sh -c 'if ! command -v chainloop >/dev/null 2>&1; then exit 0; fi; exec " + command + "'" } + +// GuardHookCommandWithOutput is GuardHookCommand for a hook whose agent needs a +// response even when the CLI is missing: the guard prints output before it +// exits 0. output must not contain a single quote. +func GuardHookCommandWithOutput(command, output string) string { + return "sh -c 'if ! command -v chainloop >/dev/null 2>&1; then echo " + strconv.Quote(output) + + "; exit 0; fi; exec " + command + "'" +} From c44a01bbc548171735c7c8fd9b52a94643cb5bb1 Mon Sep 17 00:00:00 2001 From: berce <75228263+bercedev@users.noreply.github.com> Date: Thu, 22 Oct 2026 12:09:46 +0300 Subject: [PATCH 3/4] fix(trace): keep user handlers in a Chainloop hook group on uninstall removeHookEntry dropped every group that held a Chainloop handler, together with any user handler in it. It now strips only the Chainloop handler and drops the group when nothing else is left. Signed-off-by: berce <75228263+bercedev@users.noreply.github.com> --- app/cli/internal/trace/claude/hooks.go | 18 +++++++++++++++++- app/cli/internal/trace/claude/hooks_test.go | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/app/cli/internal/trace/claude/hooks.go b/app/cli/internal/trace/claude/hooks.go index e8a30a0ee..6fbe4840a 100644 --- a/app/cli/internal/trace/claude/hooks.go +++ b/app/cli/internal/trace/claude/hooks.go @@ -323,13 +323,29 @@ func removeHookEntry(hooks map[string]any, event string) bool { } var kept []any + changed := false for _, entry := range entries { if !entryContainsChainloopHook(entry) { kept = append(kept, entry) + continue + } + changed = true + // Strip only the Chainloop handler: the group can also hold the user's own. + m, _ := entry.(map[string]any) + handlers, _ := m["hooks"].([]any) + var others []any + for _, h := range handlers { + if !handlerIsChainloop(h) { + others = append(others, h) + } + } + if len(others) > 0 { + m["hooks"] = others + kept = append(kept, m) } } - if len(kept) == len(entries) { + if !changed { return false } diff --git a/app/cli/internal/trace/claude/hooks_test.go b/app/cli/internal/trace/claude/hooks_test.go index 669d3f0c1..b7940d397 100644 --- a/app/cli/internal/trace/claude/hooks_test.go +++ b/app/cli/internal/trace/claude/hooks_test.go @@ -569,6 +569,22 @@ func TestInstallUpgradeKeepsUserHandlersInTheSameGroup(t *testing.T) { }) } +func TestUninstallKeepsUserHandlersInAChainloopGroup(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, settingsFile) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700)) + shared := `{"hooks":{"PostToolUse":[{"matcher":"` + postHookToolMatcher + `","hooks":[{"type":"command","command":"echo user-hook"},{"type":"command","command":"` + + strings.ReplaceAll(trace.GuardHookCommand("chainloop trace hook claude post-tool-use"), `"`, `\"`) + `"}]}]}}` + require.NoError(t, os.WriteFile(path, []byte(shared), 0600)) + + require.NoError(t, New().UninstallHooks(root)) + postToolUse := readSettings(t, root)["hooks"].(map[string]any)[eventPostToolUse].([]any) + require.Len(t, postToolUse, 1) + group := postToolUse[0].(map[string]any) + assert.Equal(t, postHookToolMatcher, group["matcher"]) + assert.Equal(t, []any{map[string]any{"type": "command", "command": "echo user-hook"}}, group["hooks"]) +} + func TestInstallUpgradesUnguardedHooks(t *testing.T) { root := t.TempDir() path := filepath.Join(root, settingsFile) From c619fbdc6531374f9898bc7314ff741c65db8d6e Mon Sep 17 00:00:00 2001 From: berce <75228263+bercedev@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:41:09 +0300 Subject: [PATCH 4/4] refactor(trace): build the Codex POSIX hook guard with trace.GuardHookCommand The Codex provider kept its own copy of the guard's shell string. It now uses trace.GuardHookCommand, like the Claude Code and Cursor providers, and derives the prefix it strips when recognising its handler from the same function. The Windows prefix stays in the Codex package, since only Codex uses it. Signed-off-by: berce <75228263+bercedev@users.noreply.github.com> --- app/cli/internal/trace/codex/hooks.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/cli/internal/trace/codex/hooks.go b/app/cli/internal/trace/codex/hooks.go index 40b2a0099..e700cbc43 100644 --- a/app/cli/internal/trace/codex/hooks.go +++ b/app/cli/internal/trace/codex/hooks.go @@ -31,8 +31,9 @@ import ( const ( hookMarker = "chainloop trace hook codex" settingsFile = ".codex/hooks.json" - // Skip tracing when the CLI is missing, before any Go handler can run. - hookCommandPrefix = "sh -c 'if ! command -v chainloop >/dev/null 2>&1; then exit 0; fi; exec " + // Skip tracing on Windows when the CLI is missing, before any Go handler + // can run. The POSIX command uses trace.GuardHookCommand, like the other + // providers. windowsHookPrefix = `cmd.exe /d /s /c "where chainloop >nul 2>nul & if errorlevel 1 (exit /b 0) else (` // Codex hook event names. @@ -254,7 +255,7 @@ func addHookEntry(hooks map[string]any, event, command, matcher, statusMessage s existingMatcher, _ := group["matcher"].(string) if len(handlers) == 1 && existingMatcher == matcher { handler, _ := handlers[0].(map[string]any) - if handler["command"] == hookCommandPrefix+command+"'" && handler["commandWindows"] == windowsHookPrefix+command+`)"` && handler["statusMessage"] == statusMessage { + if handler["command"] == trace.GuardHookCommand(command) && handler["commandWindows"] == windowsHookPrefix+command+`)"` && handler["statusMessage"] == statusMessage { return false } } @@ -271,7 +272,7 @@ func buildHookEntry(command, matcher, statusMessage string) map[string]any { map[string]any{ "type": "command", "statusMessage": statusMessage, - "command": hookCommandPrefix + command + "'", + "command": trace.GuardHookCommand(command), "commandWindows": windowsHookPrefix + command + `)"`, }, }, @@ -319,14 +320,18 @@ func removeHookEntry(hooks map[string]any, event string) bool { return true } +// guardPrefix is the part of a guarded command that comes before the wrapped +// command; trace.GuardHookCommand closes it with a single quote. +var guardPrefix = strings.TrimSuffix(trace.GuardHookCommand(""), "'") + func isChainloopHandler(handler any) bool { h, ok := handler.(map[string]any) if !ok { return false } command, _ := h["command"].(string) - if strings.HasPrefix(command, hookCommandPrefix) && strings.HasSuffix(command, "'") { - command = strings.TrimSuffix(strings.TrimPrefix(command, hookCommandPrefix), "'") + if strings.HasPrefix(command, guardPrefix) && strings.HasSuffix(command, "'") { + command = strings.TrimSuffix(strings.TrimPrefix(command, guardPrefix), "'") } return strings.HasPrefix(command, hookMarker+" ") }