diff --git a/app/cli/cmd/trace_init.go b/app/cli/cmd/trace_init.go index 4d80f4b2d..8cecd15a6 100644 --- a/app/cli/cmd/trace_init.go +++ b/app/cli/cmd/trace_init.go @@ -61,8 +61,9 @@ exists, so you need to be logged in. On a terminal it asks which organization and project to use, offering what .chainloop.yml already holds so pressing Enter keeps it. A new project can be named freely; the name is normalized to the lowercase, dash-separated form -Chainloop stores. It then asks which harnesses to trace, with Claude Code -ticked; use space to tick more. Passing --org, --project or a harness flag +Chainloop stores. It then asks which harnesses to trace, with the ones the +repository is already set up for ticked, or Claude Code on a first run; use +space to tick more. Passing --org, --project or a harness flag (--claude, --cursor, --opencode) skips the matching question. Nothing is asked in CI or when the output is redirected: there --project is @@ -96,7 +97,7 @@ The organization, project, workflow and require-trace values are saved to // repository untouched. selected, err := resolveTraceProviders(newNamingPrompter(os.LookupEnv), traceProviderFlags{claude: claudeFlag, cursor: cursorFlag, opencode: opencodeFlag}, - traceInitCanPrompt()) + traceInitCanPrompt(), func() []string { return configuredTraceProviders(repoRoot) }) if err != nil { return stopIfAborted(err) } diff --git a/app/cli/cmd/trace_init_providers.go b/app/cli/cmd/trace_init_providers.go index e4f4cfc18..e88d1f735 100644 --- a/app/cli/cmd/trace_init_providers.go +++ b/app/cli/cmd/trace_init_providers.go @@ -18,6 +18,7 @@ package cmd import ( "errors" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/claude" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/cursor" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/opencode" @@ -78,11 +79,34 @@ func traceProviderNames() []string { return names } +// configuredTraceProviders lists the harnesses whose hooks the repository +// already carries, in the registry's order, so a re-run of init offers what an +// earlier one set up. A harness whose configuration cannot be read counts as +// not configured: init is how it gets repaired, so it must not stop init. +func configuredTraceProviders(repoRoot string) []string { + var configured []trace.Provider + for _, p := range providers.All() { + installed, err := p.HooksInstalled(repoRoot) + if err != nil { + logger.Debug().Err(err).Str("harness", p.Name()).Msg("could not read the harness configuration") + continue + } + + if installed { + configured = append(configured, p) + } + } + + return providerNames(configured) +} + // resolveTraceProviders picks the agents whose hooks init installs. Flags win // and skip the question, the same way --org and --project do. Otherwise a -// terminal session ticks them off a list with the default preselected, and a -// non-interactive one keeps that default so scripted runs are unchanged. -func resolveTraceProviders(p prompter, flags traceProviderFlags, interactive bool) ([]string, error) { +// terminal session ticks them off a list with the harnesses already configured +// preselected, or the default on a first run, and a non-interactive one keeps +// the default so scripted runs are unchanged. configured is only called when +// the question is asked, so the other paths read nothing from the repository. +func resolveTraceProviders(p prompter, flags traceProviderFlags, interactive bool, configured func() []string) ([]string, error) { if flags.any() { return flags.names(), nil } @@ -91,7 +115,12 @@ func resolveTraceProviders(p prompter, flags traceProviderFlags, interactive boo return []string{providers.DefaultProvider}, nil } - chosen, err := p.MultiSelect(providersPromptTitle, traceProviderNames(), []string{providers.DefaultProvider}) + defaults := configured() + if len(defaults) == 0 { + defaults = []string{providers.DefaultProvider} + } + + chosen, err := p.MultiSelect(providersPromptTitle, traceProviderNames(), defaults) if err != nil { return nil, err } diff --git a/app/cli/cmd/trace_init_providers_test.go b/app/cli/cmd/trace_init_providers_test.go index c9972d3ce..83170a267 100644 --- a/app/cli/cmd/trace_init_providers_test.go +++ b/app/cli/cmd/trace_init_providers_test.go @@ -16,6 +16,8 @@ package cmd import ( + "os" + "path/filepath" "testing" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/providers" @@ -34,11 +36,15 @@ func TestResolveTraceProviders(t *testing.T) { name string flags traceProviderFlags interactive bool + // configured are the harnesses whose hooks the repository already has + configured []string // answer is what the user ticks in the multi-select answer []string want []string // wantPrompted is whether the multi-select should have been shown wantPrompted bool + // wantDefaults is what the multi-select comes up with ticked + wantDefaults []string wantErr string }{ { @@ -64,6 +70,32 @@ func TestResolveTraceProviders(t *testing.T) { answer: []string{"cursor", "opencode"}, want: []string{"cursor", "opencode"}, wantPrompted: true, + wantDefaults: []string{providers.DefaultProvider}, + }, + { + name: "a re-run comes up with the configured harnesses ticked", + interactive: true, + configured: []string{providerClaudeCode, "cursor", "opencode"}, + answer: []string{providerClaudeCode, "cursor", "opencode"}, + want: []string{providerClaudeCode, "cursor", "opencode"}, + wantPrompted: true, + wantDefaults: []string{providerClaudeCode, "cursor", "opencode"}, + }, + { + name: "a re-run without the default ticks only what is configured", + interactive: true, + configured: []string{"cursor"}, + answer: []string{"cursor"}, + want: []string{"cursor"}, + wantPrompted: true, + wantDefaults: []string{"cursor"}, + }, + { + name: "flags still win over the configured harnesses", + flags: traceProviderFlags{opencode: true}, + interactive: true, + configured: []string{"cursor"}, + want: []string{"opencode"}, }, { name: "picking nothing is refused", @@ -78,7 +110,13 @@ func TestResolveTraceProviders(t *testing.T) { t.Run(tc.name, func(t *testing.T) { p := &fakePrompter{multiSelectAnswer: tc.answer} - got, err := resolveTraceProviders(p, tc.flags, tc.interactive) + var readConfigured bool + configured := func() []string { + readConfigured = true + return tc.configured + } + + got, err := resolveTraceProviders(p, tc.flags, tc.interactive, configured) if tc.wantErr != "" { require.Error(t, err) @@ -91,22 +129,70 @@ func TestResolveTraceProviders(t *testing.T) { if !tc.wantPrompted { assert.Empty(t, p.multiSelects, "no question should have been asked") + assert.False(t, readConfigured, "the repository is only read to preselect the question") return } require.Len(t, p.multiSelects, 1) - // Every registered provider is offered, with the default ticked. + // Every registered provider is offered. assert.Equal(t, traceProviderNames(), p.multiSelects[0].options) - assert.Equal(t, []string{providers.DefaultProvider}, p.multiSelects[0].defaults) + assert.Equal(t, tc.wantDefaults, p.multiSelects[0].defaults) }) } } func TestResolveTraceProvidersPromptError(t *testing.T) { - _, err := resolveTraceProviders(&fakePrompter{multiSelectErr: errAborted}, traceProviderFlags{}, true) + _, err := resolveTraceProviders(&fakePrompter{multiSelectErr: errAborted}, traceProviderFlags{}, true, func() []string { return nil }) require.ErrorIs(t, err, errAborted) } +// TestConfiguredTraceProviders reads the harnesses back from hooks the real +// providers wrote, which is what a re-run of trace init finds (PFM-7633). +func TestConfiguredTraceProviders(t *testing.T) { + testCases := []struct { + name string + installed []string + want []string + }{ + { + name: "a repository never set up", + want: []string{}, + }, + { + name: "some harnesses set up", + installed: []string{"opencode", "cursor"}, + // In the registry's order, the same order the prompt lists them in. + want: []string{"cursor", "opencode"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + repoRoot := t.TempDir() + for _, p := range providers.ByNames(tc.installed) { + require.NoError(t, p.InstallHooks(repoRoot)) + } + + assert.Equal(t, tc.want, configuredTraceProviders(repoRoot)) + }) + } +} + +// TestConfiguredTraceProvidersSkipsUnreadableSettings keeps a broken harness +// configuration from stopping init, which would otherwise be the way to repair it. +func TestConfiguredTraceProvidersSkipsUnreadableSettings(t *testing.T) { + repoRoot := t.TempDir() + + cursor := providers.ByName("cursor") + require.NoError(t, cursor.InstallHooks(repoRoot)) + + claudeSettings := providers.ByName(providerClaudeCode).SettingsFile(repoRoot) + require.NoError(t, os.MkdirAll(filepath.Dir(claudeSettings), 0o755)) + require.NoError(t, os.WriteFile(claudeSettings, []byte("{not json"), 0o600)) + + assert.Equal(t, []string{"cursor"}, configuredTraceProviders(repoRoot)) +} + func TestTraceProviderFlags(t *testing.T) { t.Run("no flag set", func(t *testing.T) { assert.False(t, traceProviderFlags{}.any()) diff --git a/app/cli/documentation/cli-reference.md b/app/cli/documentation/cli-reference.md index 7ca0e9c10..9ecf4b11a 100644 --- a/app/cli/documentation/cli-reference.md +++ b/app/cli/documentation/cli-reference.md @@ -3310,8 +3310,9 @@ exists, so you need to be logged in. On a terminal it asks which organization and project to use, offering what .chainloop.yml already holds so pressing Enter keeps it. A new project can be named freely; the name is normalized to the lowercase, dash-separated form -Chainloop stores. It then asks which harnesses to trace, with Claude Code -ticked; use space to tick more. Passing --org, --project or a harness flag +Chainloop stores. It then asks which harnesses to trace, with the ones the +repository is already set up for ticked, or Claude Code on a first run; use +space to tick more. Passing --org, --project or a harness flag (--claude, --cursor, --opencode) skips the matching question. Nothing is asked in CI or when the output is redirected: there --project is diff --git a/app/cli/internal/trace/claude/hooks.go b/app/cli/internal/trace/claude/hooks.go index 2b975c01c..0b376d0fb 100644 --- a/app/cli/internal/trace/claude/hooks.go +++ b/app/cli/internal/trace/claude/hooks.go @@ -22,6 +22,7 @@ import ( "io" "os" "path/filepath" + "slices" "strings" "github.com/chainloop-dev/chainloop/app/cli/internal/trace" @@ -163,6 +164,25 @@ func (p *Provider) UninstallHooks(repoRoot string) error { return writeJSONFile(settingsPath, settings) } +// HooksInstalled reports whether .claude/settings.json holds any Chainloop +// hook. User-authored hooks are ignored. +func (p *Provider) HooksInstalled(repoRoot string) (bool, error) { + settings, err := readJSONFile(filepath.Join(repoRoot, settingsFile)) + if err != nil { + return false, err + } + + hooks, _ := settings["hooks"].(map[string]any) + for _, h := range hookEvents { + entries, _ := hooks[h.event].([]any) + if slices.ContainsFunc(entries, entryContainsChainloopHook) { + return true, nil + } + } + + return false, nil +} + // maxHookPayloadBytes caps hook payload reads to defend against runaway or // malformed payloads. const maxHookPayloadBytes = 16 * 1024 * 1024 diff --git a/app/cli/internal/trace/claude/hooks_test.go b/app/cli/internal/trace/claude/hooks_test.go index 5f666f451..0b507b152 100644 --- a/app/cli/internal/trace/claude/hooks_test.go +++ b/app/cli/internal/trace/claude/hooks_test.go @@ -375,6 +375,48 @@ func TestInstallHooksOnNullSettings(t *testing.T) { assert.Contains(t, readSettings(t, repoRoot), "hooks") } +func TestHooksInstalled(t *testing.T) { + testCases := []struct { + name string + settings string + want bool + wantErr bool + }{ + { + name: "only user hooks", + settings: `{"hooks":{"PostToolUse":[{"hooks":[{"type":"command","command":"format.sh"}]}]}}`, + want: false, + }, + { + name: "a chainloop hook next to user hooks", + settings: `{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"format.sh"},{"type":"command","command":"chainloop trace hook claude session-start"}]}]}}`, + want: true, + }, + { + name: "malformed settings", + settings: `{not json`, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + repoRoot := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".claude"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, settingsFile), []byte(tc.settings), 0600)) + + got, err := New().HooksInstalled(repoRoot) + if tc.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + func readSettings(t *testing.T, repoRoot string) map[string]any { t.Helper() diff --git a/app/cli/internal/trace/cursor/hooks.go b/app/cli/internal/trace/cursor/hooks.go index 6ab9eb0cd..688712576 100644 --- a/app/cli/internal/trace/cursor/hooks.go +++ b/app/cli/internal/trace/cursor/hooks.go @@ -22,6 +22,7 @@ import ( "io" "os" "path/filepath" + "slices" "strings" "github.com/chainloop-dev/chainloop/app/cli/internal/trace" @@ -164,6 +165,25 @@ func (p *Provider) UninstallHooks(repoRoot string) error { return writeJSONFile(settingsPath, settings) } +// HooksInstalled reports whether .cursor/hooks.json holds any Chainloop hook. +// User-authored hooks are ignored. +func (p *Provider) HooksInstalled(repoRoot string) (bool, error) { + settings, err := readJSONFile(filepath.Join(repoRoot, settingsFile)) + if err != nil { + return false, err + } + + hooks, _ := settings["hooks"].(map[string]any) + for _, h := range cursorHookEvents { + entries, _ := hooks[h.event].([]any) + if slices.ContainsFunc(entries, entryContainsChainloopHook) { + return true, nil + } + } + + return false, nil +} + // cursorHookInput is the wire-level structure of a Cursor hook JSON payload. // Only the fields we consume are modelled; unknown fields are ignored. type cursorHookInput struct { diff --git a/app/cli/internal/trace/cursor/hooks_test.go b/app/cli/internal/trace/cursor/hooks_test.go index 967572343..d42369d06 100644 --- a/app/cli/internal/trace/cursor/hooks_test.go +++ b/app/cli/internal/trace/cursor/hooks_test.go @@ -209,3 +209,45 @@ func TestReadHookInputFallsBackToSessionID(t *testing.T) { require.NoError(t, err) assert.Equal(t, "fallback-session", in.SessionID, "expected fallback to session_id") } + +func TestHooksInstalled(t *testing.T) { + testCases := []struct { + name string + settings string + want bool + wantErr bool + }{ + { + name: "only user hooks", + settings: `{"version":1,"hooks":{"sessionStart":[{"command":"./my-hook.sh","timeout":10}]}}`, + want: false, + }, + { + name: "a chainloop hook next to user hooks", + settings: `{"version":1,"hooks":{"sessionStart":[{"command":"./my-hook.sh"},{"command":"chainloop trace hook cursor session-start"}]}}`, + want: true, + }, + { + name: "malformed settings", + settings: `{not json`, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + repoRoot := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".cursor"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, settingsFile), []byte(tc.settings), 0600)) + + got, err := New().HooksInstalled(repoRoot) + if tc.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/app/cli/internal/trace/opencode/hooks.go b/app/cli/internal/trace/opencode/hooks.go index 2ac5f7247..7e76101e3 100644 --- a/app/cli/internal/trace/opencode/hooks.go +++ b/app/cli/internal/trace/opencode/hooks.go @@ -380,6 +380,17 @@ func (p *Provider) UninstallHooks(repoRoot string) error { return err } +// HooksInstalled reports whether the plugin file is present. The file is +// entirely chainloop-owned, so its presence is the installation. +func (p *Provider) HooksInstalled(repoRoot string) (bool, error) { + _, err := os.Stat(filepath.Join(repoRoot, settingsFile)) + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + + return err == nil, err +} + // maxHookPayloadBytes caps hook payload reads to defend against runaway or // malformed payloads. const maxHookPayloadBytes = 16 * 1024 * 1024 diff --git a/app/cli/internal/trace/provider.go b/app/cli/internal/trace/provider.go index 114cd9b56..9bda6655b 100644 --- a/app/cli/internal/trace/provider.go +++ b/app/cli/internal/trace/provider.go @@ -108,6 +108,11 @@ type Provider interface { // UninstallHooks removes the agent's hooks from the repo. UninstallHooks(repoRoot string) error + // HooksInstalled reports whether the repo already carries the agent's + // Chainloop hooks, so `trace init` can offer the harnesses a repository + // is set up for when it runs again. Hooks the user wrote do not count. + HooksInstalled(repoRoot string) (bool, error) + // ReadHookInput reads hook invocation input from the given reader. ReadHookInput(r io.Reader) (*HookInput, error) diff --git a/app/cli/internal/trace/providers/hooks_installed_test.go b/app/cli/internal/trace/providers/hooks_installed_test.go new file mode 100644 index 000000000..85df212bf --- /dev/null +++ b/app/cli/internal/trace/providers/hooks_installed_test.go @@ -0,0 +1,69 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package providers + +import ( + "testing" + + "github.com/chainloop-dev/chainloop/app/cli/internal/trace" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHooksInstalled checks, for every registered provider, that HooksInstalled +// follows what InstallHooks and UninstallHooks leave in the repository. trace +// init relies on it to offer the harnesses already set up on a re-run. +func TestHooksInstalled(t *testing.T) { + testCases := []struct { + name string + setup func(t *testing.T, p trace.Provider, repoRoot string) + want bool + }{ + { + name: "a repository never set up", + setup: func(*testing.T, trace.Provider, string) {}, + want: false, + }, + { + name: "after init installs the hooks", + setup: func(t *testing.T, p trace.Provider, repoRoot string) { + require.NoError(t, p.InstallHooks(repoRoot)) + }, + want: true, + }, + { + name: "after the hooks are uninstalled again", + setup: func(t *testing.T, p trace.Provider, repoRoot string) { + require.NoError(t, p.InstallHooks(repoRoot)) + require.NoError(t, p.UninstallHooks(repoRoot)) + }, + want: false, + }, + } + + for _, p := range All() { + for _, tc := range testCases { + t.Run(p.Name()+"/"+tc.name, func(t *testing.T) { + repoRoot := t.TempDir() + tc.setup(t, p, repoRoot) + + got, err := p.HooksInstalled(repoRoot) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } + } +}