Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 89 additions & 52 deletions apps/desktop/src/lib/trpc/routers/projects/projects.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { existsSync, statSync } from "node:fs";
import { access } from "node:fs/promises";
import { access, mkdir, rm } from "node:fs/promises";
import { basename, join } from "node:path";
import {
BRANCH_PREFIX_MODES,
Expand Down Expand Up @@ -62,29 +62,20 @@ type OpenNewMultiResult =
| { canceled: false; multi: true; results: FolderOutcome[] }
| OpenNewError;

/**
* Initializes a git repository in the given path with an initial commit.
* Reused by openNew, openFromPath, and initGitAndOpen.
*/
async function initGitRepo(path: string): Promise<{ defaultBranch: string }> {
const git = simpleGit(path);

// Initialize git repository with 'main' as default branch
// Try with --initial-branch=main (Git 2.28+), fall back to plain init
try {
await git.init(["--initial-branch=main"]);
} catch (err) {
// Likely an older Git version that doesn't support --initial-branch
console.warn("Git init with --initial-branch failed, using fallback:", err);
await git.init();
}

// Create initial commit so we have a valid branch ref
try {
await git.raw(["commit", "--allow-empty", "-m", "Initial commit"]);
} catch (err) {
const errorMessage = err instanceof Error ? err.message : String(err);
// Check for common git config issues
if (
errorMessage.includes("empty ident") ||
errorMessage.includes("user.email") ||
Expand All @@ -103,11 +94,6 @@ async function initGitRepo(path: string): Promise<{ defaultBranch: string }> {
return { defaultBranch };
}

/**
* Creates or updates a project record in the database.
* If a project with the same mainRepoPath exists, updates lastOpenedAt.
* Otherwise, creates a new project.
*/
function upsertProject(mainRepoPath: string, defaultBranch: string): Project {
const name = basename(mainRepoPath);

Expand Down Expand Up @@ -140,22 +126,15 @@ function upsertProject(mainRepoPath: string, defaultBranch: string): Project {
return project;
}

/**
* Ensures a project has a main (branch) workspace.
* If one doesn't exist, creates it automatically.
* This is called after opening/creating a project to provide a default workspace.
*/
async function ensureMainWorkspace(project: Project): Promise<void> {
const existingBranchWorkspace = getBranchWorkspace(project.id);

// If branch workspace already exists, just touch it and return
if (existingBranchWorkspace) {
touchWorkspace(existingBranchWorkspace.id);
setLastActiveWorkspace(existingBranchWorkspace.id);
return;
}

// Get current branch from main repo
const branch = await getCurrentBranch(project.mainRepoPath);
if (!branch) {
console.warn(
Expand All @@ -164,8 +143,7 @@ async function ensureMainWorkspace(project: Project): Promise<void> {
return;
}

// Insert new branch workspace with conflict handling for race conditions
// The unique partial index (projectId WHERE type='branch') prevents duplicates
// Unique partial index (projectId WHERE type='branch') prevents duplicates
const insertResult = localDb
.insert(workspaces)
.values({
Expand All @@ -181,7 +159,6 @@ async function ensureMainWorkspace(project: Project): Promise<void> {

const wasExisting = insertResult.length === 0;

// Only shift existing workspaces if we successfully inserted
if (!wasExisting) {
const newWorkspaceId = insertResult[0].id;
const projectWorkspaces = localDb
Expand All @@ -205,7 +182,6 @@ async function ensureMainWorkspace(project: Project): Promise<void> {
}
}

// Get the workspace (either newly created or existing from race condition)
const workspace = insertResult[0] ?? getBranchWorkspace(project.id);

if (!workspace) {
Expand All @@ -230,45 +206,33 @@ async function ensureMainWorkspace(project: Project): Promise<void> {
}
}

// Safe filename regex: letters, numbers, dots, underscores, hyphens, spaces, and common unicode
// Allows most valid Git repo names while avoiding path traversal characters
// Callers must additionally reject dot-only names (".", "..") to prevent path traversal
const SAFE_REPO_NAME_REGEX = /^[a-zA-Z0-9._\- ]+$/;
const ALLOWED_URL_PROTOCOLS = new Set(["http:", "https:", "ssh:", "git:"]);
const SSH_GIT_URL_REGEX = /^[\w.-]+@[\w.-]+:[\w./-]+$/;

/**
* Extracts and validates a repository name from a git URL.
* Handles HTTP/HTTPS URLs, SSH-style URLs (git@host:user/repo), and edge cases.
*/
function extractRepoName(urlInput: string): string | null {
// Normalize: trim whitespace and strip trailing slashes
let normalized = urlInput.trim().replace(/\/+$/, "");

if (!normalized) return null;

let repoSegment: string | undefined;

// Try parsing as HTTP/HTTPS URL first
try {
const parsed = new URL(normalized);
if (parsed.protocol === "http:" || parsed.protocol === "https:") {
// Get pathname and strip query/hash (URL constructor handles this)
const pathname = parsed.pathname;
repoSegment = pathname.split("/").filter(Boolean).pop();
}
} catch {
// Not a valid URL, try SSH-style parsing
// Not a standard URL — fall through to SSH-style parsing
}

// Fallback to SSH-style parsing (git@github.com:user/repo.git)
if (!repoSegment) {
// Handle SSH format: git@host:path or just path segments
const colonIndex = normalized.indexOf(":");
if (colonIndex !== -1 && !normalized.includes("://")) {
// SSH-style: take everything after the colon
normalized = normalized.slice(colonIndex + 1);
}
// Split by '/' and get the last segment
repoSegment = normalized.split("/").filter(Boolean).pop();
}

Expand All @@ -279,13 +243,10 @@ function extractRepoName(urlInput: string): string | null {

try {
repoSegment = decodeURIComponent(repoSegment);
} catch {
// Invalid encoding, continue with raw value
}
} catch {}

repoSegment = repoSegment.trim();

// Validate against safe filename regex
if (!repoSegment || !SAFE_REPO_NAME_REGEX.test(repoSegment)) {
return null;
}
Expand Down Expand Up @@ -334,6 +295,28 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
.all();
}),

selectDirectory: publicProcedure
.input(
z.object({
defaultPath: z.string().optional(),
}),
)
.mutation(async ({ input }) => {
const window = getWindow();
if (!window) {
return { canceled: true as const, path: null };
}
const result = await dialog.showOpenDialog(window, {
properties: ["openDirectory", "createDirectory"],
title: "Select Directory",
defaultPath: input.defaultPath,
});
if (result.canceled || result.filePaths.length === 0) {
return { canceled: true as const, path: null };
}
return { canceled: false as const, path: result.filePaths[0] };
}),

getBranches: publicProcedure
.input(z.object({ projectId: z.string() }))
.query(
Expand All @@ -359,14 +342,11 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {

const git = simpleGit(project.mainRepoPath);

// Check if origin remote exists
let hasOrigin = false;
try {
const remotes = await git.getRemotes();
hasOrigin = remotes.some((r) => r.name === "origin");
} catch {
// If we can't get remotes, assume no origin
}
} catch {}

const branchSummary = await git.branch(["-a"]);

Expand All @@ -383,13 +363,11 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
}
}

// Get branch dates for sorting - fetch from both local and remote
const branchMap = new Map<
string,
{ lastCommitDate: number; isLocal: boolean; isRemote: boolean }
>();

// First, get remote branch dates (if origin exists)
if (hasOrigin) {
try {
const remoteBranchInfo = await git.raw([
Expand Down Expand Up @@ -422,7 +400,6 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
});
}
} catch {
// Fallback for remote branches
for (const name of remoteBranchSet) {
branchMap.set(name, {
lastCommitDate: 0,
Expand All @@ -433,7 +410,6 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
}
}

// Then, add local-only branches
try {
const localBranchInfo = await git.raw([
"for-each-ref",
Expand Down Expand Up @@ -797,6 +773,67 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
}
}),

createEmptyRepo: publicProcedure
.input(
z.object({
name: z
.string()
.min(1)
.refine(
(val) => SAFE_REPO_NAME_REGEX.test(val) && !/^\.+$/.test(val),
{
message:
"Name can only contain letters, numbers, dots, underscores, hyphens, and spaces",
},
),
parentDir: z.string().min(1),
}),
)
.mutation(async ({ input }) => {
try {
const repoPath = join(input.parentDir, input.name);

if (existsSync(repoPath)) {
return {
canceled: false as const,
success: false as const,
error: `A folder named "${input.name}" already exists at this location.`,
};
}

await mkdir(repoPath, { recursive: true });

let defaultBranch: string;
try {
({ defaultBranch } = await initGitRepo(repoPath));
} catch (gitErr) {
await rm(repoPath, { recursive: true, force: true });
throw gitErr;
}
const project = upsertProject(repoPath, defaultBranch);
await ensureMainWorkspace(project);
Comment on lines +813 to +814
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

createEmptyRepo: no cleanup if upsertProject or ensureMainWorkspace throws after a successful initGitRepo.

The previous review addressed cleanup for initGitRepo failure (inner try-catch + rm at lines 807–812). However, if upsertProject (line 813) or ensureMainWorkspace (line 814) throw a DB error, the outer catch at line 826 returns an error response but leaves the git-initialized directory on disk (and potentially a partial project record). On retry with the same name, existsSync(repoPath) returns true → the user sees "A folder already exists" and is blocked until they manually delete the directory.

🐛 Proposed fix — extend cleanup scope
+    let project: Project;
     try {
         ({ defaultBranch } = await initGitRepo(repoPath));
     } catch (gitErr) {
         await rm(repoPath, { recursive: true, force: true });
         throw gitErr;
     }
-    const project = upsertProject(repoPath, defaultBranch);
-    await ensureMainWorkspace(project);
+    try {
+        project = upsertProject(repoPath, defaultBranch);
+        await ensureMainWorkspace(project);
+    } catch (dbErr) {
+        await rm(repoPath, { recursive: true, force: true });
+        throw dbErr;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` around lines 813 -
814, createEmptyRepo currently leaves the initialized git folder (repoPath) and
possibly a partial DB record if upsertProject or ensureMainWorkspace throws;
wrap the calls to upsertProject and ensureMainWorkspace in their own try/catch
(or a try/finally) so that on any error you remove the repoPath directory
(fs.rmSync or fs.promises.rm with recursive) and, if a project object was
returned, delete the created project record (e.g. call whatever
deleteProject/deleteProjectById or prisma.project.delete using project.id)
before rethrowing/returning the error; reference the createEmptyRepo function,
the repoPath variable, the upsertProject call (which returns project), and the
ensureMainWorkspace call when making the changes.


track("project_opened", {
project_id: project.id,
method: "create_empty",
});

return {
canceled: false as const,
success: true as const,
project,
};
} catch (error) {
const errorMessage =
error instanceof Error ? error.message : String(error);
return {
canceled: false as const,
success: false as const,
error: `Failed to create repository: ${errorMessage}`,
};
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}),

update: publicProcedure
.input(
z.object({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { Button } from "@superset/ui/button";
import { Input } from "@superset/ui/input";
import { useState } from "react";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { useProjectCreationHandler } from "../../hooks/useProjectCreationHandler";

interface CloneRepoTabProps {
onError: (error: string) => void;
parentDir: string;
}

export function CloneRepoTab({ onError, parentDir }: CloneRepoTabProps) {
const [url, setUrl] = useState("");
const cloneRepo = electronTrpc.projects.cloneRepo.useMutation();
const { handleResult, handleError, isCreatingWorkspace } =
useProjectCreationHandler(onError);

const isLoading = cloneRepo.isPending || isCreatingWorkspace;

const handleClone = () => {
if (!url.trim()) {
onError("Please enter a repository URL");
return;
}
if (!parentDir.trim()) {
onError("Please select a project location");
return;
}

cloneRepo.mutate(
{ url: url.trim(), targetDirectory: parentDir.trim() },
{
onSuccess: (result) => handleResult(result, () => setUrl("")),
onError: handleError,
},
);
};

return (
<div className="flex flex-col gap-5">
<div>
<label
htmlFor="clone-url"
className="block text-sm font-medium text-foreground mb-2"
>
Repository URL
</label>
<Input
id="clone-url"
value={url}
onChange={(e) => setUrl(e.target.value)}
placeholder="https:// or git@github.com:user/repo.git"
disabled={isLoading}
onKeyDown={(e) => {
if (e.key === "Enter" && !isLoading) {
handleClone();
}
}}
autoFocus
/>
</div>
<div className="flex justify-end pt-2 border-t border-border/40">
<Button onClick={handleClone} disabled={isLoading} size="sm">
{isLoading ? "Cloning..." : "Clone"}
</Button>
</div>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { CloneRepoTab } from "./CloneRepoTab";
Loading
Loading