Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion cmd/atb/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
17 changes: 11 additions & 6 deletions internal/shell/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand Down
106 changes: 105 additions & 1 deletion internal/shell/shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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")
Expand Down
Loading