Skip to content

Commit d4a55af

Browse files
author
razvan
committed
fix: address PR #35 review comments
- Log effective result.Source instead of hardcoded 'file_path' when nested_workspace_override is active (comment #1) - Add TestResolveFilePathNestedWorkspaceOverride to resolver_test.go verifying parent root, source, and reduced confidence (comment #2) - Use TrimRight on entryRoot before appending separator in FindParentWorkspace to prevent double separator on root paths like '/' or 'C:\' (comment #3)
1 parent a563f02 commit d4a55af

File tree

3 files changed

+34
-3
lines changed

3 files changed

+34
-3
lines changed

pkg/workspace/registry/registry.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,9 @@ func (r *Registry) FindParentWorkspace(path string) (string, bool) {
263263

264264
for _, entry := range r.entries {
265265
entryRoot := strings.ToLower(filepath.Clean(entry.Root))
266-
// The path must be strictly inside the entry root (not equal to it)
267-
prefix := entryRoot + string(filepath.Separator)
266+
// The path must be strictly inside the entry root (not equal to it).
267+
// TrimRight avoids double separator when entryRoot is "/" or "C:\".
268+
prefix := strings.TrimRight(entryRoot, string(filepath.Separator)) + string(filepath.Separator)
268269
if strings.HasPrefix(cleanPath, prefix) {
269270
// Pick the deepest (most specific) parent
270271
if len(entryRoot) > bestLen {

pkg/workspace/resolver/resolver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ func (r *Resolver) handleFilePath(ctx context.Context, path string) (*contract.R
185185
}
186186
}
187187

188-
r.log(ctx, "file_path", map[string]any{"root": result.Root, "source": "file_path"})
188+
r.log(ctx, "file_path", map[string]any{"root": result.Root, "source": result.Source})
189189
if result.Source == "" {
190190
result.Source = "file_path"
191191
}

pkg/workspace/resolver/resolver_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,32 @@ func TestResolveFilePath(t *testing.T) {
5656
}
5757
}
5858

59+
func TestResolveFilePathNestedWorkspaceOverride(t *testing.T) {
60+
// Detector finds /tmp/parent/nested as the workspace root (has its own .git)
61+
detector := &fakeDetector{candidate: &contract.WorkspaceCandidate{
62+
Root: "/tmp/parent/nested",
63+
Reason: contract.ReasonFilePath,
64+
}}
65+
// Registry knows /tmp/parent is an already-registered workspace
66+
reg := &fakeRegistry{parentRoot: "/tmp/parent"}
67+
r := New(Dependencies{Detector: detector, Registry: reg})
68+
req := contract.ResolveWorkspaceRequest{FilePath: "/tmp/parent/nested/src/main.py"}
69+
70+
resp, err := r.Resolve(context.Background(), req)
71+
if err != nil {
72+
t.Fatalf("unexpected error: %v", err)
73+
}
74+
if resp.ResolvedRoot != "/tmp/parent" {
75+
t.Fatalf("expected parent root /tmp/parent, got %s", resp.ResolvedRoot)
76+
}
77+
if resp.PathResolutionSource != "nested_workspace_override" {
78+
t.Fatalf("expected source nested_workspace_override, got %s", resp.PathResolutionSource)
79+
}
80+
if resp.PathResolutionConfidence >= 0.95 {
81+
t.Fatalf("expected reduced confidence (<0.95), got %f", resp.PathResolutionConfidence)
82+
}
83+
}
84+
5985
func TestResolveAlias(t *testing.T) {
6086
registry := &fakeRegistry{candidate: &contract.WorkspaceCandidate{Root: "/tmp/alias", Reason: contract.ReasonWorkspaceAlias}}
6187
r := New(Dependencies{Registry: registry})
@@ -235,6 +261,7 @@ type fakeRegistry struct {
235261
err *contract.ResolveWorkspaceError
236262
feedbackCount int
237263
promoteCount int
264+
parentRoot string // if set, FindParentWorkspace returns this
238265
}
239266

240267
func (f *fakeRegistry) ResolveAlias(ctx context.Context, alias string) (*contract.WorkspaceCandidate, *contract.ResolveWorkspaceError) {
@@ -268,6 +295,9 @@ func (f *fakeRegistry) GetActiveWorkspace() (string, error) {
268295
}
269296

270297
func (f *fakeRegistry) FindParentWorkspace(path string) (string, bool) {
298+
if f.parentRoot != "" {
299+
return f.parentRoot, true
300+
}
271301
return "", false
272302
}
273303

0 commit comments

Comments
 (0)