Merge pull request #2566 from dgageot/board/refactor-codebase-to-use-slices-package-16f36c45

Use the slices package to simplify slice operations

Author: dgageot

pkg/config/hooks.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"slices"
 	"strings"
 
 	"github.com/docker/docker-agent/pkg/config/latest"
@@ -10,55 +11,41 @@ import (
 // Each string is treated as a shell command to run.
 // Empty strings are silently skipped.
 func HooksFromCLI(preToolUse, postToolUse, sessionStart, sessionEnd, onUserInput []string) *latest.HooksConfig {
-	hooks := &latest.HooksConfig{}
-
-	if len(preToolUse) > 0 {
-		var defs []latest.HookDefinition
-		for _, cmd := range preToolUse {
-			if strings.TrimSpace(cmd) == "" {
-				continue
-			}
-			defs = append(defs, latest.HookDefinition{Type: "command", Command: cmd})
-		}
-		if len(defs) > 0 {
-			hooks.PreToolUse = []latest.HookMatcherConfig{{Hooks: defs}}
-		}
+	hooks := &latest.HooksConfig{
+		PreToolUse:   matcherFromCommands(preToolUse),
+		PostToolUse:  matcherFromCommands(postToolUse),
+		SessionStart: defsFromCommands(sessionStart),
+		SessionEnd:   defsFromCommands(sessionEnd),
+		OnUserInput:  defsFromCommands(onUserInput),
 	}
 
-	if len(postToolUse) > 0 {
-		var defs []latest.HookDefinition
-		for _, cmd := range postToolUse {
-			if strings.TrimSpace(cmd) == "" {
-				continue
-			}
-			defs = append(defs, latest.HookDefinition{Type: "command", Command: cmd})
-		}
-		if len(defs) > 0 {
-			hooks.PostToolUse = []latest.HookMatcherConfig{{Hooks: defs}}
-		}
+	if hooks.IsEmpty() {
+		return nil
 	}
+	return hooks
+}
 
-	for _, cmd := range sessionStart {
-		if strings.TrimSpace(cmd) != "" {
-			hooks.SessionStart = append(hooks.SessionStart, latest.HookDefinition{Type: "command", Command: cmd})
-		}
-	}
-	for _, cmd := range sessionEnd {
-		if strings.TrimSpace(cmd) != "" {
-			hooks.SessionEnd = append(hooks.SessionEnd, latest.HookDefinition{Type: "command", Command: cmd})
-		}
-	}
-	for _, cmd := range onUserInput {
-		if strings.TrimSpace(cmd) != "" {
-			hooks.OnUserInput = append(hooks.OnUserInput, latest.HookDefinition{Type: "command", Command: cmd})
+// defsFromCommands turns a list of CLI shell commands into hook definitions,
+// skipping any blank entries.
+func defsFromCommands(cmds []string) []latest.HookDefinition {
+	var defs []latest.HookDefinition
+	for _, cmd := range cmds {
+		if strings.TrimSpace(cmd) == "" {
+			continue
 		}
+		defs = append(defs, latest.HookDefinition{Type: "command", Command: cmd})
 	}
+	return defs
+}
 
-	if hooks.IsEmpty() {
+// matcherFromCommands wraps the result of defsFromCommands in a single
+// HookMatcherConfig so the commands apply to all tools (empty matcher).
+func matcherFromCommands(cmds []string) []latest.HookMatcherConfig {
+	defs := defsFromCommands(cmds)
+	if len(defs) == 0 {
 		return nil
 	}
-
-	return hooks
+	return []latest.HookMatcherConfig{{Hooks: defs}}
 }
 
 // MergeHooks merges CLI hooks into an existing HooksConfig.
@@ -74,18 +61,18 @@ func MergeHooks(base, cli *latest.HooksConfig) *latest.HooksConfig {
 	}
 
 	merged := &latest.HooksConfig{
-		PreToolUse:      append(append([]latest.HookMatcherConfig{}, base.PreToolUse...), cli.PreToolUse...),
-		PostToolUse:     append(append([]latest.HookMatcherConfig{}, base.PostToolUse...), cli.PostToolUse...),
-		SessionStart:    append(append([]latest.HookDefinition{}, base.SessionStart...), cli.SessionStart...),
-		TurnStart:       append(append([]latest.HookDefinition{}, base.TurnStart...), cli.TurnStart...),
-		BeforeLLMCall:   append(append([]latest.HookDefinition{}, base.BeforeLLMCall...), cli.BeforeLLMCall...),
-		AfterLLMCall:    append(append([]latest.HookDefinition{}, base.AfterLLMCall...), cli.AfterLLMCall...),
-		SessionEnd:      append(append([]latest.HookDefinition{}, base.SessionEnd...), cli.SessionEnd...),
-		OnUserInput:     append(append([]latest.HookDefinition{}, base.OnUserInput...), cli.OnUserInput...),
-		Stop:            append(append([]latest.HookDefinition{}, base.Stop...), cli.Stop...),
-		Notification:    append(append([]latest.HookDefinition{}, base.Notification...), cli.Notification...),
-		OnError:         append(append([]latest.HookDefinition{}, base.OnError...), cli.OnError...),
-		OnMaxIterations: append(append([]latest.HookDefinition{}, base.OnMaxIterations...), cli.OnMaxIterations...),
+		PreToolUse:      slices.Concat(base.PreToolUse, cli.PreToolUse),
+		PostToolUse:     slices.Concat(base.PostToolUse, cli.PostToolUse),
+		SessionStart:    slices.Concat(base.SessionStart, cli.SessionStart),
+		TurnStart:       slices.Concat(base.TurnStart, cli.TurnStart),
+		BeforeLLMCall:   slices.Concat(base.BeforeLLMCall, cli.BeforeLLMCall),
+		AfterLLMCall:    slices.Concat(base.AfterLLMCall, cli.AfterLLMCall),
+		SessionEnd:      slices.Concat(base.SessionEnd, cli.SessionEnd),
+		OnUserInput:     slices.Concat(base.OnUserInput, cli.OnUserInput),
+		Stop:            slices.Concat(base.Stop, cli.Stop),
+		Notification:    slices.Concat(base.Notification, cli.Notification),
+		OnError:         slices.Concat(base.OnError, cli.OnError),
+		OnMaxIterations: slices.Concat(base.OnMaxIterations, cli.OnMaxIterations),
 	}
 	return merged
 }

pkg/fsx/collect_test.go

@@ -3,7 +3,7 @@ package fsx
 import (
 	"os"
 	"path/filepath"
-	"sort"
+	"slices"
 	"strings"
 	"testing"
 
@@ -379,7 +379,7 @@ func TestCollectFiles_SortedOutput(t *testing.T) {
 
 	// Note: CollectFiles doesn't guarantee order, but results should be consistent
 	// Sort for comparison
-	sort.Strings(got)
+	slices.Sort(got)
 	assert.True(t, strings.HasSuffix(got[0], "a.go"))
 	assert.True(t, strings.HasSuffix(got[1], "m.go"))
 	assert.True(t, strings.HasSuffix(got[2], "z.go"))

pkg/hooks/builtins/add_directory_listing.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
-	"sort"
+	"slices"
 	"strings"
 
 	"github.com/docker/docker-agent/pkg/hooks"
@@ -56,7 +56,7 @@ func addDirectoryListing(_ context.Context, in *hooks.Input, _ []string) (*hooks
 	if len(names) == 0 {
 		return nil, nil
 	}
-	sort.Strings(names)
+	slices.Sort(names)
 
 	truncated := 0
 	if len(names) > maxDirectoryEntries {

pkg/hooks/handler.go

@@ -9,6 +9,8 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
+	"strings"
 	"sync"
 
 	"github.com/docker/docker-agent/pkg/shellpath"
@@ -150,27 +152,26 @@ func hookEnv(base []string, overrides map[string]string) []string {
 	if len(overrides) == 0 {
 		return base
 	}
-	env := append([]string{}, base...)
+	env := slices.Clone(base)
 	if len(env) == 0 {
 		env = os.Environ()
 	}
+
+	// Map each existing key to its position so overrides can replace in place.
 	index := make(map[string]int, len(env))
 	for i, entry := range env {
-		for j, ch := range entry {
-			if ch == '=' {
-				index[entry[:j]] = i
-				break
-			}
+		if key, _, ok := strings.Cut(entry, "="); ok {
+			index[key] = i
 		}
 	}
+
 	for key, value := range overrides {
 		entry := key + "=" + value
 		if i, ok := index[key]; ok {
 			env[i] = entry
-			continue
+		} else {
+			env = append(env, entry)
 		}
-		index[key] = len(env)
-		env = append(env, entry)
 	}
 	return env
 }

pkg/runtime/runtime_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"io"
 	"reflect"
+	"slices"
 	"strings"
 	"sync"
 	"testing"
@@ -2405,7 +2406,7 @@ func (r *recordingProvider) CreateChatCompletionStream(_ context.Context, _ []ch
 	defer r.mu.Unlock()
 
 	// Record the tool names for this call.
-	r.recordedCalls = append(r.recordedCalls, append([]tools.Tool{}, toolList...))
+	r.recordedCalls = append(r.recordedCalls, slices.Clone(toolList))
 
 	if r.callIdx >= len(r.streams) {
 		return newStreamBuilder().AddStopWithUsage(1, 1).Build(), nil

pkg/session/migrations_unit_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"errors"
+	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -156,7 +157,7 @@ func TestMigrationManager_CheckForUnknownMigrations_AllowsEqualOrOlderDB(t *test
 	require.NoError(t, NewMigrationManagerWithMigrations(db, migrations).checkForUnknownMigrations(t.Context()))
 
 	// Newer binary that knows about more migrations than the DB has applied: fine.
-	newer := append([]Migration{}, migrations...)
+	newer := slices.Clone(migrations)
 	newer = append(newer, Migration{ID: 2, Name: "002_b", UpSQL: `CREATE TABLE b (id INTEGER)`})
 	require.NoError(t, NewMigrationManagerWithMigrations(db, newer).checkForUnknownMigrations(t.Context()))
 }

pkg/tui/components/editor/editor.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"strings"
 	"time"
 
@@ -1506,7 +1507,7 @@ func (e *editor) removeLastNAttachments(n int) {
 		if !e.attachments[i].isTemp {
 			// Strip the placeholder text ("@/path/file.png ") that AttachFile inserted
 			value = strings.Replace(value, e.attachments[i].placeholder+" ", "", 1)
-			e.attachments = append(e.attachments[:i], e.attachments[i+1:]...)
+			e.attachments = slices.Delete(e.attachments, i, i+1)
 			removed++
 		}
 	}

pkg/tui/components/messages/messages.go

@@ -1151,9 +1151,9 @@ func (m *model) AddLoadingMessage(description string) tea.Cmd {
 func (m *model) ReplaceLoadingWithUser(content string, sessionPos int) tea.Cmd {
 	for i := len(m.messages) - 1; i >= 0; i-- {
 		if m.messages[i].Type == types.MessageTypeLoading {
-			m.messages = append(m.messages[:i], m.messages[i+1:]...)
+			m.messages = slices.Delete(m.messages, i, i+1)
 			if i < len(m.views) {
-				m.views = append(m.views[:i], m.views[i+1:]...)
+				m.views = slices.Delete(m.views, i, i+1)
 			}
 			m.invalidateAllItems()
 			break

pkg/tui/dialog/working_dir_picker.go

@@ -417,11 +417,8 @@ func (d *workingDirPickerDialog) cycleSectionBackward() { d.cycleSection(-1) }
 
 func (d *workingDirPickerDialog) cycleSection(delta int) {
 	n := len(dirPickerSectionOrder)
-	for i, s := range dirPickerSectionOrder {
-		if s == d.section {
-			d.section = dirPickerSectionOrder[(i+delta+n)%n]
-			break
-		}
+	if i := slices.Index(dirPickerSectionOrder, d.section); i >= 0 {
+		d.section = dirPickerSectionOrder[(i+delta+n)%n]
 	}
 	d.updateSectionFocus()
 }

pkg/tui/service/supervisor/supervisor.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"log/slog"
 	"path/filepath"
+	"slices"
 	"sync"
 
 	tea "charm.land/bubbletea/v2"
@@ -166,9 +167,7 @@ func (s *Supervisor) handleRuntimeEvent(sessionID string, msg tea.Msg) {
 	case *runtime.StreamStoppedEvent:
 		runner.IsRunning = false
 		runner.PendingEvent = nil // Clear any pending attention event since stream ended
-		if runner.NeedsAttn {
-			runner.NeedsAttn = false
-		}
+		runner.NeedsAttn = false
 		s.notifyTabsUpdated()
 
 	case *runtime.SessionTitleEvent:
@@ -234,12 +233,7 @@ func (s *Supervisor) buildTabInfoLocked() []messages.TabInfo {
 
 // activeIndexLocked returns the index of the active tab (must be called with lock held).
 func (s *Supervisor) activeIndexLocked() int {
-	for i, id := range s.order {
-		if id == s.activeID {
-			return i
-		}
-	}
-	return 0
+	return max(0, slices.Index(s.order, s.activeID))
 }
 
 // SwitchTo switches to a different session.
@@ -354,14 +348,14 @@ func (s *Supervisor) Spawner() SessionSpawner {
 }
 
 // CloseSession closes a session and removes it from the supervisor.
-func (s *Supervisor) CloseSession(sessionID string) (nextActiveID string) {
+func (s *Supervisor) CloseSession(sessionID string) string {
 	s.mu.Lock()
 
 	runner, ok := s.runners[sessionID]
 	if !ok {
-		nextActiveID = s.activeID
+		activeID := s.activeID
 		s.mu.Unlock()
-		return nextActiveID
+		return activeID
 	}
 
 	// Cancel the session context
@@ -375,35 +369,31 @@ func (s *Supervisor) CloseSession(sessionID string) (nextActiveID string) {
 
 	// Remove from order slice, remembering where it was.
 	closedIdx := 0
-	for i, id := range s.order {
-		if id == sessionID {
-			closedIdx = i
-			s.order = append(s.order[:i], s.order[i+1:]...)
-			break
-		}
+	if i := slices.Index(s.order, sessionID); i >= 0 {
+		closedIdx = i
+		s.order = slices.Delete(s.order, i, i+1)
 	}
 
 	// If this was the active session, switch to the previous tab (or the
 	// first one when closing the first tab).
 	if s.activeID == sessionID {
-		if len(s.order) > 0 {
-			prevIdx := max(closedIdx-1, 0)
-			s.activeID = s.order[prevIdx]
-		} else {
+		if len(s.order) == 0 {
 			s.activeID = ""
+		} else {
+			s.activeID = s.order[max(closedIdx-1, 0)]
 		}
 	}
 
 	s.notifyTabsUpdated()
-	nextActiveID = s.activeID
+	activeID := s.activeID
 	s.mu.Unlock()
 
 	// Run cleanup outside the lock so it can't deadlock.
 	if cleanup != nil {
 		go cleanup()
 	}
 
-	return nextActiveID
+	return activeID
 }
 
 // Count returns the number of sessions.
@@ -430,8 +420,8 @@ func (s *Supervisor) ReorderTab(fromIdx, toIdx int) {
 	}
 
 	id := s.order[fromIdx]
-	s.order = append(s.order[:fromIdx], s.order[fromIdx+1:]...)
-	s.order = append(s.order[:toIdx], append([]string{id}, s.order[toIdx:]...)...)
+	s.order = slices.Delete(s.order, fromIdx, fromIdx+1)
+	s.order = slices.Insert(s.order, toIdx, id)
 	s.notifyTabsUpdated()
 }