Skip to content

bump direct go dependencies#2709

Open
dgageot wants to merge 2 commits intodocker:mainfrom
dgageot:board/82860f53d9d3f864
Open

bump direct go dependencies#2709
dgageot wants to merge 2 commits intodocker:mainfrom
dgageot:board/82860f53d9d3f864

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 7, 2026

Module From To Status
github.com/go-git/go-git/v5 v5.18.0 v5.19.0 bumped (with minimal fix in pkg/fsx/vcs.go to resolve symlinks on absPath)
github.com/openai/openai-go/v3 v3.34.0 v3.35.0 bumped

Note on the go-git bump

go-git/v5 v5.19.0 pulls in go-billy/v5 v5.9.0, which now resolves symlinks in worktree.Filesystem.Root() (e.g. returns /private/var/... instead of /var/... on macOS). This caused VCSMatcher.ShouldIgnore to fail its strings.HasPrefix(absPath, m.repoRoot) check, breaking TestFilePickerShowIgnoredInGitRepo.

The minimal fix in pkg/fsx/vcs.go is to additionally call filepath.EvalSymlinks on absPath so it lines up with m.repoRoot.

Assisted-By: docker-agent

dgageot added 2 commits May 7, 2026 20:21
Resolve symlinks on the absolute path in VCSMatcher.ShouldIgnore so it matches m.repoRoot, which go-billy v5.9.0 (pulled in by go-git v5.19.0) now returns with symlinks resolved (e.g. /private/var/... instead of /var/... on macOS).

Assisted-By: docker-agent
@dgageot dgageot requested a review from a team as a code owner May 7, 2026 18:26
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Assessment: 🟢 APPROVE

The dependency bumps look correct. The filepath.EvalSymlinks fix in pkg/fsx/vcs.go is sound: since go-billy v5.9.0 already returns a symlink-resolved path from worktree.Filesystem.Root(), both m.repoRoot and absPath are consistently resolved, and the HasPrefix check works correctly on macOS. go.mod/go.sum are self-consistent.

Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Assessment: 🟢 APPROVE

Reviewed the dependency bumps (go-git/v5 v5.19.0, openai-go/v3 v3.35.0) and the symlink-resolution fix in pkg/fsx/vcs.go.

Fix correctness: The filepath.EvalSymlinks(absPath) addition is correct and well-targeted. Since go-billy/v5 v5.9.0 now returns a symlink-resolved path from worktree.Filesystem.Root(), applying the same resolution to absPath before the strings.HasPrefix check is the right approach. The m.repoRoot field is populated directly from Root() at construction time, so both sides of the comparison are consistently resolved.

One minor note (below the comment threshold): The EvalSymlinks error is silently discarded — if it fails for a transient reason on a valid file, the unresolved absPath would be compared against the resolved m.repoRoot, potentially causing ShouldIgnore to return false for a file that should be checked. In practice this is very low risk (the file exists and was just seen by the caller), but propagating or logging the error would make failures more visible.

Dependency changes: go.mod and go.sum look mechanically correct — hashes updated consistently with the bumps.

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.

2 participants