Skip to content

Commit 4b720df

Browse files
committed
Improvements
Signed-off-by: David Gageot <david.gageot@docker.com>
1 parent e5c8b26 commit 4b720df

4 files changed

Lines changed: 101 additions & 112 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ models:
127127

128128
#### Testing Best Practices
129129

130-
This project uses `github.com/stretchr/testify` for assertions.
130+
This project uses `github.com/stretchr/testify` for assertions.
131131

132132
In Go tests, always prefer `require` and `assert` from the `testify` package over manual error handling.
133133

cmd/root/alias.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func newAliasAddCmd() *cobra.Command {
4545
Use: "add <alias-name> <agent-path>",
4646
Short: "Add a new alias",
4747
Args: cobra.ExactArgs(2),
48-
RunE: func(cmd *cobra.Command, args []string) error {
48+
RunE: func(_ *cobra.Command, args []string) error {
4949
telemetry.TrackCommand("alias", append([]string{"add"}, args...))
5050
return createAlias(args[0], args[1])
5151
},
@@ -59,7 +59,7 @@ func newAliasListCmd() *cobra.Command {
5959
Aliases: []string{"ls"},
6060
Short: "List all registered aliases",
6161
Args: cobra.NoArgs,
62-
RunE: func(cmd *cobra.Command, args []string) error {
62+
RunE: func(*cobra.Command, []string) error {
6363
telemetry.TrackCommand("alias", []string{"list"})
6464
return listAliases()
6565
},
@@ -73,7 +73,7 @@ func newAliasRemoveCmd() *cobra.Command {
7373
Aliases: []string{"rm"},
7474
Short: "Remove a registered alias",
7575
Args: cobra.ExactArgs(1),
76-
RunE: func(cmd *cobra.Command, args []string) error {
76+
RunE: func(_ *cobra.Command, args []string) error {
7777
telemetry.TrackCommand("alias", append([]string{"remove"}, args...))
7878
return removeAlias(args[0])
7979
},
@@ -89,14 +89,13 @@ func createAlias(name, agentPath string) error {
8989
}
9090

9191
// Expand tilde in path if it's a local file path
92-
if strings.HasPrefix(agentPath, "~/") {
93-
if abs, err := expandTilde(agentPath); err == nil {
94-
agentPath = abs
95-
}
92+
absAgentPath, err := expandTilde(agentPath)
93+
if err != nil {
94+
return err
9695
}
9796

9897
// Store the alias
99-
s.Set(name, agentPath)
98+
s.Set(name, absAgentPath)
10099

101100
// Save to file
102101
if err := s.Save(); err != nil {
@@ -105,7 +104,7 @@ func createAlias(name, agentPath string) error {
105104

106105
fmt.Printf("Alias '%s' created successfully\n", name)
107106
fmt.Printf(" Alias: %s\n", name)
108-
fmt.Printf(" Agent: %s\n", agentPath)
107+
fmt.Printf(" Agent: %s\n", absAgentPath)
109108
fmt.Printf("\nYou can now run: cagent run %s\n", name)
110109

111110
return nil

pkg/aliases/aliases.go

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,49 +11,42 @@ import (
1111
)
1212

1313
// Aliases represents the aliases configuration
14-
type Aliases struct {
15-
Aliases map[string]string `yaml:"aliases"`
16-
}
14+
type Aliases map[string]string
1715

18-
// GetAliasesFilePath returns the path to the aliases file
19-
func GetAliasesFilePath() string {
16+
func aliasesFilePath() string {
2017
return filepath.Join(paths.GetConfigDir(), "aliases.yaml")
2118
}
2219

2320
// Load loads aliases from the configuration file
2421
func Load() (*Aliases, error) {
25-
return LoadFrom(GetAliasesFilePath())
22+
return loadFrom(aliasesFilePath())
2623
}
2724

28-
// LoadFrom loads aliases from a specific file path
29-
func LoadFrom(path string) (*Aliases, error) {
25+
// loadFrom loads aliases from a specific file path
26+
func loadFrom(path string) (*Aliases, error) {
3027
data, err := os.ReadFile(path)
3128
if err != nil {
3229
if os.IsNotExist(err) {
33-
return &Aliases{Aliases: make(map[string]string)}, nil
30+
return &Aliases{}, nil
3431
}
3532
return nil, fmt.Errorf("failed to read aliases file: %w", err)
3633
}
3734

38-
var s Aliases
35+
s := Aliases{}
3936
if err := yaml.Unmarshal(data, &s); err != nil {
4037
return nil, fmt.Errorf("failed to parse aliases file: %w", err)
4138
}
4239

43-
if s.Aliases == nil {
44-
s.Aliases = make(map[string]string)
45-
}
46-
4740
return &s, nil
4841
}
4942

5043
// Save saves aliases to the configuration file
5144
func (s *Aliases) Save() error {
52-
return s.SaveTo(GetAliasesFilePath())
45+
return s.saveTo(aliasesFilePath())
5346
}
5447

55-
// SaveTo saves aliases to a specific file path
56-
func (s *Aliases) SaveTo(path string) error {
48+
// saveTo saves aliases to a specific file path
49+
func (s *Aliases) saveTo(path string) error {
5750
// Ensure directory exists
5851
dir := filepath.Dir(path)
5952
if err := os.MkdirAll(dir, 0o755); err != nil {
@@ -74,25 +67,25 @@ func (s *Aliases) SaveTo(path string) error {
7467

7568
// Get retrieves the agent path for an alias
7669
func (s *Aliases) Get(name string) (string, bool) {
77-
path, ok := s.Aliases[name]
70+
path, ok := (*s)[name]
7871
return path, ok
7972
}
8073

8174
// Set creates or updates an alias
8275
func (s *Aliases) Set(name, agentPath string) {
83-
s.Aliases[name] = agentPath
76+
(*s)[name] = agentPath
8477
}
8578

8679
// Delete removes an alias
8780
func (s *Aliases) Delete(name string) bool {
88-
if _, exists := s.Aliases[name]; exists {
89-
delete(s.Aliases, name)
81+
if _, exists := (*s)[name]; exists {
82+
delete(*s, name)
9083
return true
9184
}
9285
return false
9386
}
9487

9588
// List returns all aliases
9689
func (s *Aliases) List() map[string]string {
97-
return s.Aliases
90+
return *s
9891
}

pkg/aliases/aliases_test.go

Lines changed: 77 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -8,87 +8,84 @@ import (
88
"github.com/stretchr/testify/require"
99
)
1010

11-
func TestAliases(t *testing.T) {
11+
func TestAliases_Empty(t *testing.T) {
1212
tmpDir := t.TempDir()
1313
aliasesFile := filepath.Join(tmpDir, "aliases.yaml")
1414

15-
t.Run("Load empty aliases", func(t *testing.T) {
16-
s, err := LoadFrom(aliasesFile)
17-
require.NoError(t, err)
18-
assert.Empty(t, s.Aliases)
19-
})
20-
21-
t.Run("Set and Get alias", func(t *testing.T) {
22-
s, err := LoadFrom(aliasesFile)
23-
require.NoError(t, err)
24-
25-
s.Set("test", "agentcatalog/test-agent")
26-
27-
value, ok := s.Get("test")
28-
assert.True(t, ok)
29-
assert.Equal(t, "agentcatalog/test-agent", value)
30-
})
31-
32-
t.Run("Save and load aliases", func(t *testing.T) {
33-
s := &Aliases{
34-
Aliases: map[string]string{
35-
"code": "agentcatalog/notion-expert",
36-
"myagent": "/path/to/myagent.yaml",
37-
},
38-
}
39-
40-
err := s.SaveTo(aliasesFile)
41-
require.NoError(t, err)
42-
43-
loaded, err := LoadFrom(aliasesFile)
44-
require.NoError(t, err)
45-
46-
assert.Len(t, loaded.Aliases, 2)
47-
assert.Equal(t, "agentcatalog/notion-expert", loaded.Aliases["code"])
48-
assert.Equal(t, "/path/to/myagent.yaml", loaded.Aliases["myagent"])
49-
})
50-
51-
t.Run("Delete alias", func(t *testing.T) {
52-
s := &Aliases{
53-
Aliases: map[string]string{
54-
"code": "agentcatalog/notion-expert",
55-
"myagent": "/path/to/myagent.yaml",
56-
},
57-
}
58-
59-
deleted := s.Delete("code")
60-
assert.True(t, deleted)
61-
62-
_, ok := s.Get("code")
63-
assert.False(t, ok)
64-
65-
assert.Len(t, s.Aliases, 1)
66-
67-
deleted = s.Delete("nonexistent")
68-
assert.False(t, deleted)
69-
})
70-
71-
t.Run("List aliases", func(t *testing.T) {
72-
s := &Aliases{
73-
Aliases: map[string]string{
74-
"code": "agentcatalog/notion-expert",
75-
"myagent": "/path/to/myagent.yaml",
76-
},
77-
}
78-
79-
list := s.List()
80-
assert.Len(t, list, 2)
81-
assert.Equal(t, "agentcatalog/notion-expert", list["code"])
82-
})
83-
84-
t.Run("Get non-existent alias", func(t *testing.T) {
85-
s := &Aliases{
86-
Aliases: map[string]string{
87-
"code": "agentcatalog/notion-expert",
88-
},
89-
}
90-
91-
_, ok := s.Get("nonexistent")
92-
assert.False(t, ok)
93-
})
15+
s, err := loadFrom(aliasesFile)
16+
17+
require.NoError(t, err)
18+
assert.Empty(t, s)
19+
}
20+
21+
func TestAliases_SetGet(t *testing.T) {
22+
tmpDir := t.TempDir()
23+
aliasesFile := filepath.Join(tmpDir, "aliases.yaml")
24+
25+
s, err := loadFrom(aliasesFile)
26+
require.NoError(t, err)
27+
assert.Empty(t, s)
28+
29+
s.Set("test", "agentcatalog/test-agent")
30+
31+
value, ok := s.Get("test")
32+
assert.True(t, ok)
33+
assert.Equal(t, "agentcatalog/test-agent", value)
34+
}
35+
36+
func TestAliases_SaveAndLoad(t *testing.T) {
37+
tmpDir := t.TempDir()
38+
aliasesFile := filepath.Join(tmpDir, "aliases.yaml")
39+
40+
s := &Aliases{
41+
"code": "agentcatalog/notion-expert",
42+
"myagent": "/path/to/myagent.yaml",
43+
}
44+
45+
err := s.saveTo(aliasesFile)
46+
require.NoError(t, err)
47+
48+
loaded, err := loadFrom(aliasesFile)
49+
require.NoError(t, err)
50+
51+
assert.Len(t, *loaded, 2)
52+
assert.Equal(t, "agentcatalog/notion-expert", (*loaded)["code"])
53+
assert.Equal(t, "/path/to/myagent.yaml", (*loaded)["myagent"])
54+
}
55+
56+
func TestAliases_Delete(t *testing.T) {
57+
s := &Aliases{
58+
"code": "agentcatalog/notion-expert",
59+
"myagent": "/path/to/myagent.yaml",
60+
}
61+
62+
deleted := s.Delete("code")
63+
assert.True(t, deleted)
64+
65+
_, ok := s.Get("code")
66+
assert.False(t, ok)
67+
assert.Len(t, *s, 1)
68+
69+
deleted = s.Delete("nonexistent")
70+
assert.False(t, deleted)
71+
}
72+
73+
func TestAliases_List(t *testing.T) {
74+
s := &Aliases{
75+
"code": "agentcatalog/notion-expert",
76+
"myagent": "/path/to/myagent.yaml",
77+
}
78+
79+
list := s.List()
80+
assert.Len(t, list, 2)
81+
assert.Equal(t, "agentcatalog/notion-expert", list["code"])
82+
}
83+
84+
func TestAliases_GetUnknown(t *testing.T) {
85+
s := &Aliases{
86+
"code": "agentcatalog/notion-expert",
87+
}
88+
89+
_, ok := s.Get("nonexistent")
90+
assert.False(t, ok)
9491
}

0 commit comments

Comments
 (0)