fix(host-service): tolerate [blob:none] suffix in git remote -v#4064
fix(host-service): tolerate [blob:none] suffix in git remote -v#4064AviPeltz wants to merge 1 commit into
[blob:none] suffix in git remote -v#4064Conversation
`git remote -v` appends a partial-clone marker (`[blob:none]`, `[treeless]`, etc.) after `(fetch)` whenever `remote.<name>.promisor` is set. The parser anchored on `(fetch)$`, so the line was silently dropped and `resolveLocalRepo` fell back to the alphabetically-first remote. For users who cloned with `--filter=blob:none`, the v2 add-folder flow detected the wrong remote and either rejected the folder or matched against the wrong cloud project.
📝 WalkthroughWalkthroughUpdated the regex in ChangesGit Remote Partial-Clone Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryFixes a silent remote-drop in Confidence Score: 5/5Safe to merge — minimal targeted regex fix with a focused regression test and no behavioural change for previously-valid input. The change is a single-character regex relaxation that is strictly a superset of the prior matches. The fix is well-reasoned, the comment is clear, and the new integration test reproduces the exact failure path. No existing tests are affected, and no new code paths are introduced. No files require special attention.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/project/utils/git-remote.ts | Regex anchor $ dropped in favour of `(?:\s |
| packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts | New integration regression test configures a real repo with remote.origin.promisor / remote.origin.partialclonefilter to reproduce the [blob:none] suffix and assert origin wins over the alphabetically-earlier aaa remote. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["git remote -v output line"] --> B{"line matches\n/^(\\S+)\\s+(\\S+)\\s+\\(fetch\\)(?:\\s|$)/"}
B -- "Yes (normal clone)\ne.g. origin git@github.com:Org/Repo.git (fetch)" --> C["remotes.set(name, url)"]
B -- "Yes (partial clone)\ne.g. origin git@github.com:Org/Repo.git (fetch) [blob:none]" --> C
B -- "No (push line)\ne.g. origin git@github.com:Org/Repo.git (push)" --> D["skip line"]
C --> E["getGitHubRemotes → filter GitHub remotes"]
E --> F{"has 'origin'?"}
F -- "Yes" --> G["return origin"]
F -- "No" --> H["return first GitHub remote"]
Reviews (1): Last reviewed commit: "fix(host-service): tolerate `[blob:none]..." | Re-trigger Greptile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts (1)
110-128: ⚡ Quick winStrengthen this regression test by asserting the partial-clone marker is present.
Right now the test proves origin preference, but it doesn’t prove the repo is in the exact
[blob:none]output shape that previously broke parsing. Add a quick assertion ongit.remote(["-v"])beforeresolveLocalReposo this test can’t silently collapse into a duplicate of the existing multi-remote case.Suggested test hardening
test("returns origin when it is configured as a partial clone (`[blob:none]` suffix)", async () => { @@ await git.raw(["config", "remote.origin.promisor", "true"]); await git.raw(["config", "remote.origin.partialclonefilter", "blob:none"]); + + const remoteV = await git.remote(["-v"]); + expect(remoteV).toMatch(/\borigin\b.*\(fetch\).*?\[blob:none\]/); const resolved = await resolveLocalRepo(repo);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts` around lines 110 - 128, The test needs to assert the presence of the partial-clone marker so it cannot regress into the multi-remote scenario: after configuring the repo (the calls to initRepoAt, git.addRemote and git.raw setting remote.origin.promisor and remote.origin.partialclonefilter) call git.remote(["-v"]) and assert the returned string/lines include the exact "[blob:none]" marker for the origin remote before invoking resolveLocalRepo; ensure you reference the same git instance used in the test and assert that the origin line contains both the origin URL and the "[blob:none]" suffix so the test is exercising the exact failing input shape for resolveLocalRepo.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts`:
- Around line 110-128: The test needs to assert the presence of the
partial-clone marker so it cannot regress into the multi-remote scenario: after
configuring the repo (the calls to initRepoAt, git.addRemote and git.raw setting
remote.origin.promisor and remote.origin.partialclonefilter) call
git.remote(["-v"]) and assert the returned string/lines include the exact
"[blob:none]" marker for the origin remote before invoking resolveLocalRepo;
ensure you reference the same git instance used in the test and assert that the
origin line contains both the origin URL and the "[blob:none]" suffix so the
test is exercising the exact failing input shape for resolveLocalRepo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a1e20e3-e819-4cd3-9f61-5a26e24cc6f6
📒 Files selected for processing (2)
packages/host-service/src/trpc/router/project/utils/git-remote.tspackages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Summary
git remote -vappends a partial-clone marker ([blob:none],[treeless], …) after(fetch)wheneverremote.<name>.promisoris set. The parser anchored on(fetch)$, so the line was silently dropped fromgetAllRemoteUrlsandresolveLocalRepofell back to the alphabetically-first remote.--filter=blob:none(very common for monorepos), the v2 add-folder flow detected the wrong remote URL and either rejected the folder or matched against an unrelated cloud project — e.g.origin: superset-sh/supersetwas dropped, leavingandreasasprou/supersetto win as alphabetically first.$end-of-line anchor and require(fetch)to be followed by whitespace or end-of-line. Strict superset of the prior matches; no input correctly parsed before is now mishandled.Test plan
bun test packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts— 24 pass"aaa"instead of"origin") and passes afterbun run lintclean--filter=blob:noneclone in the v2 desktop UI and confirmoriginis detectedSummary by cubic
Fixes remote URL parsing to handle partial-clone markers like
[blob:none]ingit remote -v, so we correctly detectoriginin the v2 add-folder flow. Prevents falling back to the alphabetically-first remote and picking the wrong project.(fetch)followed by whitespace or end-of-line, tolerating markers like[blob:none]and[treeless].originis kept when partial-clone markers are present.Written for commit defc69a. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes