Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves Windows absolute-path support for “local” repositories across the UI and backend, aligning repository type detection with Windows drive/UNC path formats and normalizing MSYS/Git Bash-style paths for Windows runtime.
Changes:
- Add shared frontend URL/path classifier to treat Windows drive/UNC paths as local repositories.
- Update repository UI/form logic to hide branch for local repos and avoid prefixing
/for Windows paths. - Add backend helpers to detect Windows local paths and normalize MSYS-style paths before filesystem access, plus corresponding Go tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/views/project/Repositories.vue | Uses a shared helper to decide when to display the git branch (avoids treating Windows local paths as remote). |
| web/src/lib/repositoryUrl.js | Introduces shared repository URL/path classification and isLocalRepositoryPath. |
| web/src/components/RepositoryForm.vue | Reuses shared classifier for validation/type detection; avoids forcing / prefix for Windows drive/UNC paths. |
| util/local_path.go | Adds Windows path normalization for MSYS/Git Bash-style paths and Windows local-path detection. |
| util/local_path_test.go | Adds unit tests for Windows local-path detection and normalization behavior. |
| services/tasks/LocalJob.go | Uses normalized local repo path when checking existence (os.Stat). |
| db/Repository.go | Treats Windows drive/UNC paths as RepositoryLocal; normalizes local paths in GetGitURL. |
| db/Repository_test.go | Adds tests verifying Windows paths are classified as RepositoryLocal. |
| export function getRepositoryUrlType(url) { | ||
| if (url == null || url === '') { | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
web/src/lib/repositoryUrl.js adds new URL/path classification logic (including Windows drive and UNC handling), but there are no unit tests covering the expected cases. Since the repo already has Mocha unit tests for web/src/lib/*, please add tests for getRepositoryUrlType/isLocalRepositoryPath (e.g., /abs/path, D:\repo, D:/repo, \\server\\share, git@host:repo, https://..., unknown schemes).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14c06e3326
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
util/local_path.go
Outdated
| if p[0] == '\\' && p[1] == '\\' { | ||
| return len(p) > 2 | ||
| } | ||
| return isDriveLetter(p[0]) && p[1] == ':' |
There was a problem hiding this comment.
IsWindowsLocalRepositoryPath currently treats any string starting with <letter>: as a local repo path, so values like a://example/repo.git are misclassified as local instead of URL-like input. This is a regression from the previous behavior and causes invalid URLs to pass type checks (and skip branch requirements) until runtime, where local-path handling fails on os.Stat. Restrict the drive-letter rule to actual filesystem forms (e.g. ^[A-Za-z]:(\\|/|$)) to avoid classifying URI schemes as local paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Tightened
IsWindowsLocalRepositoryPathso drive-letter paths now requireX:followed by either end-of-string,\, or/, and reject non-filesystem forms likeD:repo. This prevents broad misclassification from just<letter>:. util/local_path.goL35-L57 - Added a guard to reject URL-like
a://...inputs while still accepting valid drive/UNC local path formats. util/local_path.goL53-L56 - Expanded unit coverage with regression tests for
D:repoanda://example/repo.gitto lock in the corrected behavior. util/local_path_test.goL14-L25
Testing
- ✅
go test ./util
Co-authored-by: Copilot <[email protected]>
…LocalRepositoryPath Co-authored-by: fiftin <[email protected]> Agent-Logs-Url: https://github.com/semaphoreui/semaphore/sessions/bbce1987-8c41-40ba-bd52-87b87cce5d48
No description provided.