diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index caa1a9a..ae74ca4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,14 +26,5 @@ jobs: with: go-version: "1.26" - - name: Install staticcheck - run: go install honnef.co/go/tools/cmd/staticcheck@latest - - - name: Install golangci-lint - run: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest - - - name: Install govulncheck - run: go install golang.org/x/vuln/cmd/govulncheck@latest - - name: Run verification suite run: make verify diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ba7005..b634e83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `atb update` now verifies the SHA256 checksum of the downloaded archive before replacing the current binary, failing closed on mismatch. ### Changed +- Verification tool versions (`staticcheck`, `golangci-lint`, `govulncheck`) are now pinned in the Makefile and shared by CI, replacing `@latest` installs for reproducible builds. - `State.Tool()` now returns `(ToolState, bool)` by value instead of a pointer to a copy, making copy semantics explicit and eliminating a potential mutation footgun. - `confirmApply` now reads from an injected `io.Reader` instead of hardcoding `os.Stdin`, improving testability and enabling non-interactive integrations. ### Fixed +- Shell-hook prompting now skips tools whose hooks have already been applied or declined, and reconciles state to `applied` when the init line is already present in the rc file. - Canceling the skill-target picker during install now persists an explicit opt-out, preventing `update tools` and `uninstall` from writing skill files into default agent directories. - Lifecycle state is now saved immediately after install, update, and uninstall plan execution, before shell-hook prompting and skill generation, preventing receipt loss when post-processing steps fail. - The standalone installer now surfaces `mkdir -p` failures directly instead of suppressing the error and failing later with weaker diagnostics. diff --git a/Makefile b/Makefile index d9b99db..627c7bc 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,10 @@ GOVULNCHECK := $(TOOLS_BIN)/govulncheck export GOBIN := $(TOOLS_BIN) export PATH := $(TOOLS_BIN):$(PATH) +STATICCHECK_VERSION ?= 2025.1.1 +GOLANGCI_LINT_VERSION ?= v2.11.3 +GOVULNCHECK_VERSION ?= v1.1.4 + .PHONY: help verify fmt vet lint test build run vulncheck tools check-go RUN_ARGS := $(wordlist 2,$(words $(MAKECMDGOALS)),$(MAKECMDGOALS)) @@ -67,8 +71,8 @@ vulncheck: tools tools: @mkdir -p "$(TOOLS_BIN)" - @test -x "$(STATICCHECK)" || $(GO) install honnef.co/go/tools/cmd/staticcheck@latest - @if [[ ! -x "$(GOLANGCI_LINT)" ]] || ! "$(GOLANGCI_LINT)" --version 2>/dev/null | grep -q "version 2\\."; then \ - $(GO) install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest; \ + @test -x "$(STATICCHECK)" || $(GO) install honnef.co/go/tools/cmd/staticcheck@$(STATICCHECK_VERSION) + @if [[ ! -x "$(GOLANGCI_LINT)" ]] || ! "$(GOLANGCI_LINT)" --version 2>/dev/null | grep -q "$(GOLANGCI_LINT_VERSION)"; then \ + $(GO) install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION); \ fi - @test -x "$(GOVULNCHECK)" || $(GO) install golang.org/x/vuln/cmd/govulncheck@latest + @test -x "$(GOVULNCHECK)" || $(GO) install golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) diff --git a/cmd/atb/runtime.go b/cmd/atb/runtime.go index 7190e58..8a44769 100644 --- a/cmd/atb/runtime.go +++ b/cmd/atb/runtime.go @@ -451,7 +451,7 @@ func runUninstall(ctx context.Context, stdout, stderr io.Writer, toolIDs []strin } func applyShellWorkflow(stdin io.Reader, stdout io.Writer, yes bool, st *state.State, tools []catalog.Tool) error { - suggestions := shell.Suggestions(tools) + suggestions := shell.Suggestions(tools, *st) if len(suggestions) == 0 { return nil } diff --git a/internal/shell/shell.go b/internal/shell/shell.go index 965e64e..6835b0d 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -42,7 +42,8 @@ func DetectShell() string { } // Suggestions returns init-line suggestions for tools with shell hooks. -func Suggestions(tools []catalog.Tool) []Suggestion { +// Tools whose hook status is already "applied" or "declined" are skipped. +func Suggestions(tools []catalog.Tool, st state.State) []Suggestion { shellName := DetectShell() rcFile, err := rcFileForShell(shellName) if err != nil { @@ -55,6 +56,12 @@ func Suggestions(tools []catalog.Tool) []Suggestion { continue } + if receipt, ok := st.Tool(tool.ID); ok { + if receipt.ShellHookStatus == shellHookApplied || receipt.ShellHookStatus == shellHookDeclined { + continue + } + } + initLine, ok := initLineForTool(tool.ID, shellName) if !ok { continue @@ -74,16 +81,14 @@ func Suggestions(tools []catalog.Tool) []Suggestion { } // ApplyConfirmedSuggestions appends missing init lines and records applied state. +// State is reconciled to "applied" even when the line already exists in the rc file. func ApplyConfirmedSuggestions(suggestions []Suggestion, st *state.State) error { for _, suggestion := range suggestions { - written, err := appendInitLine(suggestion.RCFile, suggestion.InitLine) - if err != nil { + if _, err := appendInitLine(suggestion.RCFile, suggestion.InitLine); err != nil { return err } - if written { - recordHookStatus(st, suggestion.ToolID, shellHookApplied, time.Now().UTC()) - } + recordHookStatus(st, suggestion.ToolID, shellHookApplied, time.Now().UTC()) } return nil diff --git a/internal/shell/shell_test.go b/internal/shell/shell_test.go index 20620e6..9c9f25c 100644 --- a/internal/shell/shell_test.go +++ b/internal/shell/shell_test.go @@ -34,7 +34,7 @@ func TestSuggestions(t *testing.T) { suggestions := Suggestions([]catalog.Tool{ {ID: "direnv", Name: "direnv", ShellHook: "required"}, {ID: "jq", Name: "jq", ShellHook: "none"}, - }) + }, state.State{}) if len(suggestions) != 1 { t.Fatalf("len(Suggestions()) = %d, want 1", len(suggestions)) @@ -45,6 +45,110 @@ func TestSuggestions(t *testing.T) { } } +func TestSuggestionsSkipsAlreadyApplied(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("SHELL", "/bin/bash") + + st := state.State{Version: 1, Tools: map[string]state.ToolState{ + "direnv": {ToolID: "direnv", ShellHookStatus: shellHookApplied}, + }} + + suggestions := Suggestions([]catalog.Tool{ + {ID: "direnv", Name: "direnv", ShellHook: "required"}, + }, st) + + if len(suggestions) != 0 { + t.Fatalf("len(Suggestions()) = %d, want 0 for already applied", len(suggestions)) + } +} + +func TestSuggestionsSkipsDeclined(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("SHELL", "/bin/bash") + + st := state.State{Version: 1, Tools: map[string]state.ToolState{ + "direnv": {ToolID: "direnv", ShellHookStatus: shellHookDeclined}, + }} + + suggestions := Suggestions([]catalog.Tool{ + {ID: "direnv", Name: "direnv", ShellHook: "required"}, + }, st) + + if len(suggestions) != 0 { + t.Fatalf("len(Suggestions()) = %d, want 0 for declined", len(suggestions)) + } +} + +func TestSuggestionsIncludesPreviouslySuggested(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("SHELL", "/bin/bash") + + st := state.State{Version: 1, Tools: map[string]state.ToolState{ + "direnv": {ToolID: "direnv", ShellHookStatus: shellHookSuggested}, + }} + + suggestions := Suggestions([]catalog.Tool{ + {ID: "direnv", Name: "direnv", ShellHook: "required"}, + }, st) + + if len(suggestions) != 1 { + t.Fatalf("len(Suggestions()) = %d, want 1 for previously suggested", len(suggestions)) + } +} + +func TestApplyReconcilesToAppliedWhenLineAlreadyExists(t *testing.T) { + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + t.Setenv("SHELL", "/bin/bash") + + rcFile := filepath.Join(homeDir, ".bashrc") + initLine := `eval "$(direnv hook bash)"` + + // Pre-populate the rc file with the init line already present. + //nolint:gosec // Test fixture written to a temp directory. + if err := os.WriteFile(rcFile, []byte(initLine+"\n"), 0o644); err != nil { + t.Fatalf("os.WriteFile() error = %v", err) + } + + st := state.State{Version: 1, Tools: map[string]state.ToolState{ + "direnv": {ToolID: "direnv", ShellHookStatus: shellHookSuggested}, + }} + + suggestions := []Suggestion{{ + ToolID: "direnv", + ToolName: "direnv", + Shell: "bash", + RCFile: rcFile, + InitLine: initLine, + Required: true, + }} + + if err := ApplyConfirmedSuggestions(suggestions, &st); err != nil { + t.Fatalf("ApplyConfirmedSuggestions() error = %v", err) + } + + // File should still have exactly one copy of the line. + //nolint:gosec // The rc file path is derived from the test temp HOME. + data, err := os.ReadFile(rcFile) + if err != nil { + t.Fatalf("os.ReadFile(%q) error = %v", rcFile, err) + } + + if strings.Count(string(data), initLine) != 1 { + t.Fatalf("rc file contents = %q, want exactly one init line", string(data)) + } + + // State must be reconciled to "applied" even though the line already existed. + receipt, ok := st.Tool("direnv") + if !ok { + t.Fatal("state.Tool(\"direnv\") did not find a receipt") + } + + if receipt.ShellHookStatus != shellHookApplied { + t.Fatalf("receipt.ShellHookStatus = %q, want %q", receipt.ShellHookStatus, shellHookApplied) + } +} + func TestApplyConfirmedSuggestionsIsIdempotent(t *testing.T) { t.Setenv("HOME", t.TempDir()) t.Setenv("SHELL", "/bin/bash")