From 98b484eb9bafd5eaa0ef5dd8dbcb1553ee730d83 Mon Sep 17 00:00:00 2001 From: kiranannadatha8 <87536091+kiranannadatha8@users.noreply.github.com> Date: Mon, 4 May 2026 03:08:46 -0400 Subject: [PATCH] feat(sidebar): add "create new project" option to Add Repo dialog (#929) Co-authored-by: Claude Opus 4.7 Co-authored-by: Orca Co-authored-by: Kiran Annadata Co-authored-by: brennanb2025 --- src/main/ipc/repos-create.test.ts | 379 +++++++++++++++++ src/main/ipc/repos.ts | 194 ++++++++- src/main/ipc/telemetry.test.ts | 5 +- src/main/ipc/telemetry.ts | 48 +-- src/main/telemetry/client.test.ts | 5 +- src/main/telemetry/client.ts | 6 +- src/preload/api-types.ts | 6 + src/preload/index.ts | 6 + .../components/sidebar/AddRepoCreateStep.tsx | 387 ++++++++++++++++++ .../src/components/sidebar/AddRepoDialog.tsx | 131 +++--- .../components/sidebar/AddRepoSetupStep.tsx | 80 ++++ .../src/components/sidebar/AddRepoSteps.tsx | 2 +- 12 files changed, 1146 insertions(+), 103 deletions(-) create mode 100644 src/main/ipc/repos-create.test.ts create mode 100644 src/renderer/src/components/sidebar/AddRepoCreateStep.tsx create mode 100644 src/renderer/src/components/sidebar/AddRepoSetupStep.tsx diff --git a/src/main/ipc/repos-create.test.ts b/src/main/ipc/repos-create.test.ts new file mode 100644 index 00000000000..460f4b7d6c5 --- /dev/null +++ b/src/main/ipc/repos-create.test.ts @@ -0,0 +1,379 @@ +/** + * Unit tests for repos:create (orca#763). + * + * Pins the invariants that matter here: + * - Name validation catches empty/slash/./.. before any fs I/O. + * - Empty pre-existing directories are accepted; non-empty ones are not. + * - Only directories we create ourselves are removed on rollback — a folder + * the user picked must survive a failure so they can retry. + * - Git repos get an empty initial commit; without it, HEAD has no branch. + */ + +import { describe, expect, it, vi, beforeEach } from 'vitest' + +const { + handleMock, + removeHandlerMock, + mockStore, + mkdirMock, + accessMock, + readdirMock, + rmMock, + gitExecFileAsyncMock, + rebuildAuthorizedRootsCacheMock +} = vi.hoisted(() => ({ + handleMock: vi.fn(), + removeHandlerMock: vi.fn(), + mockStore: { + getRepos: vi.fn().mockReturnValue([]), + addRepo: vi.fn(), + removeRepo: vi.fn(), + getRepo: vi.fn(), + updateRepo: vi.fn() + }, + mkdirMock: vi.fn(), + accessMock: vi.fn(), + readdirMock: vi.fn(), + rmMock: vi.fn(), + gitExecFileAsyncMock: vi.fn(), + rebuildAuthorizedRootsCacheMock: vi.fn().mockResolvedValue(undefined) +})) + +vi.mock('electron', () => ({ + dialog: { showOpenDialog: vi.fn() }, + ipcMain: { + handle: handleMock, + removeHandler: removeHandlerMock + } +})) + +vi.mock('fs/promises', () => ({ + mkdir: mkdirMock, + access: accessMock, + readdir: readdirMock, + rm: rmMock +})) + +vi.mock('../git/runner', () => ({ + gitExecFileAsync: gitExecFileAsyncMock, + gitSpawn: vi.fn() +})) + +vi.mock('../git/repo', () => ({ + isGitRepo: vi.fn().mockReturnValue(true), + getGitUsername: vi.fn().mockReturnValue(''), + getRepoName: vi.fn().mockImplementation((path: string) => path.split('/').pop()), + getBaseRefDefault: vi.fn().mockResolvedValue('origin/main'), + searchBaseRefs: vi.fn().mockResolvedValue([]) +})) + +vi.mock('./filesystem-auth', () => ({ + rebuildAuthorizedRootsCache: rebuildAuthorizedRootsCacheMock +})) + +vi.mock('../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: vi.fn() +})) + +vi.mock('./ssh', () => ({ + getActiveMultiplexer: vi.fn() +})) + +import { registerRepoHandlers } from './repos' + +type CreateArgs = { parentPath: string; name: string; kind: 'git' | 'folder' } +type CreateResult = + | { repo: { id: string; path: string; kind: 'git' | 'folder' } } + | { error: string } + +describe('repos:create', () => { + const handlers = new Map Promise>() + const mockWindow = { + isDestroyed: () => false, + webContents: { send: vi.fn() } + } + + const callCreate = (args: CreateArgs): Promise => { + const handler = handlers.get('repos:create') + if (!handler) { + throw new Error('repos:create handler was never registered') + } + return handler(null, args) + } + + beforeEach(() => { + handlers.clear() + handleMock.mockReset() + handleMock.mockImplementation((channel: string, handler: (...a: unknown[]) => unknown) => { + handlers.set(channel, handler as (event: unknown, args: CreateArgs) => Promise) + }) + removeHandlerMock.mockReset() + mockStore.getRepos.mockReset().mockReturnValue([]) + mockStore.addRepo.mockReset() + mockWindow.webContents.send.mockReset() + rebuildAuthorizedRootsCacheMock.mockReset().mockResolvedValue(undefined) + + // Default baseline: target does NOT exist yet, mkdir succeeds, git OK. + accessMock.mockReset().mockRejectedValue(new Error('ENOENT')) + readdirMock.mockReset().mockResolvedValue([]) + mkdirMock.mockReset().mockResolvedValue(undefined) + rmMock.mockReset().mockResolvedValue(undefined) + gitExecFileAsyncMock.mockReset().mockResolvedValue({ stdout: '', stderr: '' }) + + registerRepoHandlers(mockWindow as never, mockStore as never) + }) + + it('registers the repos:create handler', () => { + expect(handlers.has('repos:create')).toBe(true) + }) + + it('unregisters any previously-registered repos:create handler', () => { + // registerRepoHandlers must call removeHandler('repos:create') before + // ipcMain.handle to avoid the "second handler for same channel" throw + // when this module is re-registered (e.g., after a reload). + expect(removeHandlerMock).toHaveBeenCalledWith('repos:create') + }) + + // ── input validation ────────────────────────────────────────────── + + it('rejects empty names', async () => { + const result = await callCreate({ parentPath: '/tmp', name: ' ', kind: 'git' }) + expect(result).toEqual({ error: 'Name cannot be empty' }) + expect(mkdirMock).not.toHaveBeenCalled() + }) + + it('rejects names containing a forward slash', async () => { + const result = await callCreate({ parentPath: '/tmp', name: 'foo/bar', kind: 'git' }) + expect(result).toMatchObject({ error: expect.stringContaining('slash') }) + expect(mkdirMock).not.toHaveBeenCalled() + }) + + it('rejects names containing a backslash', async () => { + const result = await callCreate({ parentPath: '/tmp', name: 'foo\\bar', kind: 'git' }) + expect(result).toMatchObject({ error: expect.stringContaining('slash') }) + expect(mkdirMock).not.toHaveBeenCalled() + }) + + it('rejects "." and ".." as names', async () => { + for (const name of ['.', '..']) { + mkdirMock.mockClear() + const result = await callCreate({ parentPath: '/tmp', name, kind: 'git' }) + expect(result).toMatchObject({ error: expect.stringContaining('slash') }) + expect(mkdirMock).not.toHaveBeenCalled() + } + }) + + it('rejects empty parent path', async () => { + const result = await callCreate({ parentPath: ' ', name: 'project', kind: 'git' }) + expect(result).toEqual({ error: 'Parent directory is required' }) + expect(mkdirMock).not.toHaveBeenCalled() + }) + + // ── existing-directory handling ─────────────────────────────────── + + it('rejects a non-empty existing directory without calling mkdir', async () => { + accessMock.mockResolvedValueOnce(undefined) // exists + readdirMock.mockResolvedValueOnce(['README.md', '.DS_Store']) + + const result = await callCreate({ parentPath: '/tmp', name: 'busy', kind: 'git' }) + + expect(result).toMatchObject({ error: expect.stringContaining('not empty') }) + expect(mkdirMock).not.toHaveBeenCalled() + expect(mockStore.addRepo).not.toHaveBeenCalled() + }) + + it('accepts an empty existing directory and does not call mkdir', async () => { + accessMock.mockResolvedValueOnce(undefined) // exists + readdirMock.mockResolvedValueOnce([]) + + const result = await callCreate({ parentPath: '/tmp', name: 'empty', kind: 'folder' }) + + expect(mkdirMock).not.toHaveBeenCalled() + expect(mockStore.addRepo).toHaveBeenCalledWith( + expect.objectContaining({ path: '/tmp/empty', kind: 'folder' }) + ) + expect(result).toHaveProperty('repo.kind', 'folder') + }) + + it('creates a missing directory with mkdir', async () => { + // accessMock rejects by default → path does not exist + await callCreate({ parentPath: '/tmp', name: 'brand-new', kind: 'folder' }) + + expect(mkdirMock).toHaveBeenCalledWith('/tmp/brand-new', { recursive: false }) + }) + + // ── plain folder happy path ─────────────────────────────────────── + + it('creates a plain folder without running any git commands', async () => { + const result = await callCreate({ parentPath: '/tmp', name: 'plain', kind: 'folder' }) + + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + expect(mockStore.addRepo).toHaveBeenCalledWith( + expect.objectContaining({ + path: '/tmp/plain', + displayName: 'plain', + kind: 'folder' + }) + ) + expect(result).toHaveProperty('repo.kind', 'folder') + }) + + // ── git repo happy path ─────────────────────────────────────────── + + it('creates a git repo with an empty initial commit (in order)', async () => { + const result = await callCreate({ parentPath: '/tmp', name: 'gitproj', kind: 'git' }) + + expect(mkdirMock).toHaveBeenCalledWith('/tmp/gitproj', { recursive: false }) + expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(1, ['init'], { cwd: '/tmp/gitproj' }) + expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith( + 2, + ['commit', '--allow-empty', '-m', 'Initial commit'], + { cwd: '/tmp/gitproj' } + ) + expect(mockStore.addRepo).toHaveBeenCalledWith( + expect.objectContaining({ + path: '/tmp/gitproj', + displayName: 'gitproj', + kind: 'git' + }) + ) + expect(result).toHaveProperty('repo.kind', 'git') + }) + + // ── rollback semantics ──────────────────────────────────────────── + + it('rolls back the directory it just created when git init fails', async () => { + gitExecFileAsyncMock.mockReset().mockRejectedValueOnce(new Error('git init blew up')) + + const result = await callCreate({ parentPath: '/tmp', name: 'broken', kind: 'git' }) + + expect(rmMock).toHaveBeenCalledWith('/tmp/broken', { recursive: true, force: true }) + expect(mockStore.addRepo).not.toHaveBeenCalled() + expect(result).toMatchObject({ error: expect.stringContaining('Failed to initialize') }) + }) + + it('does NOT rm a pre-existing empty directory when git init fails', async () => { + // Pretend the directory already existed (and is empty) — user pre-created it. + accessMock.mockResolvedValueOnce(undefined) + readdirMock.mockResolvedValueOnce([]) + gitExecFileAsyncMock.mockReset().mockRejectedValueOnce(new Error('git init blew up')) + + const result = await callCreate({ parentPath: '/tmp', name: 'preexisting', kind: 'git' }) + + expect(rmMock).not.toHaveBeenCalled() + expect(mockStore.addRepo).not.toHaveBeenCalled() + expect(result).toMatchObject({ error: expect.stringContaining('Failed to initialize') }) + }) + + it('surfaces an "initialize"-flavored error when git init itself fails', async () => { + // First git call (init) rejects — the commit step never runs. + gitExecFileAsyncMock.mockReset().mockRejectedValueOnce(new Error('init broke')) + + const result = await callCreate({ parentPath: '/tmp', name: 'initfail', kind: 'git' }) + + expect(rmMock).toHaveBeenCalledWith('/tmp/initfail', { recursive: true, force: true }) + expect(mockStore.addRepo).not.toHaveBeenCalled() + // Loose match — handler distinguishes init vs commit failures, and we want + // to tolerate small wording tweaks as long as it still mentions "initialize". + expect(result).toMatchObject({ error: expect.stringContaining('initialize') }) + }) + + it('rolls back directory when git commit fails (not just init)', async () => { + // init resolves, commit rejects — the failure must still trigger rollback + // and surface a commit-flavored error (distinct from the init-failure path). + gitExecFileAsyncMock + .mockReset() + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(new Error('commit broke')) + + const result = await callCreate({ parentPath: '/tmp', name: 'commitfail', kind: 'git' }) + + expect(rmMock).toHaveBeenCalledWith('/tmp/commitfail', { recursive: true, force: true }) + expect(mockStore.addRepo).not.toHaveBeenCalled() + expect(result).toMatchObject({ error: expect.stringContaining('commit') }) + }) + + it('strips only .git/ when commit fails in a pre-existing empty folder', async () => { + // User pre-created an empty folder; git init succeeded, commit failed. + // The folder itself must survive (user owns it) but the half-init'd + // .git/ should be removed so the folder looks untouched. + accessMock.mockResolvedValueOnce(undefined) + readdirMock.mockResolvedValueOnce([]) + gitExecFileAsyncMock + .mockReset() + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(new Error('commit broke')) + + const result = await callCreate({ parentPath: '/tmp', name: 'pre-existing', kind: 'git' }) + + expect(rmMock).toHaveBeenCalledWith('/tmp/pre-existing/.git', { recursive: true, force: true }) + expect(rmMock).not.toHaveBeenCalledWith('/tmp/pre-existing', { recursive: true, force: true }) + expect(mockStore.addRepo).not.toHaveBeenCalled() + expect(result).toMatchObject({ error: expect.stringContaining('commit') }) + }) + + // ── friendly messaging ──────────────────────────────────────────── + + it('surfaces a friendly message when git author identity is missing', async () => { + gitExecFileAsyncMock + .mockReset() + .mockResolvedValueOnce({ stdout: '', stderr: '' }) // git init + .mockRejectedValueOnce( + new Error('Please tell me who you are. Run git config --global user.email ...') + ) + + const result = await callCreate({ parentPath: '/tmp', name: 'authorless', kind: 'git' }) + + expect(rmMock).toHaveBeenCalledWith('/tmp/authorless', { recursive: true, force: true }) + expect(mockStore.addRepo).not.toHaveBeenCalled() + expect(result).toMatchObject({ + error: expect.stringContaining('Git author identity is not configured') + }) + }) + + // ── renderer notification ───────────────────────────────────────── + + it('notifies the renderer via repos:changed after a successful create', async () => { + await callCreate({ parentPath: '/tmp', name: 'notified', kind: 'folder' }) + expect(mockWindow.webContents.send).toHaveBeenCalledWith('repos:changed') + }) + + // ── authorized-roots cache refresh ──────────────────────────────── + + it('rebuilds the authorized-roots cache after a successful folder create', async () => { + // Roots cache must be refreshed so the renderer can read the new path + // without waiting for the next full reconciliation. + await callCreate({ parentPath: '/tmp', name: 'rooted', kind: 'folder' }) + expect(rebuildAuthorizedRootsCacheMock).toHaveBeenCalledTimes(1) + }) + + it('does NOT rebuild the authorized-roots cache on a validation failure', async () => { + const result = await callCreate({ parentPath: '/tmp', name: ' ', kind: 'git' }) + expect(result).toEqual({ error: 'Name cannot be empty' }) + expect(rebuildAuthorizedRootsCacheMock).not.toHaveBeenCalled() + }) + + it('does NOT rebuild the authorized-roots cache when dedup short-circuits', async () => { + const existing = { id: 'abc', path: '/tmp/dupe2', displayName: 'dupe2', kind: 'git' } + mockStore.getRepos.mockReturnValue([existing]) + + await callCreate({ parentPath: '/tmp', name: 'dupe2', kind: 'git' }) + + expect(rebuildAuthorizedRootsCacheMock).not.toHaveBeenCalled() + }) + + // ── dedup-by-path ───────────────────────────────────────────────── + + it('returns the existing repo when one already lives at the target path', async () => { + const existing = { id: 'abc', path: '/tmp/dupe', displayName: 'dupe', kind: 'git' } + mockStore.getRepos.mockReturnValue([existing]) + + const result = await callCreate({ parentPath: '/tmp', name: 'dupe', kind: 'git' }) + + expect(result).toEqual({ repo: existing }) + // Short-circuit before any fs or git work. + expect(mkdirMock).not.toHaveBeenCalled() + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + expect(mockStore.addRepo).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index d35624fac49..252ee4748ac 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -10,9 +10,9 @@ import { isFolderRepo } from '../../shared/repo-kind' import { REPO_COLORS } from '../../shared/constants' import { rebuildAuthorizedRootsCache } from './filesystem-auth' import type { ChildProcess } from 'child_process' -import { rm } from 'fs/promises' -import { gitSpawn } from '../git/runner' -import { join, basename } from 'path' +import { access, mkdir, readdir, rm } from 'fs/promises' +import { gitExecFileAsync, gitSpawn } from '../git/runner' +import { basename, isAbsolute, join } from 'path' import { isGitRepo, getGitUsername, @@ -51,6 +51,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v ipcMain.removeHandler('repos:getBaseRefDefault') ipcMain.removeHandler('repos:searchBaseRefs') ipcMain.removeHandler('repos:addRemote') + ipcMain.removeHandler('repos:create') ipcMain.removeHandler('sparsePresets:list') ipcMain.removeHandler('sparsePresets:save') ipcMain.removeHandler('sparsePresets:remove') @@ -198,6 +199,193 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v } ) + // Creates a new repo or folder from scratch (orca#763). An empty initial + // commit is required for git repos so HEAD has a branch ref — Orca's + // worktree features all need one. + ipcMain.handle( + 'repos:create', + async ( + _event, + args: { parentPath: string; name: string; kind: 'git' | 'folder' } + ): Promise<{ repo: Repo } | { error: string }> => { + const name = args.name?.trim() ?? '' + const parentPath = args.parentPath?.trim() ?? '' + // Why: IPC input is untrusted — coerce to the narrow union so a bogus + // string (e.g. "x") can't skip git init yet persist as kind: "x" in the + // store. Mirrors the coercion in repos:add above. + const repoKind: 'git' | 'folder' = args.kind === 'folder' ? 'folder' : 'git' + + if (!name) { + return { error: 'Name cannot be empty' } + } + // Block slashes and ./.. so the name can't escape the chosen parent. + // The UI already disables submit in these cases; this guards direct IPC use. + if (/[\\/]/.test(name) || name === '.' || name === '..') { + return { error: 'Name cannot contain slashes or be "." / ".."' } + } + if (!parentPath) { + return { error: 'Parent directory is required' } + } + // Why: blocks CWD-relative paths from slipping through the IPC boundary; + // the UI uses pickDirectory which returns absolute paths, this guards + // direct IPC use (and keeps targetPath stable across process cwd changes). + if (!isAbsolute(parentPath)) { + return { error: 'Parent directory must be an absolute path' } + } + + const targetPath = join(parentPath, name) + + // Dedup by path (same as repos:add) so a double-click on Create doesn't + // produce two sidebar entries pointing at the same folder. This is the + // first of three dedup checks; see the pre-addRepo check below for why + // the race matters even after this one passes. + const existing = store.getRepos().find((r) => r.path === targetPath) + if (existing) { + return { repo: existing } + } + + // Empty pre-existing directories are allowed (e.g. one the user made in + // Finder first). Non-empty ones are rejected so we don't overwrite files. + let createdDir = false + let targetExists = false + try { + await access(targetPath) + targetExists = true + } catch (err) { + // Why: only ENOENT means "the path is free to use". Other codes + // (EACCES, ENOTDIR, EPERM, ELOOP, ...) mean something is in the way + // that mkdir can't fix — surface a precise error instead of falling + // through to mkdir and returning a misleading "Failed to create + // directory" message. + // + // Why the message fallback: fs.promises.access always attaches a + // NodeJS.ErrnoException code in production, but plain Error objects + // thrown in tests / non-Node contexts won't — treat a message that + // reads like ENOENT as one so we don't over-reject. + const code = + err && typeof err === 'object' && 'code' in err + ? (err as NodeJS.ErrnoException).code + : undefined + const looksLikeEnoent = + code === 'ENOENT' || + (code === undefined && err instanceof Error && /ENOENT/.test(err.message)) + if (!looksLikeEnoent) { + const message = err instanceof Error ? err.message : String(err) + return { error: `Cannot access target path: ${message}` } + } + } + + if (targetExists) { + try { + const entries = await readdir(targetPath) + if (entries.length > 0) { + return { + error: `"${name}" already exists at this location and is not empty.` + } + } + } catch (err) { + // Why: access succeeded but readdir failed — the path exists but we + // can't inspect it (e.g. it's a file, not a directory; or perms). + // mkdir would definitely fail here too, so return a distinct error. + const message = err instanceof Error ? err.message : String(err) + return { error: `Failed to read directory: ${message}` } + } + } else { + try { + await mkdir(targetPath, { recursive: false }) + createdDir = true + } catch (err) { + // Why: EEXIST here means another concurrent repos:create for the + // same path won the mkdir race. If they already added the repo to + // the store, return that entry instead of a confusing error. This + // is the second dedup check; see the pre-addRepo check below for + // the full race explanation. + const code = + err && typeof err === 'object' && 'code' in err + ? (err as NodeJS.ErrnoException).code + : undefined + const isEexist = code === 'EEXIST' || (err instanceof Error && /EEXIST/.test(err.message)) + if (isEexist) { + const raceWinner = store.getRepos().find((r) => r.path === targetPath) + if (raceWinner) { + return { repo: raceWinner } + } + } + const message = err instanceof Error ? err.message : String(err) + return { error: `Failed to create directory: ${message}` } + } + } + + if (repoKind === 'git') { + // Why: track which git step is running so the catch can attribute the + // failure correctly. The identity-hint regex is only meaningful during + // commit — git init itself never produces "Please tell me who you are". + let step: 'init' | 'commit' = 'init' + try { + await gitExecFileAsync(['init'], { cwd: targetPath }) + step = 'commit' + await gitExecFileAsync(['commit', '--allow-empty', '-m', 'Initial commit'], { + cwd: targetPath + }) + } catch (err) { + // Only remove the directory if we made it. A pre-existing folder the + // user picked must survive so they can retry after fixing git config. + // Why: if we didn't make the directory but `git init` created `.git/` + // inside it, strip just `.git/` so the user's folder looks the way + // they left it. Retrying works either way, but leaving a half-init'd + // repo behind is confusing if they choose to skip the retry. + if (createdDir) { + await rm(targetPath, { recursive: true, force: true }).catch(() => {}) + } else if (step === 'commit') { + await rm(join(targetPath, '.git'), { recursive: true, force: true }).catch(() => {}) + } + const message = err instanceof Error ? err.message : String(err) + if ( + step === 'commit' && + /Please tell me who you are|user\.name|user\.email/i.test(message) + ) { + return { + error: + 'Git author identity is not configured. Run `git config --global user.name "Your Name"` and `git config --global user.email "you@example.com"`, then try again.' + } + } + const stepLabel = + step === 'init' + ? 'Failed to initialize git repository' + : 'Failed to create initial commit' + return { error: `${stepLabel}: ${message}` } + } + } + + // Why: ipcMain.handle doesn't serialize concurrent calls; re-running the + // dedup lookup here closes the window between the first check and + // addRepo. A second repos:create for the same path that raced past the + // initial dedup now returns the entry the first call persisted. + const raceWinner = store.getRepos().find((r) => r.path === targetPath) + if (raceWinner) { + // Why: do NOT rm even if this invocation created the directory — the + // other invocation is using it. Leaking a freshly-made empty folder on + // a rare race is strictly safer than deleting a directory the winning + // call (and the user) now owns. + return { repo: raceWinner } + } + + const repo: Repo = { + id: randomUUID(), + path: targetPath, + displayName: name, + badgeColor: REPO_COLORS[store.getRepos().length % REPO_COLORS.length], + addedAt: Date.now(), + kind: repoKind + } + + store.addRepo(repo) + await rebuildAuthorizedRootsCache(store) + notifyReposChanged(mainWindow) + return { repo } + } + ) + ipcMain.handle('repos:remove', async (_event, args: { repoId: string }) => { store.removeRepo(args.repoId) await rebuildAuthorizedRootsCache(store) diff --git a/src/main/ipc/telemetry.test.ts b/src/main/ipc/telemetry.test.ts index 3908206c12b..151889d4fdb 100644 --- a/src/main/ipc/telemetry.test.ts +++ b/src/main/ipc/telemetry.test.ts @@ -27,7 +27,10 @@ import { registerTelemetryHandlers } from './telemetry' function captureHandlers(): void { handlers.clear() for (const call of handleMock.mock.calls) { - const [channel, handler] = call as [string, typeof handlers extends Map ? V : never] + const [channel, handler] = call as [ + string, + typeof handlers extends Map ? V : never + ] handlers.set(channel, handler) } } diff --git a/src/main/ipc/telemetry.ts b/src/main/ipc/telemetry.ts index 4d280b5520f..2d263c04211 100644 --- a/src/main/ipc/telemetry.ts +++ b/src/main/ipc/telemetry.ts @@ -25,34 +25,28 @@ import { setOptIn, track } from '../telemetry/client' import type { EventName, EventProps } from '../../shared/telemetry-events' export function registerTelemetryHandlers(): void { - ipcMain.handle( - 'telemetry:track', - (_event, name: unknown, props: unknown): void => { - // Strict input typing: non-string names are dropped at the boundary - // before the validator even sees them. The validator would also drop - // (unknown event name), but the main-side narrow keeps the attack - // surface minimal — a flood of bogus payloads does not exercise the - // Zod parser for no reason. - if (typeof name !== 'string') { - return - } - // `props` may legitimately be omitted; treat `undefined`/`null` as an - // empty object before the validator. Anything else non-object (e.g. - // a string, a number) is a boundary violation. - if (props !== null && props !== undefined && typeof props !== 'object') { - return - } - // The casts to `EventName` / `EventProps` here are - // pass-through only — this file does NOT pretend the renderer's - // name/props are type-safe. The validator inside `track()` is the - // single enforcement point at runtime; these casts only feed the - // typed channel that the validator will re-check. - track( - name as EventName, - (props ?? {}) as EventProps - ) + ipcMain.handle('telemetry:track', (_event, name: unknown, props: unknown): void => { + // Strict input typing: non-string names are dropped at the boundary + // before the validator even sees them. The validator would also drop + // (unknown event name), but the main-side narrow keeps the attack + // surface minimal — a flood of bogus payloads does not exercise the + // Zod parser for no reason. + if (typeof name !== 'string') { + return } - ) + // `props` may legitimately be omitted; treat `undefined`/`null` as an + // empty object before the validator. Anything else non-object (e.g. + // a string, a number) is a boundary violation. + if (props !== null && props !== undefined && typeof props !== 'object') { + return + } + // The casts to `EventName` / `EventProps` here are + // pass-through only — this file does NOT pretend the renderer's + // name/props are type-safe. The validator inside `track()` is the + // single enforcement point at runtime; these casts only feed the + // typed channel that the validator will re-check. + track(name as EventName, (props ?? {}) as EventProps) + }) ipcMain.handle('telemetry:setOptIn', (_event, optedIn: unknown): void => { // Strict input typing — renderer can pass anything over IPC. diff --git a/src/main/telemetry/client.test.ts b/src/main/telemetry/client.test.ts index 52a82af9409..fef3d27fec6 100644 --- a/src/main/telemetry/client.test.ts +++ b/src/main/telemetry/client.test.ts @@ -59,7 +59,10 @@ function makeFakeStore(settings: GlobalSettings): Store { getSettings: vi.fn(() => settings), updateSettings: vi.fn((updates: Partial) => { if (updates.telemetry) { - settings.telemetry = { ...settings.telemetry, ...updates.telemetry } as typeof settings.telemetry + settings.telemetry = { + ...settings.telemetry, + ...updates.telemetry + } as typeof settings.telemetry } return settings }) diff --git a/src/main/telemetry/client.ts b/src/main/telemetry/client.ts index 353822c8b87..2fbccf9e268 100644 --- a/src/main/telemetry/client.ts +++ b/src/main/telemetry/client.ts @@ -86,11 +86,7 @@ let storeRef: Store | null = null // be bounded by `resolveConsent` + the validator. let testTransportEnabled = false -function buildCommonProps( - installId: string, - sid: string, - channel: 'stable' | 'rc' -): CommonProps { +function buildCommonProps(installId: string, sid: string, channel: 'stable' | 'rc'): CommonProps { // `.max(64)` on every free-form string field in `commonPropsSchema` is the // upper bound; node's platform / arch / release strings are always well // under that in practice. We do not truncate here because the validator's diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index b1b500973c3..aad6e227c3d 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -365,6 +365,12 @@ export type PreloadApi = { displayName?: string kind?: 'git' | 'folder' }) => Promise<{ repo: Repo } | { error: string }> + // Why: error union matches the IPC handler's return shape; renderer callers branch on `'error' in result`. + create: (args: { + parentPath: string + name: string + kind: 'git' | 'folder' + }) => Promise<{ repo: Repo } | { error: string }> onCloneProgress: (callback: (data: { phase: string; percent: number }) => void) => () => void getGitUsername: (args: { repoId: string }) => Promise getBaseRefDefault: (args: { repoId: string }) => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index 067d9274f1d..162f16293c6 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -212,6 +212,12 @@ const api = { kind?: 'git' | 'folder' }): Promise => ipcRenderer.invoke('repos:addRemote', args), + create: (args: { + parentPath: string + name: string + kind: 'git' | 'folder' + }): Promise => ipcRenderer.invoke('repos:create', args), + remove: (args: { repoId: string }): Promise => ipcRenderer.invoke('repos:remove', args), update: (args: { repoId: string; updates: Record }): Promise => diff --git a/src/renderer/src/components/sidebar/AddRepoCreateStep.tsx b/src/renderer/src/components/sidebar/AddRepoCreateStep.tsx new file mode 100644 index 00000000000..5678c9300a6 --- /dev/null +++ b/src/renderer/src/components/sidebar/AddRepoCreateStep.tsx @@ -0,0 +1,387 @@ +/** + * Step for AddRepoDialog (orca#763). + * + * Split from AddRepoDialog and AddRepoSteps to keep both under the 400-line + * oxlint limit, following the same pattern as useRemoteRepo. + */ + +import React, { useCallback, useRef, useState } from 'react' +import { toast } from 'sonner' +import { Folder, GitBranch, Home, Pencil } from 'lucide-react' +import { useAppStore } from '@/store' +import { DialogHeader, DialogTitle, DialogDescription } from '@/components/ui/dialog' +import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { isGitRepoKind } from '../../../../shared/repo-kind' +import type { Repo } from '../../../../shared/types' + +type DialogStep = 'add' | 'clone' | 'remote' | 'create' | 'setup' +type RepoKind = 'git' | 'folder' + +export function useCreateRepo( + fetchWorktrees: (repoId: string) => Promise, + setStep: (step: DialogStep) => void, + setAddedRepo: (repo: Repo | null) => void, + closeModal: () => void +) { + const [createName, setCreateName] = useState('') + const [createParent, setCreateParent] = useState('') + const [createKind, setCreateKind] = useState('git') + const [createError, setCreateError] = useState(null) + const [isCreating, setIsCreating] = useState(false) + + // Why: monotonic ID so stale create callbacks can detect they were superseded + // when the user clicks Back or closes the dialog mid-create. Mirrors the + // cloneGenRef pattern in AddRepoDialog. + const createGenRef = useRef(0) + + const resetCreateState = useCallback(() => { + createGenRef.current++ + setCreateName('') + setCreateParent('') + setCreateKind('git') + setCreateError(null) + setIsCreating(false) + }, []) + + const handlePickParent = useCallback(async () => { + const dir = await window.api.repos.pickDirectory() + if (dir) { + setCreateParent(dir) + setCreateError(null) + } + }, []) + + const handleCreate = useCallback(async () => { + const name = createName.trim() + const parentPath = createParent.trim() + if (!name || !parentPath) { + return + } + const gen = ++createGenRef.current + setIsCreating(true) + setCreateError(null) + try { + const result = await window.api.repos.create({ + parentPath, + name, + kind: createKind + }) + // Why: if the user closed the dialog or clicked Back mid-create, + // createGenRef was bumped by resetCreateState. Ignore stale results. + if (gen !== createGenRef.current) { + return + } + if ('error' in result) { + setCreateError(result.error) + return + } + const repo = result.repo + // Upsert into the store before the repos:changed event round-trips, + // so the next step can find the repo immediately. + const state = useAppStore.getState() + const existingIdx = state.repos.findIndex((r) => r.id === repo.id) + // Why: the IPC handler dedupes by path (see repos:create) and returns + // the existing repo unchanged. If its ID is already in our store, the + // handler took the dedup path — no new project was created, so don't + // claim one was. + const wasDeduped = existingIdx !== -1 + if (existingIdx === -1) { + useAppStore.setState({ repos: [...state.repos, repo] }) + } else { + const updated = [...state.repos] + updated[existingIdx] = repo + useAppStore.setState({ repos: updated }) + } + if (wasDeduped) { + toast.info('Project already added', { + description: repo.displayName + }) + } else { + toast.success('Project created', { + description: repo.displayName + }) + } + if (isGitRepoKind(repo)) { + // Why: setAddedRepo only drives the git "setup" step; the folder + // branch closes the dialog, which resets addedRepo to null anyway. + setAddedRepo(repo) + await fetchWorktrees(repo.id) + if (gen !== createGenRef.current) { + return + } + setStep('setup') + } else { + // Why: without activating the new folder, the dialog closes and users + // see no change. Matches addNonGitFolder's behavior in the store slice. + await fetchWorktrees(repo.id) + if (gen !== createGenRef.current) { + return + } + const folderWorktree = useAppStore.getState().worktreesByRepo[repo.id]?.[0] + if (folderWorktree) { + activateAndRevealWorktree(folderWorktree.id) + } + closeModal() + } + } catch (err) { + if (gen !== createGenRef.current) { + return + } + const message = err instanceof Error ? err.message : String(err) + setCreateError(message) + } finally { + // Why: only clear the loading state if this invocation is still current; + // a superseded create must not flip the flag back off for a new flow. + if (gen === createGenRef.current) { + setIsCreating(false) + } + } + }, [createName, createParent, createKind, fetchWorktrees, setStep, setAddedRepo, closeModal]) + + return { + createName, + createParent, + createKind, + createError, + isCreating, + setCreateName, + setCreateKind, + setCreateError, + resetCreateState, + handlePickParent, + handleCreate + } +} + +// ── UI helpers ─────────────────────────────────────────────────────── + +type KindCardProps = { + kind: RepoKind + selected: boolean + disabled: boolean + onSelect: () => void + onArrowNav: () => void + icon: React.ReactNode + title: string + caption: string +} + +function KindCard({ + kind, + selected, + disabled, + onSelect, + onArrowNav, + icon, + title, + caption +}: KindCardProps): React.JSX.Element { + return ( + + ) +} + +type CreateStepProps = { + createName: string + createParent: string + createKind: RepoKind + createError: string | null + isCreating: boolean + onNameChange: (value: string) => void + onKindChange: (kind: RepoKind) => void + onPickParent: () => void + onCreate: () => void +} + +export function CreateStep({ + createName, + createParent, + createKind, + createError, + isCreating, + onNameChange, + onKindChange, + onPickParent, + onCreate +}: CreateStepProps): React.JSX.Element { + const radioGroupRef = useRef(null) + + // Arrow keys cycle selection within the radiogroup (WAI-ARIA radio pattern). + const cycleKind = useCallback(() => { + const next = createKind === 'git' ? 'folder' : 'git' + onKindChange(next) + requestAnimationFrame(() => { + const nextEl = radioGroupRef.current?.querySelector( + `[data-kind="${next}"]` + ) + nextEl?.focus() + }) + }, [createKind, onKindChange]) + + const trimmedName = createName.trim() + const canSubmit = trimmedName.length > 0 && createParent.trim().length > 0 && !isCreating + + return ( + <> + + Start a new project + + Create a Git repository or a plain folder and open it in Orca. + + + + {/* Why: DialogContent is a CSS grid; grid items default to min-width:auto + (= content size), so a long path inside the Location row would blow out + the dialog width even with flex + truncate on the row itself. min-w-0 + here caps the grid track at the dialog's max-width. */} +
+ {/* Kind toggle. Real radiogroup so screen readers announce it as a choice. */} +
+ onKindChange('git')} + onArrowNav={cycleKind} + icon={} + title="Git repository" + caption="Initializes an empty Git repo" + /> + onKindChange('folder')} + onArrowNav={cycleKind} + icon={} + title="Folder" + caption="Create a new folder" + /> +
+ + {/* Name. Monospaced because it ends up as a directory name. */} +
+ + onNameChange(e.target.value)} + placeholder="my-project" + className="h-11 text-sm font-mono" + disabled={isCreating} + autoFocus + autoComplete="off" + spellCheck={false} + /> +
+ + {/* Location. The "Choose…" button morphs into a summary + Change once picked. */} +
+ Location + + {createParent ? ( +
+ + + + + {createParent} + + +
+ ) : ( + + )} +
+ + {createError && ( +

+ {createError} +

+ )} + + +
+ + ) +} diff --git a/src/renderer/src/components/sidebar/AddRepoDialog.tsx b/src/renderer/src/components/sidebar/AddRepoDialog.tsx index 408bfa7dd77..3f5d5a9beba 100644 --- a/src/renderer/src/components/sidebar/AddRepoDialog.tsx +++ b/src/renderer/src/components/sidebar/AddRepoDialog.tsx @@ -1,6 +1,6 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { toast } from 'sonner' -import { FolderOpen, GitBranchPlus, Settings, ArrowLeft, Globe, Monitor } from 'lucide-react' +import { FolderOpen, ArrowLeft, Globe, Monitor } from 'lucide-react' import { useAppStore } from '@/store' import { Dialog, @@ -11,8 +11,9 @@ import { } from '@/components/ui/dialog' import { Button } from '@/components/ui/button' import { activateAndRevealWorktree } from '@/lib/worktree-activation' -import { LinkedWorktreeItem } from './LinkedWorktreeItem' import { RemoteStep, CloneStep, useRemoteRepo } from './AddRepoSteps' +import { CreateStep, useCreateRepo } from './AddRepoCreateStep' +import { SetupStep } from './AddRepoSetupStep' import { isGitRepoKind } from '../../../../shared/repo-kind' import type { Repo, Worktree } from '../../../../shared/types' @@ -27,7 +28,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() { const openSettingsPage = useAppStore((s) => s.openSettingsPage) const openSettingsTarget = useAppStore((s) => s.openSettingsTarget) - const [step, setStep] = useState<'add' | 'clone' | 'remote' | 'setup'>('add') + const [step, setStep] = useState<'add' | 'clone' | 'remote' | 'create' | 'setup'>('add') const [addedRepo, setAddedRepo] = useState(null) const [isAdding, setIsAdding] = useState(false) const [cloneUrl, setCloneUrl] = useState('') @@ -55,6 +56,20 @@ const AddRepoDialog = React.memo(function AddRepoDialog() { handleAddRemoteRepo, handleConnectTarget } = useRemoteRepo(fetchWorktrees, setStep, setAddedRepo, closeModal) + + const { + createName, + createParent, + createKind, + createError, + isCreating, + setCreateName, + setCreateKind, + setCreateError, + resetCreateState, + handlePickParent, + handleCreate + } = useCreateRepo(fetchWorktrees, setStep, setAddedRepo, closeModal) useEffect(() => { if (!isCloning) { return @@ -79,8 +94,6 @@ const AddRepoDialog = React.memo(function AddRepoDialog() { }) }, [worktrees]) - const hasWorktrees = worktrees.length > 0 - const resetState = useCallback(() => { cloneGenRef.current++ // Why: kill the git clone process if one is running, so backing out @@ -94,8 +107,9 @@ const AddRepoDialog = React.memo(function AddRepoDialog() { setIsCloning(false) setCloneError(null) setCloneProgress(null) + resetCreateState() resetRemoteState() - }, [resetRemoteState]) + }, [resetRemoteState, resetCreateState]) // Why: reset state on close so reopening doesn't show stale step/repo. useEffect(() => { @@ -104,7 +118,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() { } }, [isOpen, resetState]) - const isInputStep = step === 'add' || step === 'clone' || step === 'remote' + const isInputStep = step === 'add' || step === 'clone' || step === 'remote' || step === 'create' const handleBrowse = useCallback(async () => { setIsAdding(true) @@ -217,7 +231,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() { {/* Step indicator row — back button (step 2 only), dots, X is rendered by DialogContent */}
- {(step === 'clone' || step === 'remote') && ( + {(step === 'clone' || step === 'remote' || step === 'create') && (
+ + {/* Secondary link rather than a fourth card — create-from-scratch + is a less common path than importing. See orca#763. */} +
+ +
) : step === 'remote' ? ( + ) : step === 'create' ? ( + { + setCreateName(value) + setCreateError(null) + }} + onKindChange={(kind) => { + setCreateKind(kind) + setCreateError(null) + }} + onPickParent={handlePickParent} + onCreate={handleCreate} + /> ) : ( - <> - - - {hasWorktrees ? 'Open or create a worktree' : 'Set up your first worktree'} - - - {hasWorktrees - ? `${addedRepo?.displayName} has ${worktrees.length} worktree${worktrees.length !== 1 ? 's' : ''}. Open one to pick up where you left off, or create a new one.` - : `Orca uses git worktrees as isolated task environments. Create one for ${addedRepo?.displayName} to get started.`} - - - - {hasWorktrees && ( -
-

- Existing worktrees -

-
- {sortedWorktrees.map((wt) => ( - handleOpenWorktree(wt)} - /> - ))} -
-
- )} - -
- - -
- - -
-
- + { + closeModal() + resetState() + }} + /> )} diff --git a/src/renderer/src/components/sidebar/AddRepoSetupStep.tsx b/src/renderer/src/components/sidebar/AddRepoSetupStep.tsx new file mode 100644 index 00000000000..27fd77a0ef6 --- /dev/null +++ b/src/renderer/src/components/sidebar/AddRepoSetupStep.tsx @@ -0,0 +1,80 @@ +/** + * Setup step for AddRepoDialog — shown after a repo is added, cloned, or created. + * Split out so the parent dialog stays under the 400-line oxlint limit. + */ + +import React from 'react' +import { GitBranchPlus, Settings } from 'lucide-react' +import { DialogHeader, DialogTitle, DialogDescription } from '@/components/ui/dialog' +import { Button } from '@/components/ui/button' +import { LinkedWorktreeItem } from './LinkedWorktreeItem' +import type { Worktree } from '../../../../shared/types' + +type SetupStepProps = { + repoName: string + sortedWorktrees: Worktree[] + onOpenWorktree: (worktree: Worktree) => void + onCreateWorktree: () => void + onConfigureRepo: () => void + onSkip: () => void +} + +export function SetupStep({ + repoName, + sortedWorktrees, + onOpenWorktree, + onCreateWorktree, + onConfigureRepo, + onSkip +}: SetupStepProps): React.JSX.Element { + const hasWorktrees = sortedWorktrees.length > 0 + const worktreeCount = sortedWorktrees.length + + return ( + <> + + + {hasWorktrees ? 'Open or create a worktree' : 'Set up your first worktree'} + + + {hasWorktrees + ? `${repoName} has ${worktreeCount} worktree${worktreeCount !== 1 ? 's' : ''}. Open one to pick up where you left off, or create a new one.` + : `Orca uses git worktrees as isolated task environments. Create one for ${repoName} to get started.`} + + + + {hasWorktrees && ( +
+

+ Existing worktrees +

+
+ {sortedWorktrees.map((wt) => ( + onOpenWorktree(wt)} /> + ))} +
+
+ )} + +
+ + +
+ + +
+
+ + ) +} diff --git a/src/renderer/src/components/sidebar/AddRepoSteps.tsx b/src/renderer/src/components/sidebar/AddRepoSteps.tsx index 438ce858505..396facda057 100644 --- a/src/renderer/src/components/sidebar/AddRepoSteps.tsx +++ b/src/renderer/src/components/sidebar/AddRepoSteps.tsx @@ -21,7 +21,7 @@ import type { SshTarget, SshConnectionState } from '../../../../shared/ssh-types export function useRemoteRepo( fetchWorktrees: (repoId: string) => Promise, - setStep: (step: 'add' | 'clone' | 'remote' | 'setup') => void, + setStep: (step: 'add' | 'clone' | 'remote' | 'create' | 'setup') => void, setAddedRepo: (repo: Repo | null) => void, closeModal: () => void ) {