Skip to content

fix: resolve relative paths against CWD when parentDir is empty#2221

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/docker-agent-issue-1655-deep-analysis-aac731b8
Mar 23, 2026
Merged

fix: resolve relative paths against CWD when parentDir is empty#2221
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/docker-agent-issue-1655-deep-analysis-aac731b8

Conversation

@dgageot
Copy link
Member

@dgageot dgageot commented Mar 23, 2026

Summary

When agent configs are loaded from OCI/URL/bytes sources, parentDir is empty. Previously, 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.

Changes

  • pkg/rag/manager.goGetAbsolutePaths(): fall back to filepath.Abs() when basePath is empty
  • pkg/rag/strategy/helpers.gomakeAbsolute() and ResolveDatabasePath(): same fix
  • Added debug logging to help diagnose path resolution issues
  • Added tests for all three functions covering both empty and non-empty parent dir scenarios

Non-breaking

When parentDir is non-empty (normal file-source case), behavior is unchanged. The fix only affects OCI/URL/bytes sources where parentDir was previously empty.

Fixes #1655

@dgageot dgageot requested a review from a team as a code owner March 23, 2026 08:09
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

Found 2 confirmed issues in the error handling logic for path resolution.

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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MEDIUM: Error handling violates function contract

When filepath.Abs(p) fails and parentDir is empty, the function returns the original relative path unchanged:

return p

This 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
@dgageot dgageot force-pushed the board/docker-agent-issue-1655-deep-analysis-aac731b8 branch from d66b5e7 to 40dd442 Compare March 23, 2026 08:29
@dgageot dgageot merged commit 6abb400 into docker:main Mar 23, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Relative paths incoherencies in agents definitions

2 participants