fix: resolve relative paths against CWD when parentDir is empty#2221
Conversation
| abs, err := filepath.Abs(p) | ||
| if err != nil { | ||
| slog.Warn("Failed to resolve absolute path, using as-is", "path", p, "error", err) | ||
| absPaths = append(absPaths, p) |
There was a problem hiding this comment.
🟡 MEDIUM: Error handling violates function contract
When filepath.Abs(p) fails, the code falls back to appending the original relative path unchanged:
absPaths = append(absPaths, p)This violates the function's contract — GetAbsolutePaths promises to return absolute paths, but can silently return relative paths when filepath.Abs fails. Downstream code expecting absolute paths may encounter file-not-found errors or incorrect path comparisons.
Recommendation: Return an error instead of silently degrading to relative paths, or document this fallback behavior explicitly in the function comment and ensure all callers handle it.
| abs, err := filepath.Abs(p) | ||
| if err != nil { | ||
| slog.Warn("Failed to resolve absolute path, using as-is", "path", p, "error", err) | ||
| return p |
There was a problem hiding this comment.
🟡 MEDIUM: Error handling violates function contract
When filepath.Abs(p) fails and parentDir is empty, the function returns the original relative path unchanged:
return pThis violates the function's contract — makeAbsolute promises to make paths absolute, but can silently return relative paths when filepath.Abs fails. This is inconsistent with ResolveDatabasePath (which returns an error on failure) and could mask real problems like filesystem permission issues.
Recommendation: Either return an error (matching ResolveDatabasePath behavior) or document this fallback behavior. Consider whether callers can safely handle relative paths.
When agent configs are loaded from OCI/URL/bytes sources, parentDir is
empty. Previously, functions like GetAbsolutePaths, makeAbsolute, and
ResolveDatabasePath would produce broken relative paths via
filepath.Join("", path). Now they fall back to filepath.Abs() which
correctly resolves against the current working directory.
Added debug logging to help diagnose path resolution issues.
Fixes docker#1655
Assisted-By: docker-agent
d66b5e7 to
40dd442
Compare
Summary
When agent configs are loaded from OCI/URL/bytes sources,
parentDiris empty. Previously,GetAbsolutePaths,makeAbsolute, andResolveDatabasePathwould produce broken relative paths viafilepath.Join("", path). Now they fall back tofilepath.Abs()which correctly resolves against the current working directory.Changes
pkg/rag/manager.go—GetAbsolutePaths(): fall back tofilepath.Abs()whenbasePathis emptypkg/rag/strategy/helpers.go—makeAbsolute()andResolveDatabasePath(): same fixNon-breaking
When
parentDiris non-empty (normal file-source case), behavior is unchanged. The fix only affects OCI/URL/bytes sources whereparentDirwas previously empty.Fixes #1655