Skip to content

Commit 701399c

Browse files
fabriciojsclaude
andcommitted
refactor(parser): consolidate kool.yml discovery into a single source of truth
The "look for kool.yml, then kool.yaml" lookup had six independent implementations that had drifted apart: core/parser used path.Join (producing forward-slash paths on Windows) while everyone else used filepath.Join, the proxy manager stat'd before parsing while the environment package used a parse error as its discovery signal, and core/environment/workspace.go hand-unrolled the loop into two sequential ParseKoolYaml calls. Add parser.FindKoolYaml and parser.LoadKoolYaml, and route every call site through them. LoadKoolYaml returns ErrKoolYmlNotFound only when no config file exists, so a malformed config is now distinguishable from a missing one at the API level. initProxy now receives the already-parsed config instead of re-reading the same file from disk, dropping one of the three kool.yml parses that happened on every environment init. Error handling at each call site is intentionally unchanged, keeping this refactor behavior-neutral. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4430923 commit 701399c

9 files changed

Lines changed: 154 additions & 64 deletions

File tree

commands/workspace.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"kool-dev/kool/core/environment"
77
"kool-dev/kool/core/parser"
88
"kool-dev/kool/core/shell"
9-
"path/filepath"
109
"sort"
1110
"strings"
1211
)
@@ -35,15 +34,13 @@ func configuredWorkspaceServices(env environment.EnvStorage) []string {
3534
if services := workspaceServices(env); len(services) > 0 {
3635
return services
3736
}
38-
for _, name := range []string{"kool.yml", "kool.yaml"} {
39-
config, err := parser.ParseKoolYaml(filepath.Join(env.Get("PWD"), name))
40-
if err == nil {
41-
services := append([]string(nil), config.Workspaces...)
42-
sort.Strings(services)
43-
return services
44-
}
37+
config, err := parser.LoadKoolYaml(env.Get("PWD"))
38+
if err != nil {
39+
return nil
4540
}
46-
return nil
41+
services := append([]string(nil), config.Workspaces...)
42+
sort.Strings(services)
43+
return services
4744
}
4845

4946
func sourceProject(env environment.EnvStorage) string {

core/environment/env.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func InitEnvironmentVariables(envStorage EnvStorage) {
6868
if config != nil && config.Proxy != nil {
6969
envStorage.Set("KOOL_PROXY_ENABLED", "true")
7070
initSourceProject(envStorage, workDir)
71-
initProxy(envStorage, workDir)
71+
initProxy(envStorage, config)
7272
}
7373

7474
// Now that we loaded up the files, we will check for
@@ -110,14 +110,15 @@ func envKeySet(entries []string) map[string]bool {
110110
return keys
111111
}
112112

113+
// loadKoolConfig decodes the kool config file for the given directory, or
114+
// returns nil when there is none - or when it cannot be decoded, since
115+
// environment setup runs before we have any means of reporting the failure.
113116
func loadKoolConfig(workDir string) *parser.KoolYaml {
114-
for _, name := range []string{"kool.yml", "kool.yaml"} {
115-
config, err := parser.ParseKoolYaml(filepath.Join(workDir, name))
116-
if err == nil {
117-
return config
118-
}
117+
config, err := parser.LoadKoolYaml(workDir)
118+
if err != nil {
119+
return nil
119120
}
120-
return nil
121+
return config
121122
}
122123

123124
func initSourceProject(envStorage EnvStorage, workDir string) {

core/environment/proxy.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,13 @@ package environment
33
import (
44
"kool-dev/kool/core/parser"
55
"os"
6-
"path/filepath"
76
"strings"
87

98
"github.com/compose-spec/compose-go/template"
109
)
1110

12-
func initProxy(envStorage EnvStorage, workDir string) {
13-
var config *parser.KoolYaml
14-
var err error
15-
for _, name := range []string{"kool.yml", "kool.yaml"} {
16-
if config, err = parser.ParseKoolYaml(filepath.Join(workDir, name)); err == nil {
17-
break
18-
}
19-
}
20-
if err != nil || config.Proxy == nil {
11+
func initProxy(envStorage EnvStorage, config *parser.KoolYaml) {
12+
if config == nil || config.Proxy == nil {
2113
return
2214
}
2315

core/environment/proxy_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func TestInitProxySourceHost(t *testing.T) {
1313
}
1414
env := NewFakeEnvStorage()
1515

16-
initProxy(env, workDir)
16+
initProxy(env, loadKoolConfig(workDir))
1717

1818
if got := env.Get("KOOL_PROXY_DOMAIN"); got != "exlink.localhost" {
1919
t.Errorf("expected proxy domain, got %q", got)
@@ -32,7 +32,7 @@ func TestInitProxyWorkspaceHost(t *testing.T) {
3232
env.Set("KOOL_WORKSPACE", "true")
3333
env.Set("KOOL_WORKSPACE_NAME", "vite-smoke")
3434

35-
initProxy(env, workDir)
35+
initProxy(env, loadKoolConfig(workDir))
3636

3737
if got := env.Get("KOOL_PROXY_HOST"); got != "vite-smoke.workspace.exlink.localhost" {
3838
t.Errorf("expected workspace proxy host, got %q", got)
@@ -49,7 +49,7 @@ func TestInitProxyGitWorktreeHost(t *testing.T) {
4949
env.Set("KOOL_WORKSPACE_NAME", "vite-smoke")
5050
env.Set("KOOL_WORKSPACE_PROVIDER", "worktree")
5151

52-
initProxy(env, workDir)
52+
initProxy(env, loadKoolConfig(workDir))
5353

5454
if got := env.Get("KOOL_PROXY_HOST"); got != "vite-smoke.workspace.exlink.localhost" {
5555
t.Errorf("expected Git worktree proxy host, got %q", got)

core/environment/workspace.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package environment
33
import (
44
"crypto/sha256"
55
"fmt"
6-
"kool-dev/kool/core/parser"
76
"os"
87
"path/filepath"
98
"sort"
@@ -96,11 +95,8 @@ type workspaceComposeConfig struct {
9695
}
9796

9897
func initWorkspaceCompose(envStorage EnvStorage, workDir string) []string {
99-
koolConfig, err := parser.ParseKoolYaml(filepath.Join(workDir, "kool.yml"))
100-
if err != nil {
101-
koolConfig, err = parser.ParseKoolYaml(filepath.Join(workDir, "kool.yaml"))
102-
}
103-
if err != nil || len(koolConfig.Workspaces) == 0 {
98+
koolConfig := loadKoolConfig(workDir)
99+
if koolConfig == nil || len(koolConfig.Workspaces) == 0 {
104100
return nil
105101
}
106102
unique := make(map[string]bool, len(koolConfig.Workspaces))

core/parser/discover.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package parser
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
)
7+
8+
// koolYamlFileNames lists the accepted kool config file names, in lookup order.
9+
var koolYamlFileNames = []string{"kool.yml", "kool.yaml"}
10+
11+
// FindKoolYaml returns the path of the kool config file within the given
12+
// directory, preferring kool.yml over kool.yaml. It returns ErrKoolYmlNotFound
13+
// when neither file exists.
14+
func FindKoolYaml(dir string) (file string, err error) {
15+
for _, name := range koolYamlFileNames {
16+
candidate := filepath.Join(dir, name)
17+
if _, err = os.Stat(candidate); err == nil {
18+
return candidate, nil
19+
}
20+
}
21+
22+
return "", ErrKoolYmlNotFound
23+
}
24+
25+
// LoadKoolYaml finds and decodes the kool config file within the given
26+
// directory. It returns ErrKoolYmlNotFound when no config file exists; any
27+
// other error means a file was found but could not be decoded, so callers can
28+
// tell a malformed config apart from a missing one.
29+
func LoadKoolYaml(dir string) (parsed *KoolYaml, err error) {
30+
var file string
31+
32+
if file, err = FindKoolYaml(dir); err != nil {
33+
return
34+
}
35+
36+
return ParseKoolYaml(file)
37+
}

core/parser/discover_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package parser
2+
3+
import (
4+
"errors"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
)
9+
10+
func TestFindKoolYamlPrefersYml(t *testing.T) {
11+
dir := t.TempDir()
12+
for _, name := range []string{"kool.yml", "kool.yaml"} {
13+
if err := os.WriteFile(filepath.Join(dir, name), []byte("scripts: {}\n"), 0644); err != nil {
14+
t.Fatal(err)
15+
}
16+
}
17+
18+
file, err := FindKoolYaml(dir)
19+
if err != nil {
20+
t.Fatalf("unexpected error: %v", err)
21+
}
22+
23+
if expected := filepath.Join(dir, "kool.yml"); file != expected {
24+
t.Errorf("expected %q, got %q", expected, file)
25+
}
26+
}
27+
28+
func TestFindKoolYamlFallsBackToYaml(t *testing.T) {
29+
dir := t.TempDir()
30+
if err := os.WriteFile(filepath.Join(dir, "kool.yaml"), []byte("scripts: {}\n"), 0644); err != nil {
31+
t.Fatal(err)
32+
}
33+
34+
file, err := FindKoolYaml(dir)
35+
if err != nil {
36+
t.Fatalf("unexpected error: %v", err)
37+
}
38+
39+
if expected := filepath.Join(dir, "kool.yaml"); file != expected {
40+
t.Errorf("expected %q, got %q", expected, file)
41+
}
42+
}
43+
44+
func TestFindKoolYamlNotFound(t *testing.T) {
45+
if _, err := FindKoolYaml(t.TempDir()); !errors.Is(err, ErrKoolYmlNotFound) {
46+
t.Errorf("expected ErrKoolYmlNotFound, got %v", err)
47+
}
48+
}
49+
50+
func TestLoadKoolYaml(t *testing.T) {
51+
dir := t.TempDir()
52+
if err := os.WriteFile(filepath.Join(dir, "kool.yml"), []byte("workspaces:\n - app\n"), 0644); err != nil {
53+
t.Fatal(err)
54+
}
55+
56+
parsed, err := LoadKoolYaml(dir)
57+
if err != nil {
58+
t.Fatalf("unexpected error: %v", err)
59+
}
60+
61+
if len(parsed.Workspaces) != 1 || parsed.Workspaces[0] != "app" {
62+
t.Errorf("expected workspaces [app], got %v", parsed.Workspaces)
63+
}
64+
}
65+
66+
func TestLoadKoolYamlMissing(t *testing.T) {
67+
if _, err := LoadKoolYaml(t.TempDir()); !errors.Is(err, ErrKoolYmlNotFound) {
68+
t.Errorf("expected ErrKoolYmlNotFound, got %v", err)
69+
}
70+
}
71+
72+
// A malformed config must be distinguishable from a missing one, so callers
73+
// can choose to report it rather than silently behaving as if unconfigured.
74+
func TestLoadKoolYamlMalformed(t *testing.T) {
75+
dir := t.TempDir()
76+
if err := os.WriteFile(filepath.Join(dir, "kool.yml"), []byte("scripts: [oops\n"), 0644); err != nil {
77+
t.Fatal(err)
78+
}
79+
80+
_, err := LoadKoolYaml(dir)
81+
if err == nil {
82+
t.Fatal("expected a decoding error, got none")
83+
}
84+
85+
if errors.Is(err, ErrKoolYmlNotFound) {
86+
t.Error("malformed config reported as not found")
87+
}
88+
}

core/parser/parser.go

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package parser
22

33
import (
44
"errors"
5-
"os"
6-
"path"
75
"sort"
86
"strings"
97

@@ -37,25 +35,16 @@ func (p *DefaultParser) AddLookupPath(rootPath string) (err error) {
3735
p.lookedUp = make(map[string]bool)
3836
}
3937

40-
ymlPath := path.Join(rootPath, "kool.yml")
41-
yamlPath := path.Join(rootPath, "kool.yaml")
42-
43-
if _, err = os.Stat(ymlPath); err == nil {
44-
koolFile = ymlPath
45-
} else if _, err = os.Stat(yamlPath); err == nil {
46-
koolFile = yamlPath
38+
if koolFile, err = FindKoolYaml(rootPath); err != nil {
39+
return
4740
}
4841

49-
if koolFile == "" {
50-
err = ErrKoolYmlNotFound
51-
} else {
52-
if !p.lookedUp[koolFile] {
53-
p.targetFiles = append(p.targetFiles, koolFile)
54-
}
55-
56-
p.lookedUp[koolFile] = true
42+
if !p.lookedUp[koolFile] {
43+
p.targetFiles = append(p.targetFiles, koolFile)
5744
}
5845

46+
p.lookedUp[koolFile] = true
47+
5948
return
6049
}
6150

services/proxy/manager.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -289,20 +289,10 @@ func (m *DefaultManager) Trust() error {
289289
}
290290

291291
func (m *DefaultManager) loadConfig() (*config, error) {
292-
workDir := m.env.Get("PWD")
293-
var file string
294-
for _, name := range []string{"kool.yml", "kool.yaml"} {
295-
candidate := filepath.Join(workDir, name)
296-
if _, err := os.Stat(candidate); err == nil {
297-
file = candidate
298-
break
299-
}
300-
}
301-
if file == "" {
292+
parsed, err := parser.LoadKoolYaml(m.env.Get("PWD"))
293+
if errors.Is(err, parser.ErrKoolYmlNotFound) {
302294
return nil, nil
303295
}
304-
305-
parsed, err := parser.ParseKoolYaml(file)
306296
if err != nil || parsed.Proxy == nil {
307297
return nil, err
308298
}

0 commit comments

Comments
 (0)