feat(import): initialize git for non-git folders on v2 folder import#5036
Open
AviPeltz wants to merge 2 commits into
Open
feat(import): initialize git for non-git folders on v2 folder import#5036AviPeltz wants to merge 2 commits into
AviPeltz wants to merge 2 commits into
Conversation
Importing a folder that wasn't a git repo yet dead-ended on "Not a git repository". v2 now detects the case and offers to `git init` the folder (with user confirmation), then imports it as a local-only project — no remote required. Likely fixes #5033. Server (host-service): - resolve-repo: add tryRevParseGitRoot (non-throwing) and initLocalRepoInPlace (git init in place + empty initial commit; idempotent; nested dirs resolve to parent root). Export validateDirectoryPath; revParseGitRoot now wraps the non-throwing variant. - project.findByPath returns an additive optional `needsGitInit` instead of throwing on a non-git folder. - create importLocal gains `initIfNeeded`; createFromImportLocal inits in place only when set. Cloud path unchanged (local-only => no clone URL). Desktop: - New imperative git-init-confirm store + GitInitConfirmDialog, mounted once via AddRepositoryModals. useFolderFirstImport branches on needsGitInit, confirms, then creates with initIfNeeded. All 5 hook consumers untouched. Tests: 6 new resolve-repo cases; 2 new hook cases (confirm/cancel).
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDetect non-git folders server-side, prompt desktop user for git-init confirmation, and on confirmation initialize the folder and import the project (opt-in initIfNeeded path). ChangesGit Init Confirmation for Non-Git Folders
Sequence DiagramsequenceDiagram
participant User
participant Desktop as useFolderFirstImport
participant Store as GitInitConfirmStore
participant Dialog as GitInitConfirmDialog
participant Server as projectRouter
participant Handler as createFromImportLocal
User->>Desktop: select folder for import
Desktop->>Server: findByPath(folderPath)
Server-->>Desktop: needsGitInit=true
Desktop->>Store: request(repoPath)
Store->>Dialog: open(repoPath)
User->>Dialog: click Initialize & import
Dialog->>Store: resolve(true)
Store-->>Desktop: Promise<true>
Desktop->>Server: create(importLocal, initIfNeeded=true)
Server->>Handler: createFromImportLocal(repoPath, initIfNeeded=true)
Handler->>Handler: resolveOrInitLocalRepo
Handler-->>Server: ResolvedRepo
Server-->>Desktop: CreateResult
Desktop-->>User: import complete
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit 70b131e on Jun 2, 2026 7:54am UTC. |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Contributor
Greptile SummaryThis PR fixes a dead-end UX where importing a folder that isn't a git repository threw
Confidence Score: 4/5Safe to merge — the feature is well-encapsulated, opt-in, and idempotent; the only issue is a display-only path-splitting inconsistency on Windows. The detect-confirm-init flow is correctly structured: validation gates are in place before any filesystem writes, the TOCTOU re-check inside GitInitConfirmDialog.tsx — the folder name derivation should use
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/project/utils/resolve-repo.ts | Adds tryRevParseGitRoot (non-throwing), initLocalRepoInPlace (idempotent git-init in place + empty commit), and exports validateDirectoryPath; refactors revParseGitRoot to wrap the new non-throwing variant. Logic is solid and TOCTOU handling is correct. |
| packages/host-service/src/trpc/router/project/project.ts | findByPath now uses tryRevParseGitRoot, returning needsGitInit:true instead of throwing for non-git folders; still 400s on missing/non-dir paths via validateDirectoryPath. The importLocal schema gets initIfNeeded boolean flag. |
| packages/host-service/src/trpc/router/project/handlers.ts | Adds resolveOrInitLocalRepo helper and threads initIfNeeded through createFromImportLocal. Double tryRevParseGitRoot call is intentional for TOCTOU safety. Cloud path is correctly unchanged. |
| apps/desktop/src/renderer/stores/git-init-confirm.ts | New imperative zustand store for the git-init confirm dialog. Module-level pendingResolve pattern matches the existing add-repository-modal precedent; concurrent-call safety is documented and correctly implemented. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts | Branches on needsGitInit, awaits confirmation, then calls create with initIfNeeded:true. The create.mutate call sits inside the same try/catch as findByPath so PRECONDITION_FAILED errors are correctly surfaced via onError. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx | New confirm dialog driven by git-init-confirm store. Uses split('/').pop() for the folder display name, which is incorrect on Windows where paths use backslashes; getBaseName from pathBasename should be used instead. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx | Mounts GitInitConfirmDialog alongside NewProjectModal inside a Fragment; minimal and correct change. |
| packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts | Adds 6 integration tests for initLocalRepoInPlace covering plain folder, non-empty adoption, idempotency, nested subdir resolution, missing path, and file path. Identity env vars are correctly saved/restored. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.test.ts | Adds two test cases for the needsGitInit branch (confirm → create with initIfNeeded:true; cancel → null return). Mock plumbing is correct. |
Sequence Diagram
sequenceDiagram
participant U as User
participant UI as useFolderFirstImport
participant D as GitInitConfirmDialog
participant HS as host-service (findByPath)
participant HC as host-service (create)
participant FS as Filesystem
U->>UI: select folder
UI->>HS: "project.findByPath({ repoPath })"
alt path is a git repo
HS-->>UI: "{ candidates, cloudErrors }"
UI->>UI: existing import / setup flow
else path is NOT a git repo
HS->>HS: tryRevParseGitRoot → null
HS->>HS: validateDirectoryPath (400 if missing/not-dir)
HS-->>UI: "{ candidates:[], cloudErrors:[], needsGitInit:true }"
UI->>D: requestGitInit(repoPath)
D-->>U: Initialize git repository? dialog
alt User confirms
U->>D: "click Initialize & import"
D-->>UI: "confirmed = true"
UI->>HC: "project.create({ importLocal, initIfNeeded:true })"
HC->>HC: resolveOrInitLocalRepo
HC->>HC: tryRevParseGitRoot (TOCTOU re-check)
HC->>FS: "git init --initial-branch=main"
HC->>FS: git commit --allow-empty
HC->>HC: persistFromResolved (DB + cloud saga)
HC-->>UI: "{ projectId, repoPath, mainWorkspaceId }"
UI->>UI: finalizeSetup
else User cancels
U->>D: click Cancel / close
D-->>UI: "confirmed = false"
UI-->>U: return null (no-op)
end
end
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx:22
The folder name is derived using `split("/").pop()`, which silently falls back to the full path on Windows because Windows paths use backslashes (`C:\Users\me\myproject`.split("/") returns the entire string as one element). `useFolderFirstImport` already imports `getBaseName` from `renderer/lib/pathBasename` for the same job — use that here for consistent cross-platform behaviour.
```suggestion
const folderName = repoPath ? getBaseName(repoPath) : "this folder";
```
Reviews (1): Last reviewed commit: "feat(import): initialize git for non-git..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const repoPath = useGitInitConfirmStore((s) => s.repoPath); | ||
| const resolve = useGitInitConfirmStore((s) => s.resolve); | ||
|
|
||
| const folderName = repoPath?.split("/").pop() ?? repoPath ?? "this folder"; |
Contributor
There was a problem hiding this comment.
split("/").pop(), which silently falls back to the full path on Windows because Windows paths use backslashes (C:\Users\me\myproject.split("/") returns the entire string as one element). useFolderFirstImport already imports getBaseName from renderer/lib/pathBasename for the same job — use that here for consistent cross-platform behaviour.
Suggested change
| const folderName = repoPath?.split("/").pop() ?? repoPath ?? "this folder"; | |
| const folderName = repoPath ? getBaseName(repoPath) : "this folder"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx
Line: 22
Comment:
The folder name is derived using `split("/").pop()`, which silently falls back to the full path on Windows because Windows paths use backslashes (`C:\Users\me\myproject`.split("/") returns the entire string as one element). `useFolderFirstImport` already imports `getBaseName` from `renderer/lib/pathBasename` for the same job — use that here for consistent cross-platform behaviour.
```suggestion
const folderName = repoPath ? getBaseName(repoPath) : "this folder";
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx`:
- Line 22: The computed folderName currently does repoPath?.split("/").pop()
which yields an empty string for paths ending with a slash; update the logic to
normalize repoPath before extracting the basename (either trim trailing slashes
from repoPath or call the shared basename helper used elsewhere) so folderName
is never empty—use the normalized path (repoPathNormalized) when computing
folderName (keep the existing fallback to repoPath or "this folder") and update
the GitInitConfirmDialog component to reference the normalized value.
In `@packages/host-service/src/trpc/router/project/handlers.ts`:
- Around line 206-213: The change allows file paths to be converted into repos
because resolveOrInitLocalRepo calls tryRevParseGitRoot before validating the
original repoPath; fix resolveOrInitLocalRepo (and the analogous call at the
other site) by first verifying repoPath is a directory (e.g., fs.stat/exists +
isDirectory) and, if not a directory, immediately call
resolveLocalRepo(repoPath) so non-directory paths are rejected as before; only
if repoPath is a directory and initIfNeeded is true should you call
tryRevParseGitRoot and fall back to initLocalRepoInPlace(repoPath).
In `@packages/host-service/src/trpc/router/project/project.ts`:
- Around line 172-182: The code calls tryRevParseGitRoot(input.repoPath) before
validating that input.repoPath is a directory, which lets file paths inside a
repo be accepted; move or add the directory validation so the path is confirmed
a directory before probing Git. Specifically, invoke
validateDirectoryPath(input.repoPath, "Path") (or a direct fs.stat check for
directory) prior to calling tryRevParseGitRoot, and only call
tryRevParseGitRoot/resolveLocalRepo if the path is a directory; preserve the
existing return shape (candidates/cloudErrors/needsGitInit) when validation
fails.
In `@packages/host-service/src/trpc/router/project/utils/resolve-repo.ts`:
- Around line 103-110: The current tryRevParseGitRoot function swallows all
errors from createUserSimpleGit(path).revparse, which hides permission, broken
.git, or missing-git failures; change the catch to only return null when the
error is the canonical "not a git repository" case (e.g., error message/stderr
contains "not a git repository" case-insensitive), and rethrow any other errors
so callers can surface/init properly; reference the function tryRevParseGitRoot
and the revparse call on createUserSimpleGit(path) when making this change.
In `@plans/20260601-v2-import-git-init-non-git-folder.md`:
- Line 142: Change the prose instance of `discriminatedUnion` to either the
spaced phrase "discriminated union" or render it as an inline code identifier
(e.g., `discriminatedUnion`) for clarity and consistency; locate the token
"discriminatedUnion" in the sentence starting "The create modes are a
discriminatedUnion" and replace it with the preferred form.
🪄 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: 4b72c4cf-dfed-485e-be9d-150c4a9c01cb
📒 Files selected for processing (11)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.test.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.tsapps/desktop/src/renderer/stores/git-init-confirm.tspackages/host-service/src/trpc/router/project/handlers.tspackages/host-service/src/trpc/router/project/project.tspackages/host-service/src/trpc/router/project/utils/resolve-repo.test.tspackages/host-service/src/trpc/router/project/utils/resolve-repo.tsplans/20260601-v2-import-git-init-non-git-folder.md
Comment on lines
+206
to
+213
| async function resolveOrInitLocalRepo( | ||
| repoPath: string, | ||
| initIfNeeded: boolean, | ||
| ): Promise<ResolvedRepo> { | ||
| if (!initIfNeeded) return resolveLocalRepo(repoPath); | ||
| const root = await tryRevParseGitRoot(repoPath); | ||
| return root ? resolveLocalRepo(root) : initLocalRepoInPlace(repoPath); | ||
| } |
Contributor
There was a problem hiding this comment.
initIfNeeded now lets file paths slip through as repo imports.
Before this change, createFromImportLocal always went through resolveLocalRepo(repoPath), which rejected non-directories. With the new probe-first flow, passing /repo/src/file.ts plus initIfNeeded: true resolves /repo and imports it successfully. Please validate the original repoPath before tryRevParseGitRoot so the mutation stays folder-only.
Also applies to: 219-222
🤖 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/handlers.ts` around lines 206 -
213, The change allows file paths to be converted into repos because
resolveOrInitLocalRepo calls tryRevParseGitRoot before validating the original
repoPath; fix resolveOrInitLocalRepo (and the analogous call at the other site)
by first verifying repoPath is a directory (e.g., fs.stat/exists + isDirectory)
and, if not a directory, immediately call resolveLocalRepo(repoPath) so
non-directory paths are rejected as before; only if repoPath is a directory and
initIfNeeded is true should you call tryRevParseGitRoot and fall back to
initLocalRepoInPlace(repoPath).
Comment on lines
+172
to
+182
| const root = await tryRevParseGitRoot(input.repoPath); | ||
| if (root === null) { | ||
| validateDirectoryPath(input.repoPath, "Path"); // 400 on missing / not-a-dir | ||
| return { | ||
| candidates: [], | ||
| cloudErrors: [] as { url: string; message: string }[], | ||
| needsGitInit: true as const, | ||
| }; | ||
| } | ||
|
|
||
| const resolved = await resolveLocalRepo(root); |
Contributor
There was a problem hiding this comment.
Validate input.repoPath before probing Git.
git rev-parse --show-toplevel succeeds for file paths inside a worktree, so this branch now accepts something like /repo/src/index.ts and resolves the containing repo instead of rejecting it as "Path is not a directory". That changes the import contract from "pick a folder" to "pick anything inside a repo" and can import the wrong project from an accidental file selection.
🤖 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/project.ts` around lines 172 -
182, The code calls tryRevParseGitRoot(input.repoPath) before validating that
input.repoPath is a directory, which lets file paths inside a repo be accepted;
move or add the directory validation so the path is confirmed a directory before
probing Git. Specifically, invoke validateDirectoryPath(input.repoPath, "Path")
(or a direct fs.stat check for directory) prior to calling tryRevParseGitRoot,
and only call tryRevParseGitRoot/resolveLocalRepo if the path is a directory;
preserve the existing return shape (candidates/cloudErrors/needsGitInit) when
validation fails.
Comment on lines
+103
to
+110
| export async function tryRevParseGitRoot(path: string): Promise<string | null> { | ||
| try { | ||
| return ( | ||
| await createUserSimpleGit(path).revparse(["--show-toplevel"]) | ||
| ).trim(); | ||
| } catch { | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
Don't collapse every rev-parse failure into "not a repo".
This now turns permission errors, broken .git metadata, or a missing git executable into null, which makes callers treat the folder as safe to initialize. In particular, project.findByPath can surface needsGitInit for a real repo that just failed to probe, and initLocalRepoInPlace can reinitialize it instead of surfacing the underlying failure. Only swallow the canonical "not a git repository" case here; rethrow the rest.
🤖 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.ts` around
lines 103 - 110, The current tryRevParseGitRoot function swallows all errors
from createUserSimpleGit(path).revparse, which hides permission, broken .git, or
missing-git failures; change the catch to only return null when the error is the
canonical "not a git repository" case (e.g., error message/stderr contains "not
a git repository" case-insensitive), and rethrow any other errors so callers can
surface/init properly; reference the function tryRevParseGitRoot and the
revparse call on createUserSimpleGit(path) when making this change.
Comment on lines
+165
to
+179
| const existingRoot = await tryRevParseGitRoot(repoPath); | ||
| if (existingRoot) return resolveLocalRepo(existingRoot); | ||
|
|
||
| await gitInitMainBranch(repoPath); | ||
| try { | ||
| await createUserSimpleGit(repoPath).raw([ | ||
| "commit", | ||
| "--allow-empty", | ||
| "-m", | ||
| "Initial commit", | ||
| ]); | ||
| } catch (err) { | ||
| throw asInitialCommitTrpcError(err); | ||
| } | ||
| return resolveLocalRepo(repoPath); |
Contributor
There was a problem hiding this comment.
initLocalRepoInPlace is still race-prone.
The "already initialized" check and the empty commit are separate steps. If another import initializes the same folder after Line 165 but before Line 170, this path will re-run git init and append a second empty Initial commit, which breaks the idempotency promised in the docstring and mutates user history on a retry/race. This needs a lock or a revalidation flow that can detect "someone else initialized it" before creating a new commit.
| }), | ||
| ``` | ||
|
|
||
| > **Design tension (flag vs. separate mode).** The create modes are a discriminatedUnion |
Contributor
There was a problem hiding this comment.
Clarify wording: discriminatedUnion in prose reads like a typo.
In narrative text, prefer “discriminated union” (or wrap `discriminatedUnion` as a code identifier) for readability and consistency.
🧰 Tools
🪛 LanguageTool
[grammar] ~142-~142: Ensure spelling is correct
Context: ...eparate mode).** The create modes are a discriminatedUnion > where each kind has fixed init semant...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@plans/20260601-v2-import-git-init-non-git-folder.md` at line 142, Change the
prose instance of `discriminatedUnion` to either the spaced phrase
"discriminated union" or render it as an inline code identifier (e.g.,
`discriminatedUnion`) for clarity and consistency; locate the token
"discriminatedUnion" in the sentence starting "The create modes are a
discriminatedUnion" and replace it with the preferred form.
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plans/20260601-v2-import-git-init-non-git-folder.md">
<violation number="1" location="plans/20260601-v2-import-git-init-non-git-folder.md:89">
P2: The proposed non-throwing `tryRevParseGitRoot` swallows all errors, which can misclassify real operational failures as "non-git" and hide diagnostics.</violation>
</file>
<file name="packages/host-service/src/trpc/router/project/utils/resolve-repo.ts">
<violation number="1" location="packages/host-service/src/trpc/router/project/utils/resolve-repo.ts:165">
P2: There is a TOCTOU window between the `tryRevParseGitRoot` check (line 165) and the `gitInitMainBranch` + commit sequence. If a concurrent import initializes the same folder in between, this creates a duplicate "Initial commit", breaking the idempotency guarantee stated in the docstring. Consider rechecking after `gitInitMainBranch` (which is itself idempotent) whether a commit already exists before adding one.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Desktop UI
participant Store as GitInitConfirm Store
participant Dialog as GitInitConfirmDialog
participant Hook as useFolderFirstImport
participant Host as Host Service
participant Repo as Resolve-Repo Utils
participant FS as File System
Note over UI,FS: V2 Folder Import — Non-Git Folder Flow
Hook->>Host: project.findByPath.query({ repoPath })
Host->>Repo: tryRevParseGitRoot(repoPath)
Repo->>FS: git rev-parse --show-toplevel
FS-->>Repo: Error (not a git repo)
alt Not a git repo & path exists
Host->>Repo: validateDirectoryPath(repoPath)
Repo-->>Host: OK
Host-->>Hook: { candidates: [], cloudErrors: [], needsGitInit: true }
else Is a git repo
Host-->>Hook: { candidates: [...], cloudErrors: [] }
end
alt needsGitInit detected
Hook->>Store: request(repoPath)
Store-->>Dialog: Opens modal
Dialog-->>Store: User clicks "Initialize & import"
Store-->>Hook: true (confirmed)
Hook->>Host: project.create.mutate({ mode: { kind: "importLocal", repoPath, initIfNeeded: true } })
Host->>Repo: initLocalRepoInPlace(repoPath)
Repo->>FS: validateDirectoryPath(repoPath)
Repo->>FS: tryRevParseGitRoot(repoPath) [idempotency check]
alt Not already a repo
Repo->>FS: git init --initial-branch=main
Repo->>FS: git commit --allow-empty -m "Initial commit"
Repo->>FS: resolveLocalRepo(repoPath)
FS-->>Repo: { repoPath, remoteName: null, parsed: null }
else Already a repo
Repo->>FS: resolveLocalRepo(existingRoot)
FS-->>Repo: { repoPath, remoteName, parsed }
end
Host-->>Hook: { projectId, repoPath, mainWorkspaceId }
Hook->>Hook: finalizeSetup(activeHostUrl, result)
else User cancels
Dialog-->>Store: User clicks "Cancel"
Store-->>Hook: false (cancelled)
Hook-->>UI: null (no-op)
end
Note over UI,FS: On confirmation: git init leaves parent repos undisturbed<br/>Nested subdir resolves to parent root automatically
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| async function tryRevParseGitRoot(path: string): Promise<string | null> { | ||
| try { | ||
| return (await createUserSimpleGit(path).revparse(["--show-toplevel"])).trim(); | ||
| } catch { |
Contributor
There was a problem hiding this comment.
P2: The proposed non-throwing tryRevParseGitRoot swallows all errors, which can misclassify real operational failures as "non-git" and hide diagnostics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plans/20260601-v2-import-git-init-non-git-folder.md, line 89:
<comment>The proposed non-throwing `tryRevParseGitRoot` swallows all errors, which can misclassify real operational failures as "non-git" and hide diagnostics.</comment>
<file context>
@@ -0,0 +1,262 @@
+async function tryRevParseGitRoot(path: string): Promise<string | null> {
+ try {
+ return (await createUserSimpleGit(path).revparse(["--show-toplevel"])).trim();
+ } catch {
+ return null;
+ }
</file context>
| ): Promise<ResolvedRepo> { | ||
| validateDirectoryPath(repoPath, "Path"); | ||
|
|
||
| const existingRoot = await tryRevParseGitRoot(repoPath); |
Contributor
There was a problem hiding this comment.
P2: There is a TOCTOU window between the tryRevParseGitRoot check (line 165) and the gitInitMainBranch + commit sequence. If a concurrent import initializes the same folder in between, this creates a duplicate "Initial commit", breaking the idempotency guarantee stated in the docstring. Consider rechecking after gitInitMainBranch (which is itself idempotent) whether a commit already exists before adding one.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/project/utils/resolve-repo.ts, line 165:
<comment>There is a TOCTOU window between the `tryRevParseGitRoot` check (line 165) and the `gitInitMainBranch` + commit sequence. If a concurrent import initializes the same folder in between, this creates a duplicate "Initial commit", breaking the idempotency guarantee stated in the docstring. Consider rechecking after `gitInitMainBranch` (which is itself idempotent) whether a commit already exists before adding one.</comment>
<file context>
@@ -129,6 +142,43 @@ export async function resolveLocalRepo(
+): Promise<ResolvedRepo> {
+ validateDirectoryPath(repoPath, "Path");
+
+ const existingRoot = await tryRevParseGitRoot(repoPath);
+ if (existingRoot) return resolveLocalRepo(existingRoot);
+
</file context>
…r label
split("/") mishandled Windows backslash paths and trailing slashes (empty
label). getBaseName handles both separators and drops empty segments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Importing a folder that isn't a git repo yet dead-ends on
Not a git repository: <path>in v2 — there's no offer to initialize one. Likely the root cause of #5033 ("why cannot import local git folder without remote?"), which is really about a non-git folder (remote-less git repos already import fine).The throw happens at
project.findByPath→resolveLocalRepo→revParseGitRoot, beforecreate importLocalis reached.What this does
Detect → confirm → init + import. The folder is
git init'd only after the user confirms (it writes into their directory), then imported as a local-only project — no remote required.Server (
packages/host-service)resolve-repo.ts—tryRevParseGitRoot(non-throwing) +initLocalRepoInPlace(git-inits an existing, populated folder in place + empty initial commit soensureMainWorkspaceStricthas a real branch; idempotent; a subdir of an existing repo resolves to the parent root). ExportedvalidateDirectoryPath;revParseGitRootnow wraps the non-throwing variant.project.ts—findByPathreturns an additive optionalneedsGitInit: trueinstead of throwing on a non-git folder (matches the existing "server returns a discriminated result, client branches" pattern).createimportLocalgainsinitIfNeeded.handlers.ts—createFromImportLocalinits in place only wheninitIfNeeded. Cloud path unchanged (local-only repo → no clone URL, already supported byempty/template).Desktop
git-init-confirmstore +GitInitConfirmDialog, mounted once viaAddRepositoryModals.useFolderFirstImportbranches onneedsGitInit, confirms, then creates withinitIfNeeded: true. All 5 hook consumers untouched (the confirm is encapsulated in the hook/store rather than threaded through each call site).Design note
initIfNeededis a flag onimportLocalrather than a separateinitLocalcreate mode, because the input shape ({ repoPath }) is identical. Easy to switch to a distinct mode if preferred — see the plan doc (plans/20260601-v2-import-git-init-non-git-folder.md).Out of scope / unchanged
resolveGithubRepo(the "no GitHub remote" throw) stays GitHub-feature-only (PRs/Issues).Testing
resolve-repo.test.ts: 30/30 (6 new — init, non-empty adopt, idempotency, nested subdir, missing/file rejects).useFolderFirstImport.test.ts: 2 new (confirm → create-with-init; cancel → no-op).@superset/host-service+@superset/desktoptypecheck: ✅bun run lint: ✅settings search … Git tab visible in v2(confirmed failing with these changes stashed).Summary by cubic
Adds git init on v2 folder import when the selected folder isn’t a git repo. We confirm with the user, then initialize in place and import as a local-only project to avoid the “Not a git repository” dead end.
New Features
packages/host-service):project.findByPathcan returnneedsGitInit;project.createimportLocalacceptsinitIfNeeded; in-place init viainitLocalRepoInPlacewith an empty initial commit; includestryRevParseGitRoot; idempotent and respects parent repos.apps/desktop):GitInitConfirmDialog+git-init-confirmstore;useFolderFirstImportshows confirm whenneedsGitInitand creates withinitIfNeeded: true; mounted viaAddRepositoryModals.Bug Fixes
getBaseNamefor the confirm dialog’s folder label to handle Windows paths and trailing slashes.Written for commit 70b131e. Summary will update on new commits.
Summary by CodeRabbit