From 273446d77a77db777117a044a61b1d74e06d0c1a Mon Sep 17 00:00:00 2001 From: Alex Metelli Date: Sat, 14 Mar 2026 13:58:06 +0800 Subject: [PATCH 1/2] Make shell-hook prompting and state tracking idempotent Suggestions() now accepts persisted state and skips tools whose hook status is already applied or declined. ApplyConfirmedSuggestions() unconditionally reconciles state to applied, even when the init line already exists in the rc file. Closes #26 Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 1 + cmd/atb/runtime.go | 2 +- internal/shell/shell.go | 17 ++++-- internal/shell/shell_test.go | 106 ++++++++++++++++++++++++++++++++++- 4 files changed, 118 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ba7005..ba1c32a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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/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") From 4d6c9c1801964a235bd23504fdef6bd93a411d53 Mon Sep 17 00:00:00 2001 From: Alex Metelli Date: Sat, 14 Mar 2026 14:00:29 +0800 Subject: [PATCH 2/2] Pin verification tool versions used by Makefile and CI Define staticcheck, golangci-lint, and govulncheck versions as Makefile variables instead of using @latest. CI now runs make verify directly, resolving the same pinned versions as local development. Closes #27 Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/ci.yml | 9 --------- CHANGELOG.md | 1 + Makefile | 12 ++++++++---- 3 files changed, 9 insertions(+), 13 deletions(-) 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 ba1c32a..b634e83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ 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. 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)