diff --git a/README.md b/README.md index dfa2ac1..46974f5 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,11 @@ Continue work / Address comments, whose workspace is named `PR # (branch) `). Git allows a branch in one +worktree only, so when that name is taken (checked out in your clone or in the PR's other workspace, a fork +branch named like a local one, a leftover branch) the workspace keeps Orca's branch name and the button shows a +⚠️ warning. + ## Repository page `Clone in Orca` sits before Watch / Fork / Star on the repo home and its `tree/…` / `blob/…` views (not on @@ -108,6 +113,8 @@ Clone in Orca only clones and registers the repo: it creates no worktree and sta - [ ] Review on an open PR → Orca shows the new workspace, agent receives the prompt, board status In review. - [ ] Review again → “(reused)”, a new agent tab in the same workspace. - [ ] Checkout only → workspace without agent; HEAD equals the PR head SHA. +- [ ] Review on a PR whose branch is not checked out locally → the workspace's branch is ``. +- [ ] Review on a PR whose branch is checked out in your clone → ⚠️ “Local branch kept as …”, the agent still starts. - [ ] Continue work → upstream is `origin/`. - [ ] Custom prompt → the agent receives the typed text. - [ ] Multi-line Custom prompt on a reused workspace → sent as one prompt. diff --git a/extension/src/content/button.test.ts b/extension/src/content/button.test.ts index a829bfa..2809014 100644 --- a/extension/src/content/button.test.ts +++ b/extension/src/content/button.test.ts @@ -53,6 +53,13 @@ describe('createOrcaButton', () => { expect(toast()!.dataset.state).toBe('warning'); }); + it('shows a warning on a new workspace without marking it reused', async () => { + const { q } = mount(vi.fn().mockResolvedValue({ ...ok, warning: 'Local branch kept as x' })); + q('.gho-main').click(); + await vi.waitFor(() => expect(toastText()).toBe('⚠️ PR #3 Fix — Local branch kept as x')); + expect(toast()!.dataset.state).toBe('warning'); + }); + it('explains a reloaded extension when the send rejects', async () => { const { q } = mount(vi.fn().mockRejectedValue(new Error('Extension context invalidated.'))); q('.gho-main').click(); diff --git a/extension/src/content/button.ts b/extension/src/content/button.ts index a99b18f..0dd731c 100644 --- a/extension/src/content/button.ts +++ b/extension/src/content/button.ts @@ -96,8 +96,9 @@ export function createOrcaButton(key: string, send: Send, doc: Document = docume controls.forEach((c) => (c.disabled = true)); showStatus('⏳ Starting in Orca…', 'busy'); const res = await sendSafely(() => send(action, customPrompt)); - if (res.ok && res.warning) showStatus(`⚠️ ${res.worktreeName} (reused) — ${res.warning}`, 'warning'); - else if (res.ok) showStatus(`✅ ${res.worktreeName}${res.reused ? ' (reused)' : ''}`, 'ok'); + const reused = res.ok && res.reused ? ' (reused)' : ''; + if (res.ok && res.warning) showStatus(`⚠️ ${res.worktreeName}${reused} — ${res.warning}`, 'warning'); + else if (res.ok) showStatus(`✅ ${res.worktreeName}${reused}`, 'ok'); else showStatus(`❌ ${res.message}`, 'error'); controls.forEach((c) => (c.disabled = false)); busy = false; diff --git a/host/src/git.test.ts b/host/src/git.test.ts index cd2acfb..408debb 100644 --- a/host/src/git.test.ts +++ b/host/src/git.test.ts @@ -26,6 +26,22 @@ describe('GitCli', () => { expect(run.calls[0].args).toEqual(['-C', '/wt', 'branch', '--set-upstream-to=origin/feat/x']); }); + it('renames the worktree branch', async () => { + const run = fakeRunner([['git', '']]); + expect(await new GitCli(run).renameBranch('/wt', 'feat/x')).toBe(true); + expect(run.calls[0].args).toEqual(['-C', '/wt', 'branch', '-m', 'feat/x']); + }); + + it('returns false when git refuses the rename (name taken)', async () => { + const run = fakeRunner([['git', new CommandError('git', '', "fatal: a branch named 'feat/x' already exists", false)]]); + expect(await new GitCli(run).renameBranch('/wt', 'feat/x')).toBe(false); + }); + + it('still throws timeout when the rename is killed', async () => { + const run = fakeRunner([['git', new CommandError('git', '', '', true)]]); + await expect(new GitCli(run).renameBranch('/wt', 'feat/x')).rejects.toMatchObject({ code: 'timeout' }); + }); + it('maps failures to git_failed with stderr', async () => { const run = fakeRunner([['git', new CommandError('git', '', "fatal: couldn't find remote ref pull/12/head", false)]]); await expect(new GitCli(run).fetchPrRef('/repo', 12)).rejects.toMatchObject({ code: 'git_failed', message: expect.stringContaining('pull/12/head') }); diff --git a/host/src/git.ts b/host/src/git.ts index c347a11..9354cac 100644 --- a/host/src/git.ts +++ b/host/src/git.ts @@ -30,12 +30,20 @@ export class GitCli implements GitApi { await this.git(['-C', worktreePath, 'branch', `--set-upstream-to=origin/${branch}`]); } + async renameBranch(worktreePath: string, name: string): Promise { + return this.tolerateRefusal(['-C', worktreePath, 'branch', '-m', name]); + } + async fastForward(worktreePath: string, ref: string): Promise { + return this.tolerateRefusal(['-C', worktreePath, 'merge', '--ff-only', ref]); + } + + /** true when git succeeds, false when it refuses: not fatal, the caller warns instead. Timeouts still throw. */ + private async tolerateRefusal(args: string[]): Promise { try { - await this.run('git', ['-C', worktreePath, 'merge', '--ff-only', ref], { timeoutMs: 60_000, env: GIT_ENV }); + await this.run('git', args, { timeoutMs: 60_000, env: GIT_ENV }); return true; } catch (e) { - // A refused merge (dirty tree, diverged history) is not fatal: the caller warns instead. if (e instanceof CommandError && !e.timedOut) return false; throw toHostError(e, 'git_failed'); } diff --git a/host/src/handler.test.ts b/host/src/handler.test.ts index c7f796d..41696ab 100644 --- a/host/src/handler.test.ts +++ b/host/src/handler.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { HostRequest, PrMeta } from '../../shared/types'; import { HostError } from './errors'; -import { handleRequest, hasMarker, workspaceMarker, worktreeName, type Deps } from './handler'; +import { branchKeptWarning, handleRequest, hasMarker, workspaceMarker, worktreeName, type Deps } from './handler'; import type { WorktreeInfo } from './ports'; const pr: PrMeta = { @@ -18,6 +18,7 @@ function makeDeps(overrides: { pr?: Partial; worktrees?: WorktreeInfo[] fetchPrRef: vi.fn().mockResolvedValue('origin/pr/12'), fetchBranch: vi.fn().mockResolvedValue('origin/fix/login'), setUpstream: vi.fn().mockResolvedValue(undefined), + renameBranch: vi.fn().mockResolvedValue(true), fastForward: vi.fn().mockResolvedValue(true), }, orca: { @@ -117,6 +118,27 @@ describe('handleRequest — new workspace', () => { expect(deps.orca.setStatus).toHaveBeenCalledWith(created.id, 'in-progress'); }); + it('names the local branch after the PR head branch, for every action', async () => { + for (const action of ['review', 'checkout', 'continue', 'address-comments'] as const) { + const deps = makeDeps(); + const res = await handleRequest({ ...review, action }, deps); + expect(deps.git.renameBranch).toHaveBeenCalledWith(created.path, 'fix/login'); + expect(res).not.toHaveProperty('warning'); + } + }); + + it('renames the branch before setting its upstream', async () => { + await handleRequest({ ...review, action: 'continue' }, deps); + expect(deps.git.renameBranch.mock.invocationCallOrder[0]).toBeLessThan(deps.git.setUpstream.mock.invocationCallOrder[0]); + }); + + it('keeps Orca\'s branch name with a warning when the PR branch name is taken', async () => { + deps.git.renameBranch.mockResolvedValue(false); + const res = await handleRequest(review, deps); + expect(res).toMatchObject({ ok: true, reused: false, warning: branchKeptWarning('x', 'fix/login') }); + expect(deps.orca.setStatus).toHaveBeenCalledWith(created.id, 'in-review'); + }); + it('runs repo setup hooks for same-repo PRs', async () => { await handleRequest(review, deps); expect(deps.orca.createWorktree.mock.calls[0][0]).not.toHaveProperty('setup'); diff --git a/host/src/handler.ts b/host/src/handler.ts index 1f752c0..f0acb96 100644 --- a/host/src/handler.ts +++ b/host/src/handler.ts @@ -22,6 +22,10 @@ const MAX_NAME = 80; export const STALE_WARNING = "Workspace not updated (local changes or diverged history): the agent works on an older version of the PR"; +export function branchKeptWarning(current: string, wanted: string): string { + return `Local branch kept as ${current}: ${wanted} already exists`; +} + /** Actions that work on the PR's real branch (fetch origin/, upstream set, can push). */ export function isBranchAction(action: Action): boolean { return action === 'continue' || action === 'address-comments'; @@ -106,10 +110,13 @@ async function run(req: HostRequest, { orca, git, gh }: Deps): Promise { diff --git a/host/src/ports.ts b/host/src/ports.ts index 6246600..a4f0a50 100644 --- a/host/src/ports.ts +++ b/host/src/ports.ts @@ -56,6 +56,8 @@ export interface GitApi { /** Fetches origin/; returns the base ref `origin/`. */ fetchBranch(repoPath: string, branch: string): Promise; setUpstream(worktreePath: string, branch: string): Promise; + /** `branch -m ` in the worktree: true when done, false when git refuses (name already taken). */ + renameBranch(worktreePath: string, name: string): Promise; /** `merge --ff-only ` in the worktree: true when done or already up to date, false when git refuses. */ fastForward(worktreePath: string, ref: string): Promise; }