feat(host-service): v2 workspace creation fallback to origin/main#3361
Conversation
When resolveStartPoint resolves to an origin/* ref, fetch just that single branch before creating the worktree to ensure we branch from the latest remote state. Fails gracefully if offline or auth expired. Also updates the design doc with cross-product comparison (VS Code, T3Code, GitHub Desktop) and documents the fetch strategy.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new workspace start-point resolver was added to determine the appropriate Git reference for v2 workspace creation. The resolver prefers remote-tracking refs, falls back to local refs, and ultimately uses HEAD. Default branch resolution is performed when baseBranch is not provided, and targeted fetches are attempted for remote-tracking refs during workspace creation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 SummaryThis PR improves V2 workspace creation by replacing the hardcoded Key changes:
The implementation is well-designed, with graceful error handling at every git call — a failed fetch falls back to the locally cached ref, and a failed Confidence Score: 4/5Safe to merge — logic is sound with graceful error handling at every step; two minor P2 suggestions remain. The core resolution logic is correct, well-tested across 7 scenarios, and fails gracefully at every git call. The only two non-blocking items are a stylistic inconsistency in the fetch API call (array vs. raw) and one missing test case for the HEAD fallback when no baseBranch is provided and no refs exist. Neither affects production correctness. No files require special attention; the minor style note in workspace-creation.ts is optional cleanup. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as UI/Renderer
participant WC as workspace-creation.ts
participant RSP as resolveStartPoint()
participant Git as simple-git
UI->>WC: create(baseBranch?, ...)
WC->>RSP: resolveStartPoint(git, baseBranch)
alt baseBranch provided
RSP->>Git: rev-parse --verify origin/<baseBranch>
alt origin/<baseBranch> exists
Git-->>RSP: OK
RSP-->>WC: ref=origin/<baseBranch>, resolvedFrom=remote-tracking
else not found
RSP->>Git: rev-parse --verify <baseBranch>
alt <baseBranch> exists locally
Git-->>RSP: OK
RSP-->>WC: ref=<baseBranch>, resolvedFrom=local
else not found
RSP-->>WC: ref=HEAD, resolvedFrom=fallback
end
end
else no baseBranch
RSP->>Git: symbolic-ref refs/remotes/origin/HEAD --short
alt symbolic-ref succeeds
Git-->>RSP: origin/main strips to main
else fails
RSP-->>RSP: default = main
end
RSP->>Git: rev-parse --verify origin/<default>
Note over RSP,Git: same 3-tier chain as above
RSP-->>WC: resolved ref
end
alt startPoint starts with origin/
WC->>Git: fetch origin <branch> --quiet --no-tags
alt fetch fails
Git-->>WC: error caught, logged as warning
end
end
WC->>Git: worktree add --no-track -b <branchName> <path> <startPoint>
Git-->>WC: worktree created
WC-->>UI: success
Reviews (1): Last reviewed commit: "feat(host-service): add targeted fetch b..." | Re-trigger Greptile |
| try { | ||
| await git.fetch(["origin", remoteBranch, "--quiet", "--no-tags"]); | ||
| } catch (err) { |
There was a problem hiding this comment.
Consider using
git.raw() for fetch consistency
git.fetch() in simple-git has an overload that accepts TaskOptions (a string[]), so passing ["origin", remoteBranch, "--quiet", "--no-tags"] as the first argument is interpreted as the options bag — not as positional remote / branch arguments. This means the resulting command should be git fetch origin <remoteBranch> --quiet --no-tags, which is what's intended.
However, using git.raw() is unambiguous and consistent with every other git call in this file (symbolic-ref, rev-parse, worktree add):
| try { | |
| await git.fetch(["origin", remoteBranch, "--quiet", "--no-tags"]); | |
| } catch (err) { | |
| try { | |
| await git.raw(["fetch", "origin", remoteBranch, "--quiet", "--no-tags"]); | |
| } catch (err) { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Keeping git.fetch() here — it is simple-git's typed API for this operation. The other git.raw() calls in this file are for commands without typed wrappers (worktree add, symbolic-ref, rev-parse with ^{commit}). Using the typed API when available is the more correct choice.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/host-service/src/trpc/router/workspace-creation/utils/resolve-start-point.ts (1)
24-35: Deduplicate default-branch resolution with existing helper.Line 24–35 duplicates logic that already exists in
packages/host-service/src/trpc/router/git/utils/git-helpers.ts(getDefaultBranchName). Reusing it will keep behavior centralized.♻️ Suggested refactor
import type { SimpleGit } from "simple-git"; +import { getDefaultBranchName } from "../../git/utils/git-helpers"; @@ async function resolveDefaultBranchName(git: SimpleGit): Promise<string> { - try { - const ref = await git.raw([ - "symbolic-ref", - "refs/remotes/origin/HEAD", - "--short", - ]); - return ref.trim().replace(/^origin\//, ""); - } catch { - return "main"; - } + return (await getDefaultBranchName(git)) ?? "main"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/utils/resolve-start-point.ts` around lines 24 - 35, The function resolveDefaultBranchName duplicates logic already implemented in getDefaultBranchName; replace resolveDefaultBranchName's implementation to call and return the existing getDefaultBranchName(git) helper (import it from packages/host-service/src/trpc/router/git/utils/git-helpers.ts) and remove the duplicated try/catch logic so branch-resolution behavior is centralized via getDefaultBranchName.packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts (1)
409-415: Consider documenting the project's minimum Git version requirement.The
--no-trackflag forgit worktree addhas been available since Git 2.16.0 (January 2018), so it's widely supported across modern environments and poses negligible runtime risk. However, the project doesn't document an explicit minimum Git version. Since the codebase relies on git operations through simple-git, consider adding a minimum Git version requirement (e.g., ≥ 2.16) to setup docs or a.tool-versionsfile to set expectations for contributors and deployments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts` around lines 409 - 415, The code uses git's worktree add with the "--no-track" flag in the git.raw call inside the workspace creation logic (the array including "worktree","add","--no-track","-b"), so add a documented minimum Git version requirement (recommend >= 2.16.0) to the repository setup: update README/CONTRIBUTING or add a .tool-versions entry to state Git >= 2.16.0 and mention it in the environment/setup section so contributors and CI are aware of the dependency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/host-service/test/resolve-start-point.test.ts`:
- Around line 1-2: Move the test for resolveStartPoint so it is co-located with
its source file resolve-start-point.ts: create/rename it to
resolve-start-point.test.ts in the same directory as resolve-start-point.ts,
update the import to load resolveStartPoint from the local module (e.g. import {
resolveStartPoint } from './resolve-start-point'), and remove any references to
the shared/top-level test directory so the test follows the project's
co-location convention.
In `@packages/host-service/WORKSPACE_CREATION_FALLBACK.md`:
- Around line 45-49: The markdown file triggers markdownlint MD040 because
several fenced code blocks lack language identifiers; update each referenced
fenced block (the blocks that contain the git snippets such as the one with "git
config: branch.<name>.gh-merge-base", the blocks at the ranges 62-66, 69-73,
88-92, 122-133, and 241-251) by adding an appropriate language hint (e.g.,
```text or ```bash) after the opening backticks so each fenced code block
declares its language identifier.
---
Nitpick comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/utils/resolve-start-point.ts`:
- Around line 24-35: The function resolveDefaultBranchName duplicates logic
already implemented in getDefaultBranchName; replace resolveDefaultBranchName's
implementation to call and return the existing getDefaultBranchName(git) helper
(import it from packages/host-service/src/trpc/router/git/utils/git-helpers.ts)
and remove the duplicated try/catch logic so branch-resolution behavior is
centralized via getDefaultBranchName.
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 409-415: The code uses git's worktree add with the "--no-track"
flag in the git.raw call inside the workspace creation logic (the array
including "worktree","add","--no-track","-b"), so add a documented minimum Git
version requirement (recommend >= 2.16.0) to the repository setup: update
README/CONTRIBUTING or add a .tool-versions entry to state Git >= 2.16.0 and
mention it in the environment/setup section so contributors and CI are aware of
the dependency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a46ca37d-a6e2-4c86-aeb1-aa1d82aa9039
📒 Files selected for processing (4)
packages/host-service/WORKSPACE_CREATION_FALLBACK.mdpackages/host-service/src/trpc/router/workspace-creation/utils/resolve-start-point.tspackages/host-service/src/trpc/router/workspace-creation/workspace-creation.tspackages/host-service/test/resolve-start-point.test.ts
| ``` | ||
| 1. git config: branch.<name>.gh-merge-base | ||
| 2. git symbolic-ref refs/remotes/<remote>/HEAD (remote default branch) | ||
| 3. Candidates ["main", "master"] — check local refs/heads/ then remote refs/remotes/ | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Line 45, Line 62, Line 69, Line 88, Line 122, and Line 241 start fenced blocks without a language, which triggers markdownlint MD040.
📝 Example fix pattern
-```
+```text
1. git config: branch.<name>.gh-merge-base
2. git symbolic-ref refs/remotes/<remote>/HEAD
3. Candidates ["main", "master"] ...
-```
+```Also applies to: 62-66, 69-73, 88-92, 122-133, 241-251
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/WORKSPACE_CREATION_FALLBACK.md` around lines 45 - 49,
The markdown file triggers markdownlint MD040 because several fenced code blocks
lack language identifiers; update each referenced fenced block (the blocks that
contain the git snippets such as the one with "git config:
branch.<name>.gh-merge-base", the blocks at the ranges 62-66, 69-73, 88-92,
122-133, and 241-251) by adding an appropriate language hint (e.g., ```text or
```bash) after the opening backticks so each fenced code block declares its
language identifier.
…lback case Moves the test next to its source per AGENTS.md co-location convention and adds a missing test for the cold-start HEAD fallback path (no baseBranch provided, symbolic-ref fails, no default branch exists).
…perset-sh#3361) * Workspace creation * feat(host-service): add targeted fetch before worktree creation When resolveStartPoint resolves to an origin/* ref, fetch just that single branch before creating the worktree to ensure we branch from the latest remote state. Fails gracefully if offline or auth expired. Also updates the design doc with cross-product comparison (VS Code, T3Code, GitHub Desktop) and documents the fetch strategy. * Lint * test(host-service): co-locate resolve-start-point test + add HEAD fallback case Moves the test next to its source per AGENTS.md co-location convention and adds a missing test for the cold-start HEAD fallback path (no baseBranch provided, symbolic-ref fails, no default branch exists).
Summary
resolveStartPointutility that prefersorigin/<branch>over local<branch>when creating worktrees, with HEAD as ultimate fallbackgit symbolic-refinstead of hardcoding"main"git fetch origin <branch>) for freshness without full-repo fetch overhead--no-trackto prevent new branches from auto-tracking the remote refTest plan
resolveStartPoint(7 tests passing)origin/mainresolved and fetchedSummary by cubic
Workspace creation now starts from the freshest branch by preferring
origin/<branch>or the repo’s remote default branch, with local andHEADfallbacks. It fetches only that ref before creating the worktree and disables upstream tracking to avoid accidental pushes.resolveStartPointto chooseorigin/<branch>→ local<branch>→HEAD; when nobaseBranch, detect default viagit symbolic-ref refs/remotes/origin/HEAD --shortwith a"main"fallback.origin/*, fetch just that ref (git fetch origin <branch> --quiet --no-tags) before creation; proceed gracefully if the fetch fails.--no-tracktogit worktree addto prevent auto-tracking.resolveStartPoint, including the cold-startHEADfallback case.Written for commit 3cc80ba. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests