diff --git a/package.json b/package.json index 3cb64d8a04d..97b3929fe1a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.4.73-rc.2", + "version": "1.4.73-rc.3", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai", diff --git a/src/main/agent-hooks/first-work-branch-rename.test.ts b/src/main/agent-hooks/first-work-branch-rename.test.ts index fedd84b59c2..712d6534180 100644 --- a/src/main/agent-hooks/first-work-branch-rename.test.ts +++ b/src/main/agent-hooks/first-work-branch-rename.test.ts @@ -103,6 +103,17 @@ describe('maybeAutoRenameBranchOnFirstWork', () => { expect(renameWorktreeFolder).toHaveBeenCalledWith(WORKTREE_ID, 'fix-auth') }) + it('keeps branch and display rename working when folder rename is disabled', async () => { + const { deps, onRenamed, setDisplayName } = makeDeps({ renameWorktreeFolder: undefined }) + await maybeAutoRenameBranchOnFirstWork(workingEvent(), deps) + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['branch', '-m', 'you/fix-auth'], + expect.objectContaining({ cwd: '/repo/wt' }) + ) + expect(setDisplayName).toHaveBeenCalledWith(WORKTREE_ID, 'Fix auth') + expect(onRenamed).toHaveBeenCalledWith(REPO_ID) + }) + it('skips the redundant branch-rename notify when the folder rename succeeded', async () => { // The folder rename already pushed a worktrees:changed carrying the id mapping, // so onRenamed would only trigger a second, redundant renderer re-list. diff --git a/src/main/azure-devops/pull-request-creation.test.ts b/src/main/azure-devops/pull-request-creation.test.ts new file mode 100644 index 00000000000..dfcb4c82b4e --- /dev/null +++ b/src/main/azure-devops/pull-request-creation.test.ts @@ -0,0 +1,155 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createAzureDevOpsPullRequest, + isAzureDevOpsReviewCreationAuthenticated +} from './pull-request-creation' +import { _resetAzureDevOpsRepoRefCache } from './repository-ref' + +const { gitExecFileAsyncMock, getSshGitProviderMock } = vi.hoisted(() => ({ + gitExecFileAsyncMock: vi.fn(), + getSshGitProviderMock: vi.fn() +})) + +vi.mock('../git/runner', () => ({ + gitExecFileAsync: gitExecFileAsyncMock +})) + +vi.mock('../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: getSshGitProviderMock +})) + +vi.mock('../source-control/pull-request-template', () => ({ + readHostedPullRequestTemplate: vi.fn(async () => 'Template body') +})) + +const OLD_ENV = process.env +const OLD_FETCH = globalThis.fetch + +describe('Azure DevOps pull request creation', () => { + beforeEach(() => { + process.env = { ...OLD_ENV, ORCA_AZURE_DEVOPS_TOKEN: 'pat-token' } + gitExecFileAsyncMock.mockReset() + getSshGitProviderMock.mockReset() + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'https://dev.azure.com/acme/Project/_git/repo\n', + stderr: '' + }) + _resetAzureDevOpsRepoRefCache() + }) + + afterEach(() => { + process.env = OLD_ENV + globalThis.fetch = OLD_FETCH + _resetAzureDevOpsRepoRefCache() + }) + + it('treats token-only auth as sufficient for repo-scoped creation', () => { + delete process.env.ORCA_AZURE_DEVOPS_API_BASE_URL + expect(isAzureDevOpsReviewCreationAuthenticated()).toBe(true) + }) + + it('posts a pull request create body to the repository REST endpoint', async () => { + const fetchMock = vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + const url = new URL(String(input)) + expect(url.pathname).toBe('/acme/Project/_apis/git/repositories/repo/pullRequests') + expect(url.searchParams.get('api-version')).toBe('7.1') + expect(init).toBeDefined() + const requestInit = init! + expect(requestInit.method).toBe('POST') + expect((requestInit.headers as Record).Authorization).toMatch(/^Basic /) + expect(JSON.parse(String(requestInit.body))).toEqual({ + sourceRefName: 'refs/heads/feature/azure', + targetRefName: 'refs/heads/main', + title: 'Add Azure create', + description: 'Body', + isDraft: true + }) + return Response.json({ + pullRequestId: 37, + title: 'Add Azure create', + status: 'active', + isDraft: true, + creationDate: '2026-06-01T00:00:00Z', + _links: { + web: { + href: 'https://dev.azure.com/acme/Project/_git/repo/pullrequest/37' + } + } + }) + }) + globalThis.fetch = fetchMock as never + + await expect( + createAzureDevOpsPullRequest('/repo', { + provider: 'azure-devops', + base: 'origin/main', + head: 'refs/heads/feature/azure', + title: 'Add Azure create', + body: 'Body', + draft: true + }) + ).resolves.toEqual({ + ok: true, + number: 37, + url: 'https://dev.azure.com/acme/Project/_git/repo/pullrequest/37' + }) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('resolves Azure DevOps remotes through the SSH git provider', async () => { + const remoteGit = { + exec: vi.fn(async () => ({ + stdout: 'git@ssh.dev.azure.com:v3/acme/Project/repo.git\n', + stderr: '' + })) + } + getSshGitProviderMock.mockReturnValue(remoteGit) + globalThis.fetch = vi.fn(async () => + Response.json({ + pullRequestId: 38, + title: 'Remote Azure create', + status: 'active', + creationDate: '2026-06-01T00:00:00Z' + }) + ) as never + + await expect( + createAzureDevOpsPullRequest( + '/remote/repo', + { + provider: 'azure-devops', + base: 'main', + head: 'feature/azure', + title: 'Remote Azure create' + }, + 'ssh-1' + ) + ).resolves.toMatchObject({ + ok: true, + number: 38 + }) + expect(remoteGit.exec).toHaveBeenCalledWith(['remote', 'get-url', 'origin'], '/remote/repo') + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + + it('classifies auth failures without retrying shell commands', async () => { + globalThis.fetch = vi.fn(async () => + Response.json({ message: 'Unauthorized' }, { status: 401 }) + ) as never + + await expect( + createAzureDevOpsPullRequest('/repo', { + provider: 'azure-devops', + base: 'main', + head: 'feature/azure', + title: 'Add Azure create' + }) + ).resolves.toMatchObject({ + ok: false, + code: 'auth_required' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'get-url', 'origin'], { + cwd: '/repo' + }) + }) +}) diff --git a/src/main/azure-devops/pull-request-creation.ts b/src/main/azure-devops/pull-request-creation.ts new file mode 100644 index 00000000000..ddd7a2c5a9e --- /dev/null +++ b/src/main/azure-devops/pull-request-creation.ts @@ -0,0 +1,238 @@ +import { Buffer } from 'buffer' +import type { CreateHostedReviewInput, CreateHostedReviewResult } from '../../shared/hosted-review' +import { + normalizeHostedReviewBaseRef, + normalizeHostedReviewHeadRef +} from '../../shared/hosted-review-refs' +import { + HostedReviewApiRequestError, + requestHostedReviewJson +} from '../source-control/hosted-review-api-request' +import { readHostedPullRequestTemplate } from '../source-control/pull-request-template' +import { getAzureDevOpsPullRequestForBranch } from './client' +import { mapAzureDevOpsPullRequest, type RawAzureDevOpsPullRequest } from './pull-request-mappers' +import { getAzureDevOpsRepoRef, type AzureDevOpsRepoRef } from './repository-ref' + +const CREATE_REQUEST_TIMEOUT_MS = 60_000 + +type AzureDevOpsCreateAuthConfig = { + apiBaseUrl: string | null + pat: string | null + accessToken: string | null + username: string | null +} + +function envValue(name: string): string | null { + const value = process.env[name]?.trim() ?? '' + return value.length > 0 ? value : null +} + +function normalizeApiBaseUrl(value: string): string { + return value + .trim() + .replace(/\/+$/, '') + .replace(/\/_apis$/i, '') +} + +function getAuthConfig(): AzureDevOpsCreateAuthConfig { + return { + apiBaseUrl: envValue('ORCA_AZURE_DEVOPS_API_BASE_URL'), + pat: envValue('ORCA_AZURE_DEVOPS_TOKEN') ?? envValue('ORCA_AZURE_DEVOPS_PAT'), + accessToken: envValue('ORCA_AZURE_DEVOPS_ACCESS_TOKEN'), + username: envValue('ORCA_AZURE_DEVOPS_USERNAME') + } +} + +export function isAzureDevOpsReviewCreationAuthenticated(): boolean { + const config = getAuthConfig() + return Boolean(config.pat || config.accessToken) +} + +function authHeaders(config: AzureDevOpsCreateAuthConfig): Record { + if (config.accessToken) { + return { Authorization: `Bearer ${config.accessToken}` } + } + if (config.pat) { + const encoded = Buffer.from(`${config.username ?? ''}:${config.pat}`).toString('base64') + return { Authorization: `Basic ${encoded}` } + } + return {} +} + +function apiUrl(repo: AzureDevOpsRepoRef, path: string): URL { + const config = getAuthConfig() + const baseUrl = config.apiBaseUrl ? normalizeApiBaseUrl(config.apiBaseUrl) : repo.apiBaseUrl + const url = new URL(`${baseUrl.replace(/\/+$/, '')}${path}`) + url.searchParams.set('api-version', '7.1') + return url +} + +function encodePathSegment(value: string): string { + return encodeURIComponent(value) +} + +function azureBranchRef(branch: string): string { + return `refs/heads/${branch.replace(/^refs\/heads\//, '')}` +} + +function apiErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +function classifyCreateError(error: unknown): CreateHostedReviewResult { + const message = apiErrorMessage(error) + if (message) { + console.warn('createAzureDevOpsPullRequest failed:', message) + } + const lower = message.toLowerCase() + const status = error instanceof HostedReviewApiRequestError ? error.status : null + if ( + status === 401 || + status === 403 || + lower.includes('unauthorized') || + lower.includes('forbidden') || + lower.includes('authentication') + ) { + return { + ok: false, + code: 'auth_required', + error: + 'Create PR failed: Azure DevOps is not authenticated. Next step: set ORCA_AZURE_DEVOPS_TOKEN in this environment.' + } + } + if (status === 409 || lower.includes('already exists') || lower.includes('active pull request')) { + return { + ok: false, + code: 'already_exists', + error: 'A pull request already exists for this branch.' + } + } + if (error instanceof HostedReviewApiRequestError && error.timedOut) { + return { + ok: false, + code: 'unknown_completion', + error: 'PR creation may have completed. Refreshing branch review state...' + } + } + if (status === 400 || status === 422 || lower.includes('validation')) { + return { + ok: false, + code: 'validation', + error: + 'Create PR failed: Azure DevOps rejected the pull request. Check the base branch and branch state, then try again.' + } + } + return { + ok: false, + code: 'unknown', + error: + 'Create PR failed: Azure DevOps could not create the pull request. Try again in a moment.' + } +} + +async function findExistingPullRequest( + repoPath: string, + head: string, + connectionId?: string | null +): Promise<{ number: number; url: string } | null> { + const existing = await getAzureDevOpsPullRequestForBranch(repoPath, head, null, connectionId) + return existing ? { number: existing.number, url: existing.url } : null +} + +export async function createAzureDevOpsPullRequest( + repoPath: string, + input: CreateHostedReviewInput, + connectionId?: string | null +): Promise { + if (input.provider !== 'azure-devops') { + return { + ok: false, + code: 'unsupported_provider', + error: 'Creating reviews for this provider is not supported yet.' + } + } + + const repo = await getAzureDevOpsRepoRef(repoPath, connectionId) + if (!repo) { + return { + ok: false, + code: 'unsupported_provider', + error: 'Creating pull requests requires an Azure DevOps remote.' + } + } + + const base = normalizeHostedReviewBaseRef(input.base) + const head = input.head ? normalizeHostedReviewHeadRef(input.head) : '' + const title = input.title.trim() + if (!base || !head || !title) { + return { + ok: false, + code: 'validation', + error: 'Create PR failed: base branch, head branch, and title are required.' + } + } + if (head.toLowerCase() === base.toLowerCase()) { + return { + ok: false, + code: 'validation', + error: 'Create PR failed: choose a different base branch before creating a pull request.' + } + } + + const body = + input.useTemplate && !input.body?.trim() + ? await readHostedPullRequestTemplate(repoPath, connectionId) + : (input.body ?? '') + const requestBody = { + sourceRefName: azureBranchRef(head), + targetRefName: azureBranchRef(base), + title, + description: body, + ...(input.draft ? { isDraft: true } : {}) + } + + try { + const raw = await requestHostedReviewJson( + apiUrl(repo, `/_apis/git/repositories/${encodePathSegment(repo.repository)}/pullRequests`), + { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + ...authHeaders(getAuthConfig()) + }, + body: JSON.stringify(requestBody) + }, + CREATE_REQUEST_TIMEOUT_MS + ) + const created = mapAzureDevOpsPullRequest(raw, 'neutral', repo.webBaseUrl) + if (created) { + return { ok: true, number: created.number, url: created.url } + } + const found = await findExistingPullRequest(repoPath, head, connectionId).catch(() => null) + return found + ? { ok: true, ...found } + : { + ok: false, + code: 'unknown_completion', + error: 'PR creation may have completed. Refreshing branch review state...' + } + } catch (error) { + const classified = classifyCreateError(error) + if ( + !classified.ok && + (classified.code === 'already_exists' || classified.code === 'unknown_completion') + ) { + const existing = await findExistingPullRequest(repoPath, head, connectionId).catch(() => null) + if (existing) { + return { + ok: false, + code: 'already_exists', + error: 'A pull request already exists for this branch.', + existingReview: existing + } + } + } + return classified + } +} diff --git a/src/main/daemon/pty-subprocess.test.ts b/src/main/daemon/pty-subprocess.test.ts index 8995ee38212..be617627659 100644 --- a/src/main/daemon/pty-subprocess.test.ts +++ b/src/main/daemon/pty-subprocess.test.ts @@ -54,6 +54,7 @@ const ORCA_SHELL_WRAPPER_ENV = [ ] as const const POWERSHELL_OSC133_COMMAND_ARGS = ['-NoLogo', '-NoExit', '-EncodedCommand', expect.any(String)] const ZSH_SHELL_READY_DIR = /shell-ready[\\/]zsh/ +const itOnMacHost = process.platform === 'darwin' ? it : it.skip function mockPtyProcess(pid = 12345) { const onDataListeners: ((data: string) => void)[] = [] @@ -148,7 +149,7 @@ describe('createPtySubprocess', () => { ) }) - it('repairs a deleted macOS daemon cwd before spawning node-pty', () => { + itOnMacHost('repairs a deleted macOS daemon cwd before spawning node-pty', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) const platform = Object.getOwnPropertyDescriptor(process, 'platform') @@ -231,7 +232,11 @@ describe('createPtySubprocess', () => { }) expect(handle.getForegroundProcess()).toBe('node') - expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith(proc.pid, 'node') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith( + proc.pid, + 'node', + expect.any(Object) + ) resolveForeground('codex') await vi.waitFor(() => expect(handle.getForegroundProcess()).toBe('codex')) @@ -263,7 +268,11 @@ describe('createPtySubprocess', () => { }) expect(handle.getForegroundProcess()).toBe('node.exe') - expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith(proc.pid, 'node.exe') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith( + proc.pid, + 'node.exe', + expect.any(Object) + ) resolveForeground('codex') await vi.waitFor(() => expect(handle.getForegroundProcess()).toBe('codex')) @@ -274,6 +283,197 @@ describe('createPtySubprocess', () => { } }) + it('serves daemon shell-rooted agent foreground from an async cache', async () => { + const proc = mockPtyProcess() + proc.process = 'powershell.exe' + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + let resolveForeground!: (processName: string) => void + resolveAgentForegroundProcessMock.mockReturnValue( + new Promise((resolve) => { + resolveForeground = resolve + }) + ) + + try { + const handle = createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24 + }) + + expect(handle.getForegroundProcess()).toBe('powershell.exe') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith( + proc.pid, + 'powershell.exe', + expect.any(Object) + ) + + resolveForeground('codex') + await vi.waitFor(() => expect(handle.getForegroundProcess()).toBe('codex')) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + + it('does not let stale shell enrichment clear a newer direct agent foreground cache', async () => { + const proc = mockPtyProcess() + proc.process = 'powershell.exe' + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + let resolveForeground!: (processName: string) => void + resolveAgentForegroundProcessMock.mockReturnValue( + new Promise((resolve) => { + resolveForeground = resolve + }) + ) + + try { + const handle = createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24 + }) + + expect(handle.getForegroundProcess()).toBe('powershell.exe') + proc.process = 'codex' + expect(handle.getForegroundProcess()).toBe('codex') + + resolveForeground('powershell.exe') + await Promise.resolve() + await Promise.resolve() + + proc.process = 'powershell.exe' + expect(handle.getForegroundProcess()).toBe('codex') + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + + it('keeps menu startup agent foreground through early negative shell enrichment', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-06-16T12:00:00.000Z')) + const proc = mockPtyProcess() + proc.process = 'powershell.exe' + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + resolveAgentForegroundProcessMock.mockResolvedValue('powershell.exe') + + try { + const handle = createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + command: 'codex' + }) + + expect(handle.getForegroundProcess()).toBe('codex') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith( + proc.pid, + 'powershell.exe', + expect.any(Object) + ) + + await Promise.resolve() + expect(handle.getForegroundProcess()).toBe('codex') + + vi.advanceTimersByTime(4_999) + expect(handle.getForegroundProcess()).toBe('codex') + + vi.advanceTimersByTime(2) + expect(handle.getForegroundProcess()).toBe('powershell.exe') + } finally { + vi.useRealTimers() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + + it('keeps menu startup agent foreground during a slow Windows shell enrichment window', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-06-16T12:00:00.000Z')) + const proc = mockPtyProcess() + proc.process = 'powershell.exe' + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + let resolveForeground!: (processName: string) => void + resolveAgentForegroundProcessMock.mockReturnValue( + new Promise((resolve) => { + resolveForeground = resolve + }) + ) + + try { + const handle = createPtySubprocess({ + sessionId: 'repo::C:\\repo\\orca@@deadbeef', + cols: 80, + rows: 24, + cwd: 'C:\\repo\\orca', + command: 'codex' + }) + + expect(handle.getForegroundProcess()).toBe('codex') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith( + proc.pid, + 'powershell.exe', + expect.objectContaining({ + contextPaths: expect.arrayContaining(['C:\\repo\\orca']) + }) + ) + + vi.advanceTimersByTime(2_500) + expect(handle.getForegroundProcess()).toBe('codex') + + resolveForeground('powershell.exe') + await vi.runAllTimersAsync() + } finally { + vi.useRealTimers() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + + it('uses the spawned Windows shell when node-pty reports only the terminal name', async () => { + const proc = mockPtyProcess() + proc.process = 'xterm-256color' + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + resolveAgentForegroundProcessMock.mockResolvedValue('codex') + + try { + const handle = createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + shellOverride: 'powershell.exe' + }) + + expect(handle.getForegroundProcess()).toBe('powershell.exe') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith( + proc.pid, + 'powershell.exe', + expect.any(Object) + ) + + await vi.waitFor(() => expect(handle.getForegroundProcess()).toBe('codex')) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + it('does not schedule foreground enrichment for arbitrary Windows TUIs', () => { const proc = mockPtyProcess() proc.process = 'vim.exe' @@ -301,14 +501,22 @@ describe('createPtySubprocess', () => { const proc = mockPtyProcess() proc.process = 'xterm-256color' spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'linux' }) - const handle = createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24 - }) + try { + const handle = createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24 + }) - expect(handle.getForegroundProcess()).toBeNull() + expect(handle.getForegroundProcess()).toBeNull() + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } }) it('does not inherit parent Orca pane identity when caller omits pane env', () => { diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index 792bcc32415..fb02428f20e 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -30,12 +30,17 @@ import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell' import { resolveAgentForegroundProcess } from '../providers/agent-foreground-process' import { isAgentForegroundWrapperProcess, - recognizeAgentProcess + recognizeAgentProcess, + recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' import { isShellProcess } from '../../shared/shell-process-detection' +import { parsePtySessionId } from './pty-session-id' +import { getAgentForegroundContextPaths } from '../providers/agent-foreground-context-paths' const PANE_IDENTITY_ENV_KEYS = ['ORCA_PANE_KEY', 'ORCA_TAB_ID', 'ORCA_WORKTREE_ID'] as const const FOREGROUND_AGENT_CACHE_TTL_MS = 1000 +const SHELL_FOREGROUND_REFRESH_RETRY_MS = 5_000 +const STARTUP_AGENT_FOREGROUND_BOOTSTRAP_MS = 5_000 const PTY_SPAWN_HEALTH_TIMEOUT_MS = 2_000 export type PtySubprocessOptions = { @@ -316,6 +321,19 @@ function normalizeForegroundProcessName(processName: string | null | undefined): return trimmed.split(/[\\/]/).pop() || null } +function resolveFallbackForegroundProcess( + processName: string | null | undefined, + shellPath: string +): string | null { + const normalized = normalizeForegroundProcessName(processName) + if (normalized || process.platform !== 'win32') { + return normalized + } + // Why: Windows node-pty can report the terminal name instead of the shell. + // Use the spawned shell so shell-rooted foreground enrichment still runs. + return normalizeForegroundProcessName(pathWin32.basename(shellPath)) +} + export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandle { const size = normalizePtySize(opts.cols, opts.rows) const env: Record = { @@ -540,40 +558,84 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl let disposed = false let nodePtyKillIssued = false let cachedAgentForeground: { processName: string; refreshedAt: number } | null = null + const agentForegroundContextPaths = getAgentForegroundContextPaths({ + cwd: opts.cwd, + worktreeId: parsePtySessionId(opts.sessionId).worktreeId + }) + const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command) + let startupAgentForeground: { processName: string; expiresAt: number } | null = + startupAgentRecognition + ? { + processName: startupAgentRecognition.processName, + expiresAt: Date.now() + STARTUP_AGENT_FOREGROUND_BOOTSTRAP_MS + } + : null let foregroundRefreshInFlight = false let lastForegroundRefreshStartedAt = 0 const getFallbackForegroundProcess = (): string | null => - normalizeForegroundProcessName(proc.process) + resolveFallbackForegroundProcess(proc.process, shellPath) + const getActiveStartupAgentForeground = ( + now = Date.now() + ): { processName: string; expiresAt: number } | null => { + if (!startupAgentForeground) { + return null + } + if (now > startupAgentForeground.expiresAt) { + startupAgentForeground = null + return null + } + return startupAgentForeground + } + const shouldInspectFallbackForegroundProcess = (fallbackProcess: string | null): boolean => + fallbackProcess !== null && + (isShellProcess(fallbackProcess) || isAgentForegroundWrapperProcess(fallbackProcess)) const scheduleAgentForegroundRefresh = (fallbackProcess: string | null): void => { if (dead || !proc.pid) { return } + const fallbackIsShell = fallbackProcess !== null && isShellProcess(fallbackProcess) if ( !fallbackProcess || - isShellProcess(fallbackProcess) || recognizeAgentProcess(fallbackProcess) || - !isAgentForegroundWrapperProcess(fallbackProcess) + !shouldInspectFallbackForegroundProcess(fallbackProcess) ) { return } const now = Date.now() - if ( - foregroundRefreshInFlight || - now - lastForegroundRefreshStartedAt < FOREGROUND_AGENT_CACHE_TTL_MS - ) { + const retryMs = + fallbackIsShell && !getActiveStartupAgentForeground(now) && !cachedAgentForeground + ? SHELL_FOREGROUND_REFRESH_RETRY_MS + : FOREGROUND_AGENT_CACHE_TTL_MS + if (foregroundRefreshInFlight || now - lastForegroundRefreshStartedAt < retryMs) { return } foregroundRefreshInFlight = true lastForegroundRefreshStartedAt = now - // Why: daemon `getForegroundProcess()` is sync and runs on the IPC hot path. - // Refresh wrapper-derived identities (node/python → codex/gemini/etc.) in - // the background and serve them from a short cache on later reads. - void resolveAgentForegroundProcess(proc.pid, fallbackProcess) + // Why: daemon foreground reads are sync and run on the IPC hot path. + // Refresh shell/wrapper-derived identities (powershell/node -> codex/etc.) + // in the background and serve them from a short cache on later reads. + void resolveAgentForegroundProcess(proc.pid, fallbackProcess, { + contextPaths: agentForegroundContextPaths + }) .then((processName) => { - if (dead || !processName || !recognizeAgentProcess(processName)) { + if (dead) { + return + } + if (!processName || !recognizeAgentProcess(processName)) { + const currentFallbackProcess = getFallbackForegroundProcess() + if ( + fallbackIsShell && + !getActiveStartupAgentForeground() && + currentFallbackProcess !== null && + isShellProcess(currentFallbackProcess) + ) { + cachedAgentForeground = null + startupAgentForeground = null + } return } cachedAgentForeground = { processName, refreshedAt: Date.now() } + startupAgentForeground = null }) .catch(() => { // Best-effort only: foreground enrichment must never affect PTY health. @@ -585,6 +647,7 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl proc.onExit(() => { dead = true cachedAgentForeground = null + startupAgentForeground = null // Why: UnixTerminal.destroy() registers `_socket.once('close', () => this.kill('SIGHUP'))` // (unixTerminal.js:219-229). After the child exits, the master socket's // 'close' event can fire before our dispose() path gets to neutralize @@ -611,21 +674,23 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl } try { const fallbackProcess = getFallbackForegroundProcess() - if (fallbackProcess && isShellProcess(fallbackProcess)) { - cachedAgentForeground = null - return fallbackProcess - } if (fallbackProcess && recognizeAgentProcess(fallbackProcess)) { cachedAgentForeground = { processName: fallbackProcess, refreshedAt: Date.now() } + startupAgentForeground = null return fallbackProcess } scheduleAgentForegroundRefresh(fallbackProcess) + const now = Date.now() if ( cachedAgentForeground && - Date.now() - cachedAgentForeground.refreshedAt <= FOREGROUND_AGENT_CACHE_TTL_MS + now - cachedAgentForeground.refreshedAt <= FOREGROUND_AGENT_CACHE_TTL_MS ) { return cachedAgentForeground.processName } + const activeStartupAgentForeground = getActiveStartupAgentForeground(now) + if (fallbackProcess && isShellProcess(fallbackProcess) && activeStartupAgentForeground) { + return activeStartupAgentForeground.processName + } return fallbackProcess } catch { return null diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index d97771e0959..e54ea4def8c 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -1,7 +1,10 @@ /* eslint-disable max-lines -- Why: git status/discard/chunking behavior is verified together here to keep the command contract readable in one place. */ import { beforeEach, describe, expect, it, vi } from 'vitest' import path from 'path' -import { MAX_RENDERED_DIFF_COMBINED_CHARACTERS } from '../../shared/large-diff-render-limit' +import { + MAX_RENDERED_DIFF_COMBINED_CHARACTERS, + MAX_RENDERED_DIFF_LINES_PER_SIDE +} from '../../shared/large-diff-render-limit' const { gitExecFileAsyncMock, @@ -384,6 +387,33 @@ describe('getDiff', () => { ) }) + it('omits over-limit text bodies when line-count exceeds the cap', async () => { + const oversizedByLines = 'x\n'.repeat(MAX_RENDERED_DIFF_LINES_PER_SIDE) + gitExecFileAsyncBufferMock.mockResolvedValueOnce({ stdout: Buffer.from('index-content\n') }) + statMock.mockResolvedValueOnce({ + isFile: () => true, + size: oversizedByLines.length + }) + readFileMock.mockResolvedValue(Buffer.from(oversizedByLines)) + + const result = await getDiff('/repo', 'dist/large-lines.log', false) + + expect(result.kind).toBe('text') + if (result.kind !== 'text') { + throw new Error('expected text diff result') + } + expect(result.originalContent).toBe('') + expect(result.modifiedContent).toBe('') + expect(result.largeDiffRenderLimit?.limited).toBe(true) + if (result.largeDiffRenderLimit?.limited !== true) { + throw new Error('expected large diff render limit') + } + expect(result.largeDiffRenderLimit.reason).toBe('line-count') + expect(result.largeDiffRenderLimit.lineCounts?.modified).toBeGreaterThan( + MAX_RENDERED_DIFF_LINES_PER_SIDE + ) + }) + it('marks git blobs that overflow maxBuffer as binary instead of pretending they are missing', async () => { gitExecFileAsyncBufferMock.mockRejectedValueOnce( Object.assign(new Error('stdout maxBuffer length exceeded'), { code: 'ENOBUFS' }) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 8582c1d998a..5ac3d5eacc0 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -1072,6 +1072,8 @@ function buildDiffResult( } as GitDiffResult } + // Why: if the diff exceeds safe render limits, avoid sending large text + // payloads and return metadata so the renderer can show fallback UI. const largeDiffRenderLimit = getLargeDiffRenderLimit({ originalContent, modifiedContent }) if (largeDiffRenderLimit.limited) { return { diff --git a/src/main/gitea/pull-request-creation.test.ts b/src/main/gitea/pull-request-creation.test.ts new file mode 100644 index 00000000000..9680726c302 --- /dev/null +++ b/src/main/gitea/pull-request-creation.test.ts @@ -0,0 +1,155 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createGiteaPullRequest, isGiteaReviewCreationAuthenticated } from './pull-request-creation' +import { _resetGiteaRepoRefCache } from './repository-ref' + +const { gitExecFileAsyncMock, getSshGitProviderMock } = vi.hoisted(() => ({ + gitExecFileAsyncMock: vi.fn(), + getSshGitProviderMock: vi.fn() +})) + +vi.mock('../git/runner', () => ({ + gitExecFileAsync: gitExecFileAsyncMock +})) + +vi.mock('../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: getSshGitProviderMock +})) + +vi.mock('../source-control/pull-request-template', () => ({ + readHostedPullRequestTemplate: vi.fn(async () => 'Template body') +})) + +const OLD_ENV = process.env +const OLD_FETCH = globalThis.fetch + +describe('Gitea pull request creation', () => { + beforeEach(() => { + process.env = { ...OLD_ENV, ORCA_GITEA_TOKEN: 'gitea-token' } + gitExecFileAsyncMock.mockReset() + getSshGitProviderMock.mockReset() + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'https://git.example.com/code/team/repo.git\n', + stderr: '' + }) + _resetGiteaRepoRefCache() + }) + + afterEach(() => { + process.env = OLD_ENV + globalThis.fetch = OLD_FETCH + _resetGiteaRepoRefCache() + }) + + it('requires a token for repo-scoped creation', () => { + expect(isGiteaReviewCreationAuthenticated()).toBe(true) + delete process.env.ORCA_GITEA_TOKEN + expect(isGiteaReviewCreationAuthenticated()).toBe(false) + }) + + it('posts a pull request create body to the repository REST endpoint', async () => { + const fetchMock = vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + const url = new URL(String(input)) + expect(url.origin).toBe('https://git.example.com') + expect(url.pathname).toBe('/code/api/v1/repos/team/repo/pulls') + expect(init).toBeDefined() + const requestInit = init! + expect(requestInit.method).toBe('POST') + expect((requestInit.headers as Record).Authorization).toBe( + 'token gitea-token' + ) + expect(JSON.parse(String(requestInit.body))).toEqual({ + base: 'main', + head: 'feature/gitea', + title: 'Add Gitea create', + body: 'Body', + draft: true + }) + return Response.json({ + number: 13, + title: 'Add Gitea create', + state: 'open', + draft: true, + html_url: 'https://git.example.com/code/team/repo/pulls/13', + updated_at: '2026-06-01T00:00:00Z', + mergeable: true, + head: { + ref: 'feature/gitea', + sha: 'abc123' + } + }) + }) + globalThis.fetch = fetchMock as never + + await expect( + createGiteaPullRequest('/repo', { + provider: 'gitea', + base: 'origin/main', + head: 'refs/heads/feature/gitea', + title: 'Add Gitea create', + body: 'Body', + draft: true + }) + ).resolves.toEqual({ + ok: true, + number: 13, + url: 'https://git.example.com/code/team/repo/pulls/13' + }) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('resolves Gitea remotes through the SSH git provider', async () => { + const remoteGit = { + exec: vi.fn(async () => ({ + stdout: 'git@git.example.com:code/team/repo.git\n', + stderr: '' + })) + } + getSshGitProviderMock.mockReturnValue(remoteGit) + globalThis.fetch = vi.fn(async () => + Response.json({ + number: 14, + title: 'Remote Gitea create', + state: 'open', + html_url: 'https://git.example.com/code/team/repo/pulls/14', + updated_at: '2026-06-01T00:00:00Z', + mergeable: true + }) + ) as never + + await expect( + createGiteaPullRequest( + '/remote/repo', + { + provider: 'gitea', + base: 'main', + head: 'feature/gitea', + title: 'Remote Gitea create' + }, + 'ssh-1' + ) + ).resolves.toMatchObject({ + ok: true, + number: 14 + }) + expect(remoteGit.exec).toHaveBeenCalledWith(['remote', 'get-url', 'origin'], '/remote/repo') + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + + it('classifies validation failures from the REST API', async () => { + globalThis.fetch = vi.fn(async () => + Response.json({ message: 'Validation failed' }, { status: 422 }) + ) as never + + await expect( + createGiteaPullRequest('/repo', { + provider: 'gitea', + base: 'main', + head: 'feature/gitea', + title: 'Add Gitea create' + }) + ).resolves.toMatchObject({ + ok: false, + code: 'validation' + }) + }) +}) diff --git a/src/main/gitea/pull-request-creation.ts b/src/main/gitea/pull-request-creation.ts new file mode 100644 index 00000000000..b113b1783ce --- /dev/null +++ b/src/main/gitea/pull-request-creation.ts @@ -0,0 +1,208 @@ +import type { CreateHostedReviewInput, CreateHostedReviewResult } from '../../shared/hosted-review' +import { + normalizeHostedReviewBaseRef, + normalizeHostedReviewHeadRef +} from '../../shared/hosted-review-refs' +import { + HostedReviewApiRequestError, + requestHostedReviewJson +} from '../source-control/hosted-review-api-request' +import { readHostedPullRequestTemplate } from '../source-control/pull-request-template' +import { getGiteaPullRequestForBranch } from './client' +import { mapGiteaPullRequest, type RawGiteaPullRequest } from './pull-request-mappers' +import { getGiteaRepoRef, type GiteaRepoRef } from './repository-ref' + +const CREATE_REQUEST_TIMEOUT_MS = 60_000 + +function envValue(name: string): string | null { + const value = process.env[name]?.trim() ?? '' + return value.length > 0 ? value : null +} + +function normalizeApiBaseUrl(value: string): string { + const trimmed = value.trim().replace(/\/+$/, '') + return /\/api\/v1$/i.test(trimmed) ? trimmed : `${trimmed}/api/v1` +} + +function configuredApiBaseUrl(repo: GiteaRepoRef): string { + const configured = envValue('ORCA_GITEA_API_BASE_URL') + return configured ? normalizeApiBaseUrl(configured) : repo.apiBaseUrl +} + +export function isGiteaReviewCreationAuthenticated(): boolean { + return envValue('ORCA_GITEA_TOKEN') !== null +} + +function authHeaders(): Record { + const token = envValue('ORCA_GITEA_TOKEN') + return token ? { Authorization: `token ${token}` } : {} +} + +function apiUrl(repo: GiteaRepoRef, path: string): URL { + return new URL(`${configuredApiBaseUrl(repo).replace(/\/+$/, '')}${path}`) +} + +function encodedRepoPath(repo: GiteaRepoRef): string { + return `${encodeURIComponent(repo.owner)}/${encodeURIComponent(repo.repo)}` +} + +function apiErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +function classifyCreateError(error: unknown): CreateHostedReviewResult { + const message = apiErrorMessage(error) + if (message) { + console.warn('createGiteaPullRequest failed:', message) + } + const lower = message.toLowerCase() + const status = error instanceof HostedReviewApiRequestError ? error.status : null + if ( + status === 401 || + status === 403 || + lower.includes('unauthorized') || + lower.includes('forbidden') || + lower.includes('authentication') + ) { + return { + ok: false, + code: 'auth_required', + error: + 'Create PR failed: Gitea is not authenticated. Next step: set ORCA_GITEA_TOKEN in this environment.' + } + } + if (status === 409 || lower.includes('already exists') || lower.includes('already open')) { + return { + ok: false, + code: 'already_exists', + error: 'A pull request already exists for this branch.' + } + } + if (error instanceof HostedReviewApiRequestError && error.timedOut) { + return { + ok: false, + code: 'unknown_completion', + error: 'PR creation may have completed. Refreshing branch review state...' + } + } + if (status === 400 || status === 422 || lower.includes('validation')) { + return { + ok: false, + code: 'validation', + error: + 'Create PR failed: Gitea rejected the pull request. Check the base branch and branch state, then try again.' + } + } + return { + ok: false, + code: 'unknown', + error: 'Create PR failed: Gitea could not create the pull request. Try again in a moment.' + } +} + +async function findExistingPullRequest( + repoPath: string, + head: string, + connectionId?: string | null +): Promise<{ number: number; url: string } | null> { + const existing = await getGiteaPullRequestForBranch(repoPath, head, null, connectionId) + return existing ? { number: existing.number, url: existing.url } : null +} + +export async function createGiteaPullRequest( + repoPath: string, + input: CreateHostedReviewInput, + connectionId?: string | null +): Promise { + if (input.provider !== 'gitea') { + return { + ok: false, + code: 'unsupported_provider', + error: 'Creating reviews for this provider is not supported yet.' + } + } + + const repo = await getGiteaRepoRef(repoPath, connectionId) + if (!repo) { + return { + ok: false, + code: 'unsupported_provider', + error: 'Creating pull requests requires a Gitea remote.' + } + } + + const base = normalizeHostedReviewBaseRef(input.base) + const head = input.head ? normalizeHostedReviewHeadRef(input.head) : '' + const title = input.title.trim() + if (!base || !head || !title) { + return { + ok: false, + code: 'validation', + error: 'Create PR failed: base branch, head branch, and title are required.' + } + } + if (head.toLowerCase() === base.toLowerCase()) { + return { + ok: false, + code: 'validation', + error: 'Create PR failed: choose a different base branch before creating a pull request.' + } + } + + const body = + input.useTemplate && !input.body?.trim() + ? await readHostedPullRequestTemplate(repoPath, connectionId) + : (input.body ?? '') + const requestBody = { + base, + head, + title, + body, + ...(input.draft ? { draft: true } : {}) + } + + try { + const raw = await requestHostedReviewJson( + apiUrl(repo, `/repos/${encodedRepoPath(repo)}/pulls`), + { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + ...authHeaders() + }, + body: JSON.stringify(requestBody) + }, + CREATE_REQUEST_TIMEOUT_MS + ) + const created = mapGiteaPullRequest(raw, 'neutral') + if (created) { + return { ok: true, number: created.number, url: created.url } + } + const found = await findExistingPullRequest(repoPath, head, connectionId).catch(() => null) + return found + ? { ok: true, ...found } + : { + ok: false, + code: 'unknown_completion', + error: 'PR creation may have completed. Refreshing branch review state...' + } + } catch (error) { + const classified = classifyCreateError(error) + if ( + !classified.ok && + (classified.code === 'already_exists' || classified.code === 'unknown_completion') + ) { + const existing = await findExistingPullRequest(repoPath, head, connectionId).catch(() => null) + if (existing) { + return { + ok: false, + code: 'already_exists', + error: 'A pull request already exists for this branch.', + existingReview: existing + } + } + } + return classified + } +} diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index a04bf0bc5f2..65511ede36b 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -589,6 +589,47 @@ describe('getPRForBranch', () => { }) }) + it('ignores merged PRs discovered only by branch lookup', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 5511, + title: 'Merged branch PR', + state: 'closed', + merged_at: '2026-06-16T17:15:33Z', + html_url: 'https://github.com/stablyai/orca/pull/5511', + updated_at: '2026-06-16T17:15:33Z', + draft: false, + mergeable_state: 'clean', + head: { ref: 'add-guide-for-mobile-emulator-use', sha: 'head-oid' }, + base: { ref: 'main', sha: 'base-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 5511, + title: 'Merged branch PR', + state: 'MERGED', + url: 'https://github.com/stablyai/orca/pull/5511', + statusCheckRollup: [], + updatedAt: '2026-06-16T17:15:33Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'add-guide-for-mobile-emulator-use', + baseRefOid: 'base-oid', + headRefOid: 'head-oid' + }) + }) + + const pr = await getPRForBranch('/repo-root', 'add-guide-for-mobile-emulator-use') + + expect(pr).toBeNull() + }) + it('prefers branch lookup over a fallback PR number', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock @@ -679,6 +720,165 @@ describe('getPRForBranch', () => { expect(pr).toMatchObject({ number: 42, title: 'Fallback PR lookup' }) }) + it('treats a merged branch lookup as a miss before using a fallback PR number', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 5511, + title: 'Merged branch PR', + state: 'closed', + merged_at: '2026-06-16T17:15:33Z', + html_url: 'https://github.com/stablyai/orca/pull/5511', + updated_at: '2026-06-16T17:15:33Z', + draft: false, + mergeable_state: 'clean', + head: { ref: 'feature/test', sha: 'merged-head-oid' }, + base: { ref: 'main', sha: 'base-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 5511, + title: 'Merged branch PR', + state: 'MERGED', + url: 'https://github.com/stablyai/orca/pull/5511', + statusCheckRollup: [], + updatedAt: '2026-06-16T17:15:33Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'feature/test', + baseRefOid: 'base-oid', + headRefOid: 'merged-head-oid' + }) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: 'Open fallback PR', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/42', + statusCheckRollup: [], + updatedAt: '2026-06-17T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'contributor/original', + baseRefOid: 'base-oid', + headRefOid: 'fallback-head-oid' + }) + }) + + const pr = await getPRForBranch('/repo-root', 'feature/test', null, null, 42) + + expect(pr).toMatchObject({ number: 42, title: 'Open fallback PR' }) + }) + + it('does not carry a merged upstream branch head repo into a fallback PR number', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [{ owner: 'stablyai', repo: 'orca' }], + headRepo: { owner: 'origin-owner', repo: 'orca' } + }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'fork-owner', repo: 'orca' }) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'fork/contributor/original\n', + stderr: '' + }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 5511, + title: 'Merged upstream branch PR', + state: 'closed', + merged_at: '2026-06-16T17:15:33Z', + html_url: 'https://github.com/stablyai/orca/pull/5511', + updated_at: '2026-06-16T17:15:33Z', + draft: false, + mergeable_state: 'clean', + head: { ref: 'contributor/original', sha: 'merged-head-oid' }, + base: { ref: 'main', sha: 'base-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 5511, + title: 'Merged upstream branch PR', + state: 'MERGED', + url: 'https://github.com/stablyai/orca/pull/5511', + statusCheckRollup: [], + updatedAt: '2026-06-16T17:15:33Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'contributor/original', + baseRefOid: 'base-oid', + headRefOid: 'merged-head-oid' + }) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: 'Open fallback PR', + state: 'OPEN', + url: 'https://github.com/stablyai/orca/pull/42', + statusCheckRollup: [], + updatedAt: '2026-06-17T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'fresh/fallback', + baseRefOid: 'base-oid', + headRefOid: 'fallback-head-oid' + }) + }) + + const pr = await getPRForBranch('/repo-root', 'local-created-from-pr', null, null, 42) + + expect(pr).toMatchObject({ + number: 42, + title: 'Open fallback PR', + headRepo: { owner: 'origin-owner', repo: 'orca' } + }) + }) + + it('ignores merged PRs discovered only from a fallback PR number', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 5511, + title: 'Merged fallback PR', + state: 'MERGED', + url: 'https://github.com/stablyai/orca/pull/5511', + statusCheckRollup: [], + updatedAt: '2026-06-16T17:15:33Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'add-guide-for-mobile-emulator-use', + baseRefOid: 'base-oid', + headRefOid: 'head-oid' + }) + }) + + const pr = await getPRForBranch( + '/repo-root', + 'add-guide-for-mobile-emulator-use', + null, + null, + 5511 + ) + + expect(pr).toBeNull() + }) + it('falls back to the tracked upstream branch when the local branch name differs', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 8dd9ad2f336..fddbde6e10c 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -1928,6 +1928,13 @@ function mapRestPullRequest(pr: RestPullRequest): PullRequestLookupData { } } +function isMergedImplicitPR(data: PullRequestLookupData, linkedPRNumber?: number | null): boolean { + // Why: merged PRs are historical branch matches unless the worktree has an + // explicit PR link. Showing them as implicit review context leaves nothing + // meaningful to unlink after the branch has been rebased or merged. + return typeof linkedPRNumber !== 'number' && mapPRState(data.state, data.isDraft) === 'merged' +} + function normalizePullRequestLookupData(data: PullRequestLookupData): PullRequestLookupData { return { ...data, @@ -2423,6 +2430,11 @@ export async function getPRForBranchOutcome( } } } + if (data && isMergedImplicitPR(data, linkedPRNumber)) { + data = null + dataRepo = null + dataHeadRepo = headRepo + } if (!data && typeof linkedPRNumber !== 'number' && typeof fallbackPRNumber === 'number') { const fallbackLookup = await lookupPRByNumber({ candidates, @@ -2435,6 +2447,9 @@ export async function getPRForBranchOutcome( if (!data) { return { kind: 'no-pr', fetchedAt: Date.now() } } + if (isMergedImplicitPR(data, linkedPRNumber)) { + return { kind: 'no-pr', fetchedAt: Date.now() } + } const mergeable = derivePullRequestMergeable(data) const conflictSummary = @@ -2468,6 +2483,7 @@ export async function getPRForBranchOutcome( : {}), ...(data.mergeStateStatus !== undefined ? { mergeStateStatus: data.mergeStateStatus } : {}), headSha: data.headRefOid, + ...(data.baseRefName ? { baseRefName: data.baseRefName } : {}), prRepo: dataRepo ?? undefined, headRepo: dataHeadRepo ?? undefined, conflictSummary diff --git a/src/main/github/pr-start-point.test.ts b/src/main/github/pr-start-point.test.ts index 24c12931079..91d8e5ad07e 100644 --- a/src/main/github/pr-start-point.test.ts +++ b/src/main/github/pr-start-point.test.ts @@ -26,8 +26,10 @@ describe('resolveGitHubPrStartPoint', () => { remoteUrl: 'git@github.com:contributor/orca.git' } }) - const fetchRemoteTrackingRef = vi.fn(async () => { - throw new Error('fatal: could not find remote ref') + const fetchRemoteTrackingRef = vi.fn(async (_remote: string, branch: string) => { + if (branch === 'feat/onboarding-model-choice-782') { + throw new Error('fatal: could not find remote ref') + } }) const gitExec = vi.fn(async (args: string[]) => { if (args[0] === 'rev-parse') { @@ -40,6 +42,7 @@ describe('resolveGitHubPrStartPoint', () => { repoPath: '/repo-root', prNumber: 1849, headRefName: 'feat/onboarding-model-choice-782', + baseRefName: 'main', gitExec, fetchRemoteTrackingRef, resolveRemote: async () => 'origin' @@ -49,9 +52,11 @@ describe('resolveGitHubPrStartPoint', () => { 'origin', 'feat/onboarding-model-choice-782' ) + expect(fetchRemoteTrackingRef).toHaveBeenCalledWith('origin', 'main') expect(gitExec).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/1849/head']) expect(result).toEqual({ baseBranch: 'def456', + compareBaseRef: 'refs/remotes/origin/main', headSha: 'def456', branchNameOverride: 'feat/onboarding-model-choice-782', pushTarget: { @@ -124,6 +129,7 @@ describe('resolveGitHubPrStartPoint', () => { getWorkItemMock.mockResolvedValue({ type: 'pr', branchName: 'contributor/fix', + baseRefName: 'main', isCrossRepository: true }) getPullRequestPushTargetMock.mockResolvedValue({ @@ -152,6 +158,7 @@ describe('resolveGitHubPrStartPoint', () => { expect(getWorkItemMock).toHaveBeenCalledWith('/repo-root', 1738, 'pr', null) expect(result).toEqual({ baseBranch: 'abc123', + compareBaseRef: 'refs/remotes/origin/main', headSha: 'abc123', branchNameOverride: 'contributor/fix', pushTarget: { @@ -215,15 +222,18 @@ describe('resolveGitHubPrStartPoint', () => { repoPath: '/repo-root', prNumber: 42, headRefName: 'feature/add-feature', + baseRefName: 'develop', gitExec, fetchRemoteTrackingRef, resolveRemote: async () => 'origin' }) expect(fetchRemoteTrackingRef).toHaveBeenCalledWith('origin', 'feature/add-feature') + expect(fetchRemoteTrackingRef).toHaveBeenCalledWith('origin', 'develop') expect(gitExec).toHaveBeenCalledWith(['rev-parse', '--verify', 'origin/feature/add-feature']) expect(result).toEqual({ baseBranch: 'abc123', + compareBaseRef: 'refs/remotes/origin/develop', headSha: 'abc123', branchNameOverride: 'feature/add-feature', pushTarget: { remoteName: 'origin', branchName: 'feature/add-feature' } diff --git a/src/main/github/pr-start-point.ts b/src/main/github/pr-start-point.ts index 7740b4cbd1b..2a02c4f53eb 100644 --- a/src/main/github/pr-start-point.ts +++ b/src/main/github/pr-start-point.ts @@ -8,6 +8,7 @@ type ResolveGitHubPrStartPointArgs = { repoPath: string prNumber: number headRefName?: string + baseRefName?: string isCrossRepository?: boolean connectionId?: string | null gitExec: GitExec @@ -21,6 +22,7 @@ export async function resolveGitHubPrStartPoint( args: ResolveGitHubPrStartPointArgs ): Promise { let headRefName = args.headRefName?.trim() ?? '' + let baseRefName = args.baseRefName?.trim() ?? '' let isCrossRepository = args.isCrossRepository === true let pushTarget: GitPushTarget | undefined let maintainerCanModify: boolean | undefined @@ -50,6 +52,7 @@ export async function resolveGitHubPrStartPoint( return { error: `PR #${args.prNumber} not found.` } } headRefName = (item.branchName ?? '').trim() + baseRefName = (item.baseRefName ?? '').trim() if (!headRefName) { return { error: `PR #${args.prNumber} has no head branch.` } } @@ -69,6 +72,21 @@ export async function resolveGitHubPrStartPoint( return { error: error instanceof Error ? error.message : 'Could not resolve git remote.' } } + const compareBaseRef = baseRefName ? `refs/remotes/${remote}/${baseRefName}` : undefined + + const fetchCompareBaseRef = async (): Promise<{ error: string } | null> => { + if (!baseRefName) { + return null + } + try { + await args.fetchRemoteTrackingRef(remote, baseRefName) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + return { error: `Failed to fetch ${remote}/${baseRefName}: ${message.split('\n')[0]}` } + } + return null + } + const fetchPullRequestHeadSha = async (): Promise<{ baseBranch: string } | { error: string }> => { const pullRef = `refs/pull/${args.prNumber}/head` try { @@ -100,11 +118,16 @@ export async function resolveGitHubPrStartPoint( if ('error' in result) { return result } + const compareBaseFetchError = await fetchCompareBaseRef() + if (compareBaseFetchError) { + return compareBaseFetchError + } // Why: adopt the contributor's branch name locally (mirroring the same-repo // return below) so fork-PR worktrees aren't renamed with the maintainer's // branch prefix (e.g. `me/866`). The push refspec still targets the fork. return { ...result, + ...(compareBaseRef ? { compareBaseRef } : {}), headSha: result.baseBranch, branchNameOverride: headRefName, ...(pushTarget ? { pushTarget } : {}), @@ -122,8 +145,13 @@ export async function resolveGitHubPrStartPoint( const result = await fetchPullRequestHeadSha() if (!('error' in result)) { await resolvePushTarget() + const compareBaseFetchError = await fetchCompareBaseRef() + if (compareBaseFetchError) { + return compareBaseFetchError + } return { ...result, + ...(compareBaseRef ? { compareBaseRef } : {}), headSha: result.baseBranch, branchNameOverride: headRefName, ...(pushTarget ? { pushTarget } : {}), @@ -147,9 +175,14 @@ export async function resolveGitHubPrStartPoint( if (!headSha) { return { error: `Empty SHA resolving PR #${args.prNumber} head.` } } + const compareBaseFetchError = await fetchCompareBaseRef() + if (compareBaseFetchError) { + return compareBaseFetchError + } return { baseBranch: headSha, + ...(compareBaseRef ? { compareBaseRef } : {}), headSha, branchNameOverride: headRefName, pushTarget: { remoteName: remote, branchName: headRefName } diff --git a/src/main/gitlab/mappers.ts b/src/main/gitlab/mappers.ts index 61ba93d27ac..55dffb89c64 100644 --- a/src/main/gitlab/mappers.ts +++ b/src/main/gitlab/mappers.ts @@ -97,6 +97,7 @@ type GitLabMRRaw = { has_conflicts?: boolean detailed_merge_status?: string description?: string | null + target_branch?: string author?: { username?: string | null; avatar_url?: string | null } | null } @@ -110,6 +111,7 @@ export function mapMRInfo(data: GitLabMRRaw, pipelineStatus: CheckStatus): MRInf updatedAt: data.updated_at ?? data.updatedAt ?? '', mergeable: deriveMergeable(data), headSha: data.sha, + baseRefName: data.target_branch, // Why: detail-endpoint payloads include `description`; list endpoints // strip it. Pass through what's present rather than coercing missing // values to '' so downstream UIs can distinguish "no body authored" diff --git a/src/main/index.ts b/src/main/index.ts index 66b7f8150ab..98047fd5456 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -199,7 +199,7 @@ if (appImageCliRedirect.redirected) { // worktree id change via migrateWorktreeIdentity + a rename-aware worktrees:changed // handler, so an old->new id change is no longer mistaken for a deletion. Flip off // to disable the on-disk move (branch + display rename still happen) if needed. -const ENABLE_FIRST_WORK_FOLDER_RENAME = true +const ENABLE_FIRST_WORK_FOLDER_RENAME = false // Why: the store/runtime singletons live here in index.ts; injecting them keeps // the rename orchestrator free of module-level state and unit-testable. diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 65217eedc2b..0d0a75d4023 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -78,6 +78,7 @@ import { } from '../git/huge-folder-ignore' import { assertGitPushTargetShape } from '../../shared/git-push-target-validation' import { getCommitMessageModelDiscoveryHostKey } from '../../shared/commit-message-host-key' +import type { HostedReviewProvider } from '../../shared/hosted-review' import type { ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' import { validateGitPushTarget } from '../git/push-target-validation' import { getRemoteCommitUrl, getRemoteFileUrl } from '../git/repo' @@ -101,6 +102,7 @@ import { getSshGitProvider, SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE } from '../providers/ssh-git-dispatch' +import { resolveHostedReviewBodyForGeneration } from '../source-control/pull-request-template' import { prepareLocalCommitMessageAgentEnv, type CommitMessageAgentEnvironmentResolvers @@ -1130,6 +1132,8 @@ export function registerFilesystemHandlers( title: string body: string draft: boolean + provider?: HostedReviewProvider + useTemplate?: boolean connectionId?: string sourceControlAiResolvedParams?: ResolvedSourceControlAiGenerationParams sourceControlAi?: GlobalSettings['sourceControlAi'] @@ -1166,12 +1170,19 @@ export function registerFilesystemHandlers( } let context: Awaited> try { + const currentBody = await resolveHostedReviewBodyForGeneration({ + body: args.body, + repoPath: args.worktreePath, + connectionId: args.connectionId, + provider: args.provider, + useTemplate: args.useTemplate + }) context = await getPullRequestDraftContext( (argv) => provider.exec(argv, args.worktreePath), { base: args.base, currentTitle: args.title, - currentBody: args.body, + currentBody, currentDraft: args.draft } ) @@ -1197,12 +1208,19 @@ export function registerFilesystemHandlers( const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) let context: Awaited> try { + const currentBody = await resolveHostedReviewBodyForGeneration({ + body: args.body, + repoPath: worktreePath, + connectionId: args.connectionId, + provider: args.provider, + useTemplate: args.useTemplate + }) context = await getPullRequestDraftContext( (argv, options) => gitExecFileAsync(argv, { cwd: worktreePath, ...options }), { base: args.base, currentTitle: args.title, - currentBody: args.body, + currentBody, currentDraft: args.draft } ) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 24204477e4b..9320722f4e9 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -1572,9 +1572,9 @@ export async function createRemoteWorktree( const worktreeId = `${repo.id}::${created.path}` const now = Date.now() - // Why: persisted compare refs must survive local branches whose names look - // like remote labels, e.g. a local branch literally named "origin/main". - const metadataBaseRef = remoteTrackingBase?.ref ?? baseBranch + // Why: PR/MR-created worktrees can start from a head ref/SHA while Source + // Control must compare against the review target branch. + const metadataBaseRef = args.compareBaseRef ?? remoteTrackingBase?.ref ?? baseBranch let configuredPushTarget: GitPushTarget | undefined if (preparedPushTarget) { configuredPushTarget = await configureCreatedWorktreePushTargetSsh( @@ -2120,9 +2120,9 @@ export async function createLocalWorktree( const worktreeId = `${repo.id}::${created.path}` const now = Date.now() - // Why: persisted compare refs must survive local branches whose names look - // like remote labels, e.g. a local branch literally named "origin/main". - const metadataBaseRef = remoteTrackingBase?.ref ?? baseBranch + // Why: PR/MR-created worktrees can start from a head ref/SHA while Source + // Control must compare against the review target branch. + const metadataBaseRef = args.compareBaseRef ?? remoteTrackingBase?.ref ?? baseBranch const metaUpdates: Partial = { // Why: path-derived worktree IDs can be reused after external deletion. // Fresh creations must rotate instance identity so stale lineage cannot diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 5e74dc8931f..688d42a70a2 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -1060,6 +1060,7 @@ describe('registerWorktreeHandlers', () => { repoId: 'repo-1', name: 'fix-title', baseBranch: 'abc123', + compareBaseRef: 'refs/remotes/origin/main', branchNameOverride: 'feature/fix', linkedPR: 42, pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } @@ -1076,9 +1077,54 @@ describe('registerWorktreeHandlers', () => { ['branch', '--set-upstream-to', 'origin/feature/fix', 'feature/fix'], { cwd: '/workspace/fix-title' } ) + expect(store.setWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/fix-title', + expect.objectContaining({ + baseRef: 'refs/remotes/origin/main', + linkedPR: 42 + }) + ) expect(getPRForBranchMock).toHaveBeenCalledWith('/workspace/repo', 'feature/fix') }) + it('persists an explicit compare base ahead of the checkout remote-tracking base', async () => { + runtimeStub.resolveRemoteTrackingBase.mockResolvedValueOnce({ + base: 'origin/source-branch', + remote: 'origin', + branch: 'source-branch', + ref: 'refs/remotes/origin/source-branch' + }) + runtimeStub.hasRemoteTrackingRef.mockResolvedValueOnce(true) + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/fix-title', + head: 'abc123', + branch: 'refs/heads/feature/fix', + isBare: false, + isMainWorktree: false + } + ]) + store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta) + + await handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'fix-title', + baseBranch: 'origin/source-branch', + compareBaseRef: 'refs/remotes/origin/main', + branchNameOverride: 'feature/fix', + linkedGitLabMR: 7, + pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } + }) + + expect(store.setWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/fix-title', + expect.objectContaining({ + baseRef: 'refs/remotes/origin/main', + linkedGitLabMR: 7 + }) + ) + }) + it('allows a selected Bitbucket PR branch override to match its remote push target', async () => { getBranchConflictKindMock.mockImplementation(async (_repoPath: string, branch: string) => branch === 'feature/bitbucket' ? 'remote' : null diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index a8356f394f6..4b70ae73e24 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -1054,6 +1054,7 @@ export function registerWorktreeHandlers( repoId: string prNumber: number headRefName?: string + baseRefName?: string isCrossRepository?: boolean } ): Promise => { @@ -1090,6 +1091,7 @@ export function registerWorktreeHandlers( repoPath: repo.path, prNumber: args.prNumber, headRefName: args.headRefName, + baseRefName: args.baseRefName, isCrossRepository: args.isCrossRepository, connectionId: repo.connectionId ?? null, gitExec, @@ -1120,13 +1122,18 @@ export function registerWorktreeHandlers( repoId: string mrIid: number sourceBranch?: string + targetBranch?: string isCrossRepository?: boolean } - ): Promise<{ baseBranch: string; pushTarget?: GitPushTarget } | { error: string }> => { + ): Promise< + | { baseBranch: string; compareBaseRef?: string; pushTarget?: GitPushTarget } + | { error: string } + > => { return runtime.resolveManagedMrBase({ repoSelector: `id:${args.repoId}`, mrIid: args.mrIid, sourceBranch: args.sourceBranch, + targetBranch: args.targetBranch, isCrossRepository: args.isCrossRepository }) } diff --git a/src/main/providers/agent-foreground-context-paths.ts b/src/main/providers/agent-foreground-context-paths.ts new file mode 100644 index 00000000000..7ff8a6d3e62 --- /dev/null +++ b/src/main/providers/agent-foreground-context-paths.ts @@ -0,0 +1,11 @@ +import { splitWorktreeIdForFilesystem } from '../../shared/worktree-id' + +export function getAgentForegroundContextPaths(options: { + cwd?: string + worktreeId?: string | null +}): string[] { + const worktreePath = options.worktreeId + ? splitWorktreeIdForFilesystem(options.worktreeId)?.worktreePath + : undefined + return [...new Set([options.cwd, worktreePath].filter((path): path is string => Boolean(path)))] +} diff --git a/src/main/providers/agent-foreground-process.test.ts b/src/main/providers/agent-foreground-process.test.ts index 63b9b884799..5d551b3a682 100644 --- a/src/main/providers/agent-foreground-process.test.ts +++ b/src/main/providers/agent-foreground-process.test.ts @@ -19,14 +19,46 @@ function mockPs(stdout: string): void { }) } -function windowsProcessRows(): string { +function windowsProcessJsonRows( + rows: { + CommandLine: string | null + Name: string + ParentProcessId: number + ProcessId: number + ExecutablePath?: string | null + }[] = [ + { + CommandLine: 'powershell.exe', + Name: 'powershell.exe', + ParentProcessId: 99, + ProcessId: 100 + }, + { + CommandLine: 'node C:\\Users\\dev\\AppData\\Roaming\\npm\\codex.cmd', + Name: 'node.exe', + ParentProcessId: 100, + ProcessId: 101 + } + ] +): string { + return JSON.stringify( + rows.map((row) => ({ + ExecutablePath: row.ExecutablePath ?? null, + ...row + })) + ) +} + +function windowsProcessValueRows(): string { return [ 'CommandLine=powershell.exe', + 'ExecutablePath=C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', 'Name=powershell.exe', 'ParentProcessId=99', 'ProcessId=100', '', 'CommandLine=node C:\\Users\\dev\\AppData\\Roaming\\npm\\codex.cmd', + 'ExecutablePath=C:\\Program Files\\nodejs\\node.exe', 'Name=node.exe', 'ParentProcessId=100', 'ProcessId=101', @@ -95,7 +127,7 @@ describe('resolveAgentForegroundProcess', () => { execFileMock.mockImplementation( (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void - callback(null, { stdout: windowsProcessRows(), stderr: '' }) + callback(null, { stdout: windowsProcessJsonRows(), stderr: '' }) } ) @@ -108,6 +140,79 @@ describe('resolveAgentForegroundProcess', () => { ) }) + it('recognizes Windows shell-rooted agent launches from descendant command lines', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { stdout: windowsProcessJsonRows(), stderr: '' }) + } + ) + + await expect(resolveAgentForegroundProcess(100, 'powershell.exe')).resolves.toBe('codex') + }) + + it('recognizes Windows Git Bash shell-rooted agent launches', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { + stdout: windowsProcessJsonRows([ + { + CommandLine: 'C:\\Program Files\\Git\\bin\\bash.exe --login -i', + Name: 'bash.exe', + ParentProcessId: 99, + ProcessId: 100 + }, + { + CommandLine: 'node C:\\Users\\dev\\AppData\\Roaming\\npm\\codex.cmd', + Name: 'node.exe', + ParentProcessId: 100, + ProcessId: 101 + } + ]), + stderr: '' + }) + } + ) + + await expect(resolveAgentForegroundProcess(100, 'bash.exe')).resolves.toBe('codex') + }) + + it('keeps multiline Windows command lines inside the parsed process row', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { + stdout: windowsProcessJsonRows([ + { + CommandLine: 'powershell.exe', + Name: 'powershell.exe', + ParentProcessId: 99, + ProcessId: 100 + }, + { + CommandLine: [ + 'node', + 'C:\\Users\\dev\\AppData\\Roaming\\npm\\node_modules\\@openai\\codex\\bin\\codex.js', + '--prompt', + '"line one\r\nName=gemini.exe\r\nProcessId=999"' + ].join(' '), + Name: 'node.exe', + ParentProcessId: 100, + ProcessId: 101 + } + ]), + stderr: '' + }) + } + ) + + await expect(resolveAgentForegroundProcess(100, 'powershell.exe')).resolves.toBe('codex') + }) + it('falls back to WMIC when Windows PowerShell process enumeration fails', async () => { Object.defineProperty(process, 'platform', { value: 'win32' }) execFileMock.mockImplementation((cmd: string, _args: string[], _opts: unknown, cb: unknown) => { @@ -116,7 +221,27 @@ describe('resolveAgentForegroundProcess', () => { callback(new Error('powershell unavailable'), { stdout: '', stderr: '' }) return } - callback(null, { stdout: windowsProcessRows(), stderr: '' }) + callback(null, { stdout: windowsProcessValueRows(), stderr: '' }) + }) + + await expect(resolveAgentForegroundProcess(100, 'node.exe')).resolves.toBe('codex') + expect(execFileMock).toHaveBeenCalledWith( + 'wmic', + expect.any(Array), + expect.objectContaining({ timeout: 3000 }), + expect.any(Function) + ) + }) + + it('falls back to WMIC when Windows PowerShell returns no process rows', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation((cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + if (cmd === 'powershell.exe') { + callback(null, { stdout: ' \r\n', stderr: '' }) + return + } + callback(null, { stdout: windowsProcessValueRows(), stderr: '' }) }) await expect(resolveAgentForegroundProcess(100, 'node.exe')).resolves.toBe('codex') @@ -159,6 +284,162 @@ describe('resolveAgentForegroundProcess', () => { await expect(resolveAgentForegroundProcess(100, 'node.exe')).resolves.toBe('node.exe') }) + it('fails closed for ambiguous Windows shell-rooted agent descendants', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { + stdout: [ + 'CommandLine=powershell.exe', + 'Name=powershell.exe', + 'ParentProcessId=99', + 'ProcessId=100', + '', + 'CommandLine=node C:\\Users\\dev\\AppData\\Roaming\\npm\\node_modules\\@openai\\codex\\bin\\codex.js', + 'Name=node.exe', + 'ParentProcessId=100', + 'ProcessId=101', + '', + 'CommandLine=node C:\\Users\\dev\\AppData\\Roaming\\npm\\node_modules\\@google\\gemini-cli\\bundle\\gemini.mjs', + 'Name=node.exe', + 'ParentProcessId=100', + 'ProcessId=102', + '' + ].join('\r\n'), + stderr: '' + }) + } + ) + + await expect(resolveAgentForegroundProcess(100, 'powershell.exe')).resolves.toBe( + 'powershell.exe' + ) + }) + + it('recognizes a Windows shell-rooted agent when only one candidate matches the worktree path', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { + stdout: [ + 'CommandLine=powershell.exe', + 'CreationDate=20260616110000.000000-000', + 'ExecutablePath=C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', + 'Name=powershell.exe', + 'ParentProcessId=99', + 'ProcessId=100', + '', + 'CommandLine=node C:\\Users\\dev\\AppData\\Roaming\\npm\\node_modules\\@openai\\codex\\bin\\codex.js --cwd C:\\repo\\orca', + 'CreationDate=20260616110100.000000-000', + 'ExecutablePath=C:\\Program Files\\nodejs\\node.exe', + 'Name=node.exe', + 'ParentProcessId=100', + 'ProcessId=101', + '', + 'CommandLine=node C:\\Users\\dev\\AppData\\Roaming\\npm\\node_modules\\@google\\gemini-cli\\bundle\\gemini.mjs --cwd C:\\repo\\other', + 'CreationDate=20260616110200.000000-000', + 'ExecutablePath=C:\\Program Files\\nodejs\\node.exe', + 'Name=node.exe', + 'ParentProcessId=100', + 'ProcessId=102', + '' + ].join('\r\n'), + stderr: '' + }) + } + ) + + await expect( + resolveAgentForegroundProcess(100, 'powershell.exe', { + contextPaths: ['C:\\repo\\orca'] + }) + ).resolves.toBe('codex') + }) + + it('recognizes the deepest Windows shell-rooted agent when candidates share one lineage', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { + stdout: [ + 'CommandLine=powershell.exe', + 'CreationDate=20260616110000.000000-000', + 'ExecutablePath=C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', + 'Name=powershell.exe', + 'ParentProcessId=99', + 'ProcessId=100', + '', + 'CommandLine=codex --cwd C:\\repo\\orca', + 'CreationDate=20260616110100.000000-000', + 'ExecutablePath=C:\\Users\\dev\\AppData\\Roaming\\npm\\codex.cmd', + 'Name=codex.exe', + 'ParentProcessId=100', + 'ProcessId=101', + '', + 'CommandLine=gemini --cwd C:\\repo\\orca', + 'CreationDate=20260616110200.000000-000', + 'ExecutablePath=C:\\Users\\dev\\AppData\\Roaming\\npm\\gemini.cmd', + 'Name=gemini.exe', + 'ParentProcessId=101', + 'ProcessId=102', + '' + ].join('\r\n'), + stderr: '' + }) + } + ) + + await expect( + resolveAgentForegroundProcess(100, 'powershell.exe', { + contextPaths: ['C:\\repo\\orca'] + }) + ).resolves.toBe('gemini') + }) + + it('fails closed for sibling Windows agents that both match the same worktree path', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { + stdout: [ + 'CommandLine=powershell.exe', + 'CreationDate=20260616110000.000000-000', + 'ExecutablePath=C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', + 'Name=powershell.exe', + 'ParentProcessId=99', + 'ProcessId=100', + '', + 'CommandLine=codex --cwd C:\\repo\\orca', + 'CreationDate=20260616110100.000000-000', + 'ExecutablePath=C:\\Users\\dev\\AppData\\Roaming\\npm\\codex.cmd', + 'Name=codex.exe', + 'ParentProcessId=100', + 'ProcessId=101', + '', + 'CommandLine=gemini --cwd C:\\repo\\orca', + 'CreationDate=20260616110200.000000-000', + 'ExecutablePath=C:\\Users\\dev\\AppData\\Roaming\\npm\\gemini.cmd', + 'Name=gemini.exe', + 'ParentProcessId=100', + 'ProcessId=102', + '' + ].join('\r\n'), + stderr: '' + }) + } + ) + + await expect( + resolveAgentForegroundProcess(100, 'powershell.exe', { + contextPaths: ['C:\\repo\\orca'] + }) + ).resolves.toBe('powershell.exe') + }) + it('fails closed when Windows has multiple matching wrapper descendants', async () => { Object.defineProperty(process, 'platform', { value: 'win32' }) execFileMock.mockImplementation( diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index a748f8e2ad6..1d090cc2336 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -1,10 +1,13 @@ import { execFile } from 'child_process' import { promisify } from 'util' +import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' import { - isAgentForegroundWrapperProcess, - isExpectedAgentProcess, - recognizeAgentProcessFromCommandLine -} from '../../shared/agent-process-recognition' + resolveWindowsAgentForegroundProcess, + shouldInspectWindowsAgentForeground, + type AgentForegroundResolutionOptions +} from './windows-agent-foreground-process' + +export type { AgentForegroundResolutionOptions } from './windows-agent-foreground-process' const execFileAsync = promisify(execFile) @@ -15,13 +18,6 @@ type ProcessRow = { command: string } -type WindowsProcessRow = { - pid: number - ppid: number - name: string - command: string -} - function parsePsRows(stdout: string): ProcessRow[] { const rows: ProcessRow[] = [] for (const line of stdout.split('\n')) { @@ -39,49 +35,6 @@ function parsePsRows(stdout: string): ProcessRow[] { return rows } -function parseWindowsProcessRows(stdout: string): WindowsProcessRow[] { - const rows: WindowsProcessRow[] = [] - let command = '' - let name = '' - let pid = Number.NaN - let ppid = Number.NaN - - const flush = (): void => { - if (Number.isFinite(pid) && Number.isFinite(ppid)) { - rows.push({ pid, ppid, name, command: command || name }) - } - command = '' - name = '' - pid = Number.NaN - ppid = Number.NaN - } - - for (const raw of stdout.split(/\r?\n/)) { - const line = raw.trim() - if (!line) { - flush() - continue - } - const eq = line.indexOf('=') - if (eq < 0) { - continue - } - const key = line.slice(0, eq) - const value = line.slice(eq + 1) - if (key === 'CommandLine') { - command = value - } else if (key === 'Name') { - name = value - } else if (key === 'ParentProcessId') { - ppid = Number.parseInt(value, 10) - } else if (key === 'ProcessId') { - pid = Number.parseInt(value, 10) - } - } - flush() - return rows -} - function collectDescendants( rows: Row[], rootPid: number @@ -114,18 +67,20 @@ function candidateScore(row: ProcessRow & { depth: number }): number { export async function resolveAgentForegroundProcess( shellPid: number | null | undefined, - fallbackProcess: string | null + fallbackProcess: string | null, + options: AgentForegroundResolutionOptions = {} ): Promise { if (!shellPid) { return fallbackProcess } if (process.platform === 'win32') { - if (!fallbackProcess || !isAgentForegroundWrapperProcess(fallbackProcess)) { + if (!fallbackProcess || !shouldInspectWindowsAgentForeground(fallbackProcess)) { return fallbackProcess } return ( - (await resolveAgentForegroundProcessFromWindows(shellPid, fallbackProcess)) ?? fallbackProcess + (await resolveWindowsAgentForegroundProcess(shellPid, fallbackProcess, options)) ?? + fallbackProcess ) } @@ -143,93 +98,6 @@ export async function resolveAgentForegroundProcess( return fallbackProcess } -async function resolveAgentForegroundProcessFromWindows( - shellPid: number, - fallbackProcess: string -): Promise { - const stdout = - (await queryWindowsProcessesWithPowerShell()) ?? (await queryWindowsProcessesWithWmic()) - return stdout - ? resolveAgentForegroundProcessFromWindowsRows(stdout, shellPid, fallbackProcess) - : null -} - -async function queryWindowsProcessesWithPowerShell(): Promise { - try { - const { stdout } = await execFileAsync( - 'powershell.exe', - [ - '-NoProfile', - '-NonInteractive', - '-Command', - 'Get-CimInstance Win32_Process | ForEach-Object { "CommandLine=$($_.CommandLine)"; "Name=$($_.Name)"; "ParentProcessId=$($_.ParentProcessId)"; "ProcessId=$($_.ProcessId)"; "" }' - ], - { - encoding: 'utf8', - timeout: 3000, - maxBuffer: 8 * 1024 * 1024 - } - ) - return stdout - } catch { - return null - } -} - -async function queryWindowsProcessesWithWmic(): Promise { - try { - const { stdout } = await execFileAsync( - 'wmic', - ['process', 'get', 'CommandLine,Name,ParentProcessId,ProcessId', '/format:value'], - { - encoding: 'utf8', - timeout: 3000, - maxBuffer: 8 * 1024 * 1024 - } - ) - return stdout - } catch { - // Best-effort: Windows process enumeration may be disabled, so callers - // still fall back to node-pty's process name when both probes fail. - return null - } -} - -function resolveAgentForegroundProcessFromWindowsRows( - stdout: string, - shellPid: number, - fallbackProcess: string -): string | null { - const candidates = collectDescendants(parseWindowsProcessRows(stdout), shellPid).sort( - (a, b) => b.depth - a.depth - ) - const wrapperCandidates = candidates.filter((candidate) => - windowsCandidateMatchesFallbackWrapper(candidate, fallbackProcess) - ) - if (wrapperCandidates.length !== 1) { - return null - } - const [candidate] = wrapperCandidates - const recognized = - recognizeAgentProcessFromCommandLine(candidate.command) ?? - recognizeAgentProcessFromCommandLine(candidate.name) - if (recognized) { - return recognized.processName - } - return null -} - -function windowsCandidateMatchesFallbackWrapper( - candidate: WindowsProcessRow, - fallbackProcess: string -): boolean { - const commandToken = candidate.command.trim().split(/\s+/, 1)[0] ?? '' - return ( - isExpectedAgentProcess(candidate.name, fallbackProcess) || - isExpectedAgentProcess(commandToken, fallbackProcess) - ) -} - function resolveAgentForegroundProcessFromPs(stdout: string, shellPid: number): string | null { const rows = parsePsRows(stdout) const shellRow = rows.find((row) => row.pid === shellPid) diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index 5a92c2cefdd..19c8cd81d0b 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -7,14 +7,16 @@ const { accessSyncMock, mkdirSyncMock, writeFileSyncMock, - spawnMock + spawnMock, + resolveAgentForegroundProcessMock } = vi.hoisted(() => ({ existsSyncMock: vi.fn(), statSyncMock: vi.fn(), accessSyncMock: vi.fn(), mkdirSyncMock: vi.fn(), writeFileSyncMock: vi.fn(), - spawnMock: vi.fn() + spawnMock: vi.fn(), + resolveAgentForegroundProcessMock: vi.fn() })) vi.mock('fs', () => ({ @@ -37,6 +39,10 @@ vi.mock('node-pty', () => ({ spawn: spawnMock })) +vi.mock('./agent-foreground-process', () => ({ + resolveAgentForegroundProcess: resolveAgentForegroundProcessMock +})) + vi.mock('../wsl', () => ({ parseWslPath: (path: string) => { const match = path.match(/^\\\\wsl\.localhost\\([^\\]+)(.*)$/) @@ -82,6 +88,10 @@ describe('LocalPtyProvider', () => { accessSyncMock.mockReturnValue(undefined) mkdirSyncMock.mockReset() writeFileSyncMock.mockReset() + resolveAgentForegroundProcessMock.mockReset() + resolveAgentForegroundProcessMock.mockImplementation( + async (_pid: number, fallbackProcess: string | null) => fallbackProcess + ) exitCb = undefined mockProc = { @@ -650,6 +660,24 @@ describe('LocalPtyProvider', () => { expect(await provider.getForegroundProcess(id)).toBe('zsh') }) + it('uses the spawned Windows shell when node-pty reports only the terminal name', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + mockProc.process = 'xterm-256color' + + const { id } = await provider.spawn({ + cols: 80, + rows: 24, + shellOverride: 'powershell.exe' + }) + + expect(await provider.getForegroundProcess(id)).toBe('powershell.exe') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledWith( + mockProc.pid, + 'powershell.exe', + expect.any(Object) + ) + }) + it('returns null for unknown PTY ids', async () => { expect(await provider.getForegroundProcess('nonexistent')).toBeNull() }) diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 2115e508482..7d301cfc5ef 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -40,12 +40,14 @@ import { } from '../git-bash' import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell' import { resolveAgentForegroundProcess } from './agent-foreground-process' +import { getAgentForegroundContextPaths } from './agent-foreground-context-paths' const PANE_IDENTITY_ENV_KEYS = ['ORCA_PANE_KEY', 'ORCA_TAB_ID', 'ORCA_WORKTREE_ID'] as const let ptyCounter = 0 const ptyProcesses = new Map() const ptyShellName = new Map() +const ptyAgentForegroundContextPaths = new Map() // Why: node-pty's onData/onExit register native NAPI ThreadSafeFunction // callbacks. If the PTY is killed without disposing these listeners, the // stale callbacks survive into node::FreeEnvironment() where NAPI attempts @@ -133,6 +135,7 @@ function clearPtyState(id: string): void { disposePtyListeners(id) ptyProcesses.delete(id) ptyShellName.delete(id) + ptyAgentForegroundContextPaths.delete(id) ptyLoadGeneration.delete(id) } @@ -156,6 +159,26 @@ function normalizeLocalCallerSessionId(sessionId: string | undefined): string | return requested } +function normalizeForegroundProcessName(processName: string | null | undefined): string | null { + const trimmed = processName?.trim().replace(/^["']|["']$/g, '') ?? '' + if (!trimmed || trimmed === 'xterm-256color') { + return null + } + return trimmed.split(/[\\/]/).pop() || null +} + +function resolveForegroundFallbackProcess( + processName: string | null | undefined, + shellName: string | undefined +): string | null { + if (process.platform !== 'win32' || normalizeForegroundProcessName(processName)) { + return processName || null + } + // Why: Windows node-pty can expose only the terminal name (`xterm-256color`). + // The spawned shell is the best fallback for agent foreground enrichment. + return shellName ?? processName ?? null +} + function destroyPtyProcess(proc: pty.IPty, options: { alreadyKilled?: boolean } = {}): void { // Why: node-pty's UnixTerminal.destroy() closes the master socket, which // releases the ptmx fd to the OS — without this call the fd leaks until GC @@ -507,6 +530,10 @@ export class LocalPtyProvider implements IPtyProvider { const proc = spawnResult.process ptyProcesses.set(id, proc) ptyShellName.set(id, basename(shellPath)) + ptyAgentForegroundContextPaths.set( + id, + getAgentForegroundContextPaths({ cwd: args.cwd, worktreeId: args.worktreeId }) + ) ptyLoadGeneration.set(id, loadGeneration) this.opts.onSpawned?.(id) @@ -644,6 +671,7 @@ export class LocalPtyProvider implements IPtyProvider { destroyPtyProcess(proc, { alreadyKilled: true }) ptyProcesses.delete(id) ptyShellName.delete(id) + ptyAgentForegroundContextPaths.delete(id) ptyLoadGeneration.delete(id) this.opts.onExit?.(id, -1) for (const cb of exitListeners) { @@ -711,7 +739,13 @@ export class LocalPtyProvider implements IPtyProvider { return null } try { - return await resolveAgentForegroundProcess(proc.pid, proc.process || null) + return await resolveAgentForegroundProcess( + proc.pid, + resolveForegroundFallbackProcess(proc.process || null, ptyShellName.get(id)), + { + contextPaths: ptyAgentForegroundContextPaths.get(id) + } + ) } catch { return null } diff --git a/src/main/providers/windows-agent-foreground-process.ts b/src/main/providers/windows-agent-foreground-process.ts new file mode 100644 index 00000000000..39447432a91 --- /dev/null +++ b/src/main/providers/windows-agent-foreground-process.ts @@ -0,0 +1,217 @@ +import { + isAgentForegroundWrapperProcess, + isExpectedAgentProcess, + recognizeAgentProcessFromCommandLine +} from '../../shared/agent-process-recognition' +import { isShellProcess } from '../../shared/shell-process-detection' +import { + queryWindowsProcessDescendants, + type WindowsProcessCandidate, + type WindowsProcessRow +} from './windows-foreground-process-rows' + +export type AgentForegroundResolutionOptions = { + contextPaths?: readonly string[] +} + +export function shouldInspectWindowsAgentForeground(fallbackProcess: string): boolean { + return isAgentForegroundWrapperProcess(fallbackProcess) || isShellProcess(fallbackProcess) +} + +export async function resolveWindowsAgentForegroundProcess( + shellPid: number, + fallbackProcess: string, + options: AgentForegroundResolutionOptions +): Promise { + const candidates = await queryWindowsProcessDescendants(shellPid) + if (!candidates) { + return null + } + if (isShellProcess(fallbackProcess)) { + return resolveShellForegroundProcessFromWindowsCandidates(candidates, options.contextPaths) + } + const wrapperCandidates = candidates.filter((candidate) => + windowsCandidateMatchesFallbackWrapper(candidate, fallbackProcess) + ) + if (wrapperCandidates.length !== 1) { + return resolveWrapperForegroundProcessFromWindowsCandidates( + wrapperCandidates, + options.contextPaths + ) + } + const [candidate] = wrapperCandidates + const recognized = + recognizeAgentProcessFromCommandLine(candidate.command) ?? + recognizeAgentProcessFromCommandLine(candidate.name) + if (recognized) { + return recognized.processName + } + return null +} + +function resolveShellForegroundProcessFromWindowsCandidates( + candidates: readonly WindowsProcessCandidate[], + contextPaths: readonly string[] | undefined +): string | null { + const recognizedCandidates = createRecognizedWindowsProcessCandidates(candidates, contextPaths) + const contextCandidates = recognizedCandidates.filter((candidate) => candidate.contextMatch) + if (contextCandidates.length > 0) { + return resolveRecognizedWindowsProcessCandidates(contextCandidates, candidates) + } + return resolveRecognizedWindowsProcessCandidates(recognizedCandidates, candidates) +} + +function resolveWrapperForegroundProcessFromWindowsCandidates( + candidates: readonly WindowsProcessCandidate[], + contextPaths: readonly string[] | undefined +): string | null { + const contextCandidates = createRecognizedWindowsProcessCandidates( + candidates, + contextPaths + ).filter((candidate) => candidate.contextMatch) + return contextCandidates.length > 0 + ? resolveRecognizedWindowsProcessCandidates(contextCandidates, candidates) + : null +} + +type RecognizedWindowsProcessCandidate = WindowsProcessRow & { + contextMatch: boolean + depth: number + processName: string +} + +function createRecognizedWindowsProcessCandidates( + candidates: readonly WindowsProcessCandidate[], + contextPaths: readonly string[] | undefined +): RecognizedWindowsProcessCandidate[] { + const normalizedContextPaths = normalizeContextPaths(contextPaths) + return candidates.flatMap((candidate) => { + const recognized = recognizeWindowsProcessCandidate(candidate) + if (!recognized) { + return [] + } + return [ + { + ...candidate, + contextMatch: candidateMatchesContextPath(candidate, normalizedContextPaths), + processName: recognized + } + ] + }) +} + +function resolveRecognizedWindowsProcessCandidates( + recognizedCandidates: readonly RecognizedWindowsProcessCandidate[], + allCandidates: readonly WindowsProcessCandidate[] +): string | null { + if (recognizedCandidates.length === 0) { + return null + } + const recognizedProcessNames = new Set( + recognizedCandidates.map((candidate) => candidate.processName) + ) + if (recognizedProcessNames.size === 1) { + return [...recognizedProcessNames][0] + } + + const candidatesByPid = new Map(allCandidates.map((candidate) => [candidate.pid, candidate])) + const leafCandidates = recognizedCandidates.filter( + (candidate) => + !recognizedCandidates.some( + (other) => + other.pid !== candidate.pid && + windowsCandidateIsAncestor(candidate, other, candidatesByPid) + ) + ) + const leafProcessNames = new Set(leafCandidates.map((candidate) => candidate.processName)) + // Why: Windows lacks a cheap PTY foreground marker like POSIX '+'. A single + // recognized lineage leaf is strong enough; sibling agent leaves are not. + return leafProcessNames.size === 1 ? [...leafProcessNames][0] : null +} + +function windowsCandidateIsAncestor( + candidate: WindowsProcessRow, + other: WindowsProcessRow, + candidatesByPid: ReadonlyMap +): boolean { + let current = candidatesByPid.get(other.ppid) + while (current) { + if (current.pid === candidate.pid) { + return true + } + current = candidatesByPid.get(current.ppid) + } + return false +} + +function normalizeContextPaths(contextPaths: readonly string[] | undefined): string[] { + const normalized = new Set() + for (const contextPath of contextPaths ?? []) { + const candidate = normalizePathForCommandMatch(contextPath) + if (isSafeContextPath(candidate)) { + normalized.add(candidate) + } + } + return [...normalized].sort((a, b) => b.length - a.length) +} + +function isSafeContextPath(contextPath: string): boolean { + return contextPath.length >= 4 && (/^[a-z]:\//.test(contextPath) || contextPath.startsWith('//')) +} + +function candidateMatchesContextPath( + candidate: WindowsProcessRow, + normalizedContextPaths: readonly string[] +): boolean { + if (normalizedContextPaths.length === 0) { + return false + } + const haystack = normalizePathForCommandMatch( + [candidate.command, candidate.executablePath].filter(Boolean).join('\n') + ) + return normalizedContextPaths.some((contextPath) => + commandLineContainsPath(haystack, contextPath) + ) +} + +function normalizePathForCommandMatch(value: string): string { + return value + .trim() + .replace(/^["']|["']$/g, '') + .replace(/\\/g, '/') + .replace(/\/+$/g, '') + .toLowerCase() +} + +function commandLineContainsPath(haystack: string, contextPath: string): boolean { + let index = haystack.indexOf(contextPath) + while (index !== -1) { + const before = index > 0 ? haystack[index - 1] : '' + const after = haystack[index + contextPath.length] ?? '' + const beforeOk = !before || /[\s"'(=]/.test(before) + const afterOk = !after || after === '/' || /[\s"'),;]/.test(after) + if (beforeOk && afterOk) { + return true + } + index = haystack.indexOf(contextPath, index + 1) + } + return false +} + +function recognizeWindowsProcessCandidate(candidate: WindowsProcessRow): string | null { + const recognized = + recognizeAgentProcessFromCommandLine(candidate.command) ?? + recognizeAgentProcessFromCommandLine(candidate.name) + return recognized?.processName ?? null +} + +function windowsCandidateMatchesFallbackWrapper( + candidate: WindowsProcessRow, + fallbackProcess: string +): boolean { + const commandToken = candidate.command.trim().split(/\s+/, 1)[0] ?? '' + return ( + isExpectedAgentProcess(candidate.name, fallbackProcess) || + isExpectedAgentProcess(commandToken, fallbackProcess) + ) +} diff --git a/src/main/providers/windows-foreground-process-rows.ts b/src/main/providers/windows-foreground-process-rows.ts new file mode 100644 index 00000000000..2dd23ad5b47 --- /dev/null +++ b/src/main/providers/windows-foreground-process-rows.ts @@ -0,0 +1,206 @@ +import { execFile } from 'child_process' +import { promisify } from 'util' + +const execFileAsync = promisify(execFile) +const WINDOWS_PROCESS_QUERY_TIMEOUT_MS = 3_000 +// Why: CommandLine can contain CR/LF text. JSON keeps process fields structured +// so an argument cannot masquerade as another `Name=` / `ProcessId=` row. +const POWERSHELL_PROCESS_QUERY = + '[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; ' + + 'Get-CimInstance -ClassName Win32_Process ' + + '-Property CommandLine,ExecutablePath,Name,ParentProcessId,ProcessId | ' + + 'Select-Object CommandLine,ExecutablePath,Name,ParentProcessId,ProcessId | ' + + 'ConvertTo-Json -Compress' + +export type WindowsProcessRow = { + pid: number + ppid: number + name: string + command: string + executablePath: string +} + +export type WindowsProcessCandidate = WindowsProcessRow & { depth: number } + +export async function queryWindowsProcessDescendants( + rootPid: number +): Promise { + const rows = + (await queryWindowsProcessesWithPowerShell()) ?? (await queryWindowsProcessesWithWmic()) + return rows ? collectDescendants(rows, rootPid).sort((a, b) => b.depth - a.depth) : null +} + +function parseWindowsProcessValueRows(stdout: string): WindowsProcessRow[] { + const rows: WindowsProcessRow[] = [] + let command = '' + let executablePath = '' + let name = '' + let pid = Number.NaN + let ppid = Number.NaN + + const flush = (): void => { + if (Number.isFinite(pid) && Number.isFinite(ppid)) { + rows.push({ pid, ppid, name, command: command || name, executablePath }) + } + command = '' + executablePath = '' + name = '' + pid = Number.NaN + ppid = Number.NaN + } + + for (const raw of stdout.split(/\r?\n/)) { + const line = raw.trim() + if (!line) { + flush() + continue + } + const eq = line.indexOf('=') + if (eq < 0) { + continue + } + const key = line.slice(0, eq) + const value = line.slice(eq + 1) + if (key === 'CommandLine') { + command = value + } else if (key === 'ExecutablePath') { + executablePath = value + } else if (key === 'Name') { + name = value + } else if (key === 'ParentProcessId') { + ppid = Number.parseInt(value, 10) + } else if (key === 'ProcessId') { + pid = Number.parseInt(value, 10) + } + } + flush() + return rows +} + +type WindowsProcessJsonRow = { + CommandLine?: unknown + ExecutablePath?: unknown + Name?: unknown + ParentProcessId?: unknown + ProcessId?: unknown +} + +function parseWindowsProcessJsonRows(stdout: string): WindowsProcessRow[] | null { + const trimmed = stdout.trim() + if (!trimmed) { + return [] + } + try { + const parsed = JSON.parse(trimmed) as unknown + const items = Array.isArray(parsed) ? parsed : [parsed] + return items.flatMap((item) => { + if (!item || typeof item !== 'object') { + return [] + } + const row = item as WindowsProcessJsonRow + const pid = numberFromWindowsProcessField(row.ProcessId) + const ppid = numberFromWindowsProcessField(row.ParentProcessId) + if (!Number.isFinite(pid) || !Number.isFinite(ppid)) { + return [] + } + const name = stringFromWindowsProcessField(row.Name) + const command = stringFromWindowsProcessField(row.CommandLine) || name + return [ + { + pid, + ppid, + name, + command, + executablePath: stringFromWindowsProcessField(row.ExecutablePath) + } + ] + }) + } catch { + return null + } +} + +function stringFromWindowsProcessField(value: unknown): string { + if (typeof value === 'string') { + return value + } + if (value === null || value === undefined) { + return '' + } + return String(value) +} + +function numberFromWindowsProcessField(value: unknown): number { + if (typeof value === 'number') { + return value + } + if (typeof value === 'string') { + return Number.parseInt(value, 10) + } + return Number.NaN +} + +function collectDescendants( + rows: Row[], + rootPid: number +): (Row & { depth: number })[] { + const childrenByParent = new Map() + for (const row of rows) { + const children = childrenByParent.get(row.ppid) ?? [] + children.push(row) + childrenByParent.set(row.ppid, children) + } + + const descendants: (Row & { depth: number })[] = [] + const stack = (childrenByParent.get(rootPid) ?? []).map((row) => ({ row, depth: 1 })) + while (stack.length > 0) { + const { row, depth } = stack.pop()! + descendants.push({ ...row, depth }) + for (const child of childrenByParent.get(row.pid) ?? []) { + stack.push({ row: child, depth: depth + 1 }) + } + } + return descendants +} + +async function queryWindowsProcessesWithPowerShell(): Promise { + try { + const { stdout } = await execFileAsync( + 'powershell.exe', + ['-NoProfile', '-NonInteractive', '-Command', POWERSHELL_PROCESS_QUERY], + { + encoding: 'utf8', + timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS, + maxBuffer: 8 * 1024 * 1024 + } + ) + const rows = parseWindowsProcessJsonRows(stdout) + return rows && rows.length > 0 ? rows : null + } catch { + return null + } +} + +async function queryWindowsProcessesWithWmic(): Promise { + try { + const { stdout } = await execFileAsync( + 'wmic', + [ + 'process', + 'get', + 'CommandLine,ExecutablePath,Name,ParentProcessId,ProcessId', + '/format:value' + ], + { + encoding: 'utf8', + timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS, + maxBuffer: 8 * 1024 * 1024 + } + ) + return parseWindowsProcessValueRows(stdout) + } catch { + // Best-effort: Windows process enumeration may be disabled, so callers + // still fall back to node-pty's process name when both probes fail. + return null + } +} diff --git a/src/main/runtime/orca-runtime-git.test.ts b/src/main/runtime/orca-runtime-git.test.ts index f8f6a630d76..4187f1d94be 100644 --- a/src/main/runtime/orca-runtime-git.test.ts +++ b/src/main/runtime/orca-runtime-git.test.ts @@ -18,6 +18,7 @@ const mocks = vi.hoisted(() => ({ generateCommitMessageFromContext: vi.fn(), generatePullRequestFieldsFromContext: vi.fn(), resolveCommitMessageSettings: vi.fn(), + resolveHostedReviewBodyForGeneration: vi.fn(), getSshGitProvider: vi.fn() })) @@ -53,6 +54,10 @@ vi.mock('../providers/ssh-git-dispatch', () => ({ getSshGitProvider: mocks.getSshGitProvider })) +vi.mock('../source-control/pull-request-template', () => ({ + resolveHostedReviewBodyForGeneration: mocks.resolveHostedReviewBodyForGeneration +})) + const tempDirs: string[] = [] function makeWorktree(path: string): ResolvedRuntimeGitWorktree { @@ -86,6 +91,8 @@ describe('RuntimeGitCommands', () => { mocks.generateCommitMessageFromContext.mockReset() mocks.generatePullRequestFieldsFromContext.mockReset() mocks.resolveCommitMessageSettings.mockReset() + mocks.resolveHostedReviewBodyForGeneration.mockReset() + mocks.resolveHostedReviewBodyForGeneration.mockImplementation(async ({ body }) => body) mocks.getSshGitProvider.mockReset() mocks.checkoutBranch.mockReset() mocks.listLocalBranches.mockReset() @@ -409,6 +416,69 @@ describe('RuntimeGitCommands', () => { ) }) + it('loads the hosted review template before generating pull-request fields', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const templateBody = '## Summary\n\n## Testing\n\n- [ ] Required checks' + const context = { + base: 'main', + branch: 'feature/template-aware-pr', + branchChangedByPreparation: false, + commitSummary: 'abc123 feat: test', + changeSummary: 'M README.md', + patch: '+hello', + currentTitle: '', + currentBody: templateBody, + currentDraft: false + } + const sourceControlAiResolvedParams = { + agentId: 'codex' as const, + model: 'gpt-5.5' + } + mocks.resolveHostedReviewBodyForGeneration.mockResolvedValue(templateBody) + mocks.getPullRequestDraftContext.mockResolvedValue(context) + mocks.generatePullRequestFieldsFromContext.mockResolvedValue({ + success: true, + fields: { + base: 'main', + title: 'Use existing template', + body: templateBody, + draft: false + } + }) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ worktree: makeWorktree(worktreePath) }), + getRuntimeSettings: () => ({}) as GlobalSettings + }) + + await commands.generateRuntimePullRequestFields( + 'id:wt-1', + { + base: 'main', + title: '', + body: '', + draft: false, + provider: 'gitlab', + useTemplate: true + }, + { sourceControlAiResolvedParams } + ) + + expect(mocks.resolveHostedReviewBodyForGeneration).toHaveBeenCalledWith({ + body: '', + repoPath: worktreePath, + connectionId: undefined, + provider: 'gitlab', + useTemplate: true + }) + expect(mocks.getPullRequestDraftContext).toHaveBeenCalledWith( + expect.any(Function), + expect.objectContaining({ + currentBody: templateBody + }) + ) + }) + it('resolves remote commit-message settings against the SSH host cache', async () => { const worktreePath = '/remote/repo' const context = { diff --git a/src/main/runtime/orca-runtime-git.ts b/src/main/runtime/orca-runtime-git.ts index 52e65e6d1fa..476632a9d0f 100644 --- a/src/main/runtime/orca-runtime-git.ts +++ b/src/main/runtime/orca-runtime-git.ts @@ -71,6 +71,8 @@ import { prepareLocalCommitMessageAgentEnv } from '../text-generation/commit-mes import { getPullRequestDraftContext } from '../text-generation/pull-request-context' import { normalizeRuntimeRelativePath } from './runtime-relative-paths' import { gitExecFileAsync } from '../git/runner' +import { resolveHostedReviewBodyForGeneration } from '../source-control/pull-request-template' +import type { HostedReviewProvider } from '../../shared/hosted-review' export type ResolvedRuntimeGitWorktree = Worktree & { git: GitWorktreeInfo } type RuntimeCommitMessageSettingsOverride = Partial< @@ -580,7 +582,14 @@ export class RuntimeGitCommands { async generateRuntimePullRequestFields( worktreeSelector: string, - input: { base: string; title: string; body: string; draft: boolean }, + input: { + base: string + title: string + body: string + draft: boolean + provider?: HostedReviewProvider + useTemplate?: boolean + }, settingsOverride?: RuntimeCommitMessageSettingsOverride ): Promise { const target = await this.host.resolveRuntimeGitTarget(worktreeSelector) @@ -612,11 +621,18 @@ export class RuntimeGitCommands { } let context: Awaited> try { + const currentBody = await resolveHostedReviewBodyForGeneration({ + body: input.body, + repoPath: target.worktree.path, + connectionId: target.connectionId, + provider: input.provider, + useTemplate: input.useTemplate + }) context = target.connectionId ? await getPullRequestDraftContext((argv) => provider!.exec(argv, target.worktree.path), { base: input.base, currentTitle: input.title, - currentBody: input.body, + currentBody, currentDraft: input.draft }) : await getPullRequestDraftContext( @@ -624,7 +640,7 @@ export class RuntimeGitCommands { { base: input.base, currentTitle: input.title, - currentBody: input.body, + currentBody, currentDraft: input.draft } ) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 6a2f202f3ff..e26b8361fdb 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -16034,6 +16034,10 @@ describe('OrcaRuntimeService', () => { getRepos: () => [localRepo], getRepo: (id: string) => (id === localRepo.id ? localRepo : undefined) } + getGitLabProjectRefForRemoteMock.mockResolvedValue({ + host: 'gitlab.example', + path: 'group/repo' + }) const runtime = new OrcaRuntimeService(runtimeStore as never) const gitSpy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockImplementation(async (args) => { if (args[0] === 'fetch') { @@ -16050,13 +16054,21 @@ describe('OrcaRuntimeService', () => { repoSelector: 'id:repo-1', mrIid: 42, sourceBranch: 'contrib/fix', + targetBranch: 'main', isCrossRepository: true }) - expect(result).toEqual({ baseBranch: 'fork-mr-sha' }) + expect(result).toEqual({ + baseBranch: 'fork-mr-sha', + compareBaseRef: 'refs/remotes/origin/main' + }) expect(gitSpy).toHaveBeenCalledWith(['fetch', 'origin', 'refs/merge-requests/42/head'], { cwd: TEST_REPO_PATH }) + expect(gitSpy).toHaveBeenCalledWith( + ['fetch', 'origin', '+refs/heads/main:refs/remotes/origin/main'], + { cwd: TEST_REPO_PATH } + ) expect(gitSpy).toHaveBeenCalledWith(['rev-parse', '--verify', 'FETCH_HEAD'], { cwd: TEST_REPO_PATH }) @@ -16089,7 +16101,8 @@ describe('OrcaRuntimeService', () => { return { stdout: 'remote-fork-mr-sha\n', stderr: '' } } throw new Error(`unexpected git call: ${args.join(' ')}`) - }) + }), + fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined) } registerSshGitProvider('ssh-1', provider as never) getGlabKnownHostsMock.mockResolvedValue(['gitlab.com', 'git.internal']) @@ -16099,14 +16112,24 @@ describe('OrcaRuntimeService', () => { repoSelector: 'id:repo-1', mrIid: 77, sourceBranch: 'contrib/remote-fix', + targetBranch: 'main', isCrossRepository: true }) - expect(result).toEqual({ baseBranch: 'remote-fork-mr-sha' }) + expect(result).toEqual({ + baseBranch: 'remote-fork-mr-sha', + compareBaseRef: 'refs/remotes/origin/main' + }) expect(provider.exec).toHaveBeenCalledWith( ['fetch', 'origin', 'refs/merge-requests/77/head'], '/remote/repo' ) + expect(provider.fetchRemoteTrackingRef).toHaveBeenCalledWith( + '/remote/repo', + 'origin', + 'main', + 'refs/remotes/origin/main' + ) expect(provider.exec).toHaveBeenCalledWith( ['rev-parse', '--verify', 'FETCH_HEAD'], '/remote/repo' diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index f916cbb81ed..4c270d510d2 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -10341,6 +10341,7 @@ export class OrcaRuntimeService { repoSelector: string name: string baseBranch?: string + compareBaseRef?: string branchNameOverride?: string linkedIssue?: number | null linkedPR?: number | null @@ -10810,9 +10811,9 @@ export class OrcaRuntimeService { const worktreeId = `${repo.id}::${created.path}` const now = Date.now() - // Why: persisted compare refs must survive local branches whose names look - // like remote labels, e.g. a local branch literally named "origin/main". - const metadataBaseRef = remoteTrackingBase?.ref ?? baseBranch + // Why: PR/MR-created worktrees can start from a head ref/SHA while Source + // Control must compare against the review target branch. + const metadataBaseRef = args.compareBaseRef ?? remoteTrackingBase?.ref ?? baseBranch const displayNameMeta = requestedDisplayName ? { displayName: requestedDisplayName } : shouldSetDisplayName(effectiveRequestedName, branchName, effectiveSanitizedName) @@ -11114,6 +11115,7 @@ export class OrcaRuntimeService { args: { name: string baseBranch?: string + compareBaseRef?: string branchNameOverride?: string linkedIssue?: number | null linkedPR?: number | null @@ -11159,6 +11161,7 @@ export class OrcaRuntimeService { name: args.name, ...(args.displayName ? { displayName: args.displayName } : {}), ...(args.baseBranch ? { baseBranch: args.baseBranch } : {}), + ...(args.compareBaseRef ? { compareBaseRef: args.compareBaseRef } : {}), ...(args.branchNameOverride ? { branchNameOverride: args.branchNameOverride } : {}), ...(args.runHooks ? { setupDecision: 'run' as const } : {}), ...(!args.runHooks && args.setupDecision ? { setupDecision: args.setupDecision } : {}), @@ -11851,6 +11854,7 @@ export class OrcaRuntimeService { repoSelector: string prNumber: number headRefName?: string + baseRefName?: string isCrossRepository?: boolean }): Promise { if (!this.store) { @@ -11900,6 +11904,7 @@ export class OrcaRuntimeService { repoPath: repo.path, prNumber: args.prNumber, headRefName: args.headRefName, + baseRefName: args.baseRefName, isCrossRepository: args.isCrossRepository, connectionId: repo.connectionId ?? null, gitExec, @@ -11912,8 +11917,11 @@ export class OrcaRuntimeService { repoSelector: string mrIid: number sourceBranch?: string + targetBranch?: string isCrossRepository?: boolean - }): Promise<{ baseBranch: string; pushTarget?: GitPushTarget } | { error: string }> { + }): Promise< + { baseBranch: string; compareBaseRef?: string; pushTarget?: GitPushTarget } | { error: string } + > { if (!this.store) { throw new Error('runtime_unavailable') } @@ -11932,6 +11940,7 @@ export class OrcaRuntimeService { : (gitArgs: string[]) => gitExecFileAsync(gitArgs, { cwd: repo.path }) let sourceBranch = args.sourceBranch?.trim() ?? '' + let targetBranch = args.targetBranch?.trim() ?? '' let isCrossRepository = args.isCrossRepository === true if (!sourceBranch) { @@ -11966,6 +11975,7 @@ export class OrcaRuntimeService { return { error: `MR !${args.mrIid} not found.` } } sourceBranch = (item.branchName ?? '').trim() + targetBranch = (item.baseRefName ?? '').trim() if (!sourceBranch) { return { error: `MR !${args.mrIid} has no source branch.` } } @@ -11984,6 +11994,21 @@ export class OrcaRuntimeService { } catch (error) { return { error: error instanceof Error ? error.message : 'Could not resolve git remote.' } } + const compareBaseRef = targetBranch ? `refs/remotes/${remote}/${targetBranch}` : undefined + const fetchTargetBranch = async (): Promise<{ error: string } | null> => { + if (!targetBranch || !compareBaseRef) { + return null + } + try { + await (sshGitProvider + ? sshGitProvider.fetchRemoteTrackingRef(repo.path, remote, targetBranch, compareBaseRef) + : gitExec(['fetch', remote, `+refs/heads/${targetBranch}:${compareBaseRef}`])) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + return { error: `Failed to fetch ${remote}/${targetBranch}: ${message.split('\n')[0]}` } + } + return null + } if (isCrossRepository) { const mrRef = `refs/merge-requests/${args.mrIid}/head` @@ -12005,7 +12030,11 @@ export class OrcaRuntimeService { if (!sha) { return { error: `Empty SHA resolving fork MR !${args.mrIid} head.` } } - return { baseBranch: sha } + const targetFetchError = await fetchTargetBranch() + if (targetFetchError) { + return targetFetchError + } + return { baseBranch: sha, ...(compareBaseRef ? { compareBaseRef } : {}) } } try { @@ -12025,7 +12054,15 @@ export class OrcaRuntimeService { } catch { return { error: `Remote ref ${remoteRef} does not exist after fetch.` } } - return { baseBranch: remoteRef, pushTarget: { remoteName: remote, branchName: sourceBranch } } + const targetFetchError = await fetchTargetBranch() + if (targetFetchError) { + return targetFetchError + } + return { + baseBranch: remoteRef, + ...(compareBaseRef ? { compareBaseRef } : {}), + pushTarget: { remoteName: remote, branchName: sourceBranch } + } } private async resolveGitLabIssueSourceRemote( diff --git a/src/main/runtime/rpc/methods/git-params.ts b/src/main/runtime/rpc/methods/git-params.ts index e668670b3e2..ca80c969709 100644 --- a/src/main/runtime/rpc/methods/git-params.ts +++ b/src/main/runtime/rpc/methods/git-params.ts @@ -172,7 +172,11 @@ export const GitGeneratePullRequestFields = GitGenerateCommitMessage.extend({ base: z.string().min(1, 'Missing base branch'), title: z.string(), body: z.string(), - draft: z.boolean() + draft: z.boolean(), + provider: z + .enum(['github', 'gitlab', 'bitbucket', 'azure-devops', 'gitea', 'unsupported']) + .optional(), + useTemplate: z.boolean().optional() }) export const GitBulkPaths = WorktreeSelector.extend({ diff --git a/src/main/runtime/rpc/methods/git.test.ts b/src/main/runtime/rpc/methods/git.test.ts index 227f6394ce0..b5937d8fc61 100644 --- a/src/main/runtime/rpc/methods/git.test.ts +++ b/src/main/runtime/rpc/methods/git.test.ts @@ -512,13 +512,22 @@ describe('git RPC methods', () => { title: '', body: '', draft: false, + provider: 'github', + useTemplate: true, sourceControlAiResolvedParams }) ) expect(runtime.generateRuntimePullRequestFields).toHaveBeenCalledWith( 'id:wt-1', - { base: 'main', title: '', body: '', draft: false }, + { + base: 'main', + title: '', + body: '', + draft: false, + provider: 'github', + useTemplate: true + }, { sourceControlAiResolvedParams } ) }) diff --git a/src/main/runtime/rpc/methods/git.ts b/src/main/runtime/rpc/methods/git.ts index eef0ed2f633..06cc3205452 100644 --- a/src/main/runtime/rpc/methods/git.ts +++ b/src/main/runtime/rpc/methods/git.ts @@ -278,7 +278,9 @@ export const GIT_METHODS: RpcMethod[] = [ base: params.base, title: params.title, body: params.body, - draft: params.draft + draft: params.draft, + provider: params.provider, + useTemplate: params.useTemplate } const override = buildCommitMessageGenerationOverride(params) if (override === undefined) { diff --git a/src/main/runtime/rpc/methods/worktree-schemas.ts b/src/main/runtime/rpc/methods/worktree-schemas.ts index 72dc48dd67c..0d3ffeab461 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.ts @@ -55,6 +55,7 @@ export const WorktreeCreate = z .pipe(z.string().min(1, 'Missing repo selector')), name: OptionalString, baseBranch: OptionalString, + compareBaseRef: OptionalString, branchNameOverride: OptionalString, linkedIssue: TriStateLinkedIssue, linkedPR: TriStateLinkedIssue, @@ -231,6 +232,7 @@ export const WorktreeResolvePrBase = z.object({ .transform((v) => (typeof v === 'number' && Number.isFinite(v) ? v : 0)) .pipe(z.number().int().positive('Missing PR number')), headRefName: OptionalString, + baseRefName: OptionalString, isCrossRepository: OptionalBoolean }) @@ -244,5 +246,6 @@ export const WorktreeResolveMrBase = z.object({ .transform((v) => (typeof v === 'number' && Number.isFinite(v) ? v : 0)) .pipe(z.number().int().positive('Missing MR number')), sourceBranch: OptionalString, + targetBranch: OptionalString, isCrossRepository: OptionalBoolean }) diff --git a/src/main/runtime/rpc/methods/worktree.ts b/src/main/runtime/rpc/methods/worktree.ts index bdfa9c76612..5f09061dc3c 100644 --- a/src/main/runtime/rpc/methods/worktree.ts +++ b/src/main/runtime/rpc/methods/worktree.ts @@ -63,6 +63,7 @@ export const WORKTREE_METHODS: RpcMethod[] = [ repoSelector: params.repo, name: params.name ?? '', baseBranch: params.baseBranch, + compareBaseRef: params.compareBaseRef, branchNameOverride: params.branchNameOverride, linkedIssue: params.linkedIssue, linkedPR: params.linkedPR, @@ -172,6 +173,7 @@ export const WORKTREE_METHODS: RpcMethod[] = [ repoSelector: params.repo, prNumber: params.prNumber, headRefName: params.headRefName, + baseRefName: params.baseRefName, isCrossRepository: params.isCrossRepository }) }), @@ -183,6 +185,7 @@ export const WORKTREE_METHODS: RpcMethod[] = [ repoSelector: params.repo, mrIid: params.mrIid, sourceBranch: params.sourceBranch, + targetBranch: params.targetBranch, isCrossRepository: params.isCrossRepository }) }), diff --git a/src/main/source-control/forge-provider.test.ts b/src/main/source-control/forge-provider.test.ts index fc023efc086..c08bebf1c7f 100644 --- a/src/main/source-control/forge-provider.test.ts +++ b/src/main/source-control/forge-provider.test.ts @@ -3,6 +3,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const { createGitHubPullRequestMock, createGitLabMergeRequestMock, + createAzureDevOpsPullRequestMock, + createGiteaPullRequestMock, getAzureDevOpsRepoSlugMock, getBitbucketRepoSlugMock, getGiteaRepoSlugMock, @@ -13,6 +15,8 @@ const { } = vi.hoisted(() => ({ createGitHubPullRequestMock: vi.fn(), createGitLabMergeRequestMock: vi.fn(), + createAzureDevOpsPullRequestMock: vi.fn(), + createGiteaPullRequestMock: vi.fn(), getAzureDevOpsRepoSlugMock: vi.fn(), getBitbucketRepoSlugMock: vi.fn(), getGiteaRepoSlugMock: vi.fn(), @@ -50,12 +54,20 @@ vi.mock('../azure-devops/client', () => ({ getAzureDevOpsPullRequest: vi.fn() })) +vi.mock('../azure-devops/pull-request-creation', () => ({ + createAzureDevOpsPullRequest: createAzureDevOpsPullRequestMock +})) + vi.mock('../gitea/client', () => ({ getGiteaRepoSlug: getGiteaRepoSlugMock, getGiteaPullRequestForBranch: vi.fn(), getGiteaPullRequest: vi.fn() })) +vi.mock('../gitea/pull-request-creation', () => ({ + createGiteaPullRequest: createGiteaPullRequestMock +})) + import { FORGE_PROVIDERS, detectHostedReviewProvider, @@ -67,6 +79,8 @@ describe('forge provider interface', () => { beforeEach(() => { createGitHubPullRequestMock.mockReset() createGitLabMergeRequestMock.mockReset() + createAzureDevOpsPullRequestMock.mockReset() + createGiteaPullRequestMock.mockReset() getAzureDevOpsRepoSlugMock.mockReset() getBitbucketRepoSlugMock.mockReset() getGiteaRepoSlugMock.mockReset() @@ -94,8 +108,8 @@ describe('forge provider interface', () => { ['gitlab', true], ['github', true], ['bitbucket', false], - ['azure-devops', false], - ['gitea', false] + ['azure-devops', true], + ['gitea', true] ]) createGitHubPullRequestMock.mockResolvedValue({ ok: true, @@ -160,6 +174,78 @@ describe('forge provider interface', () => { ) }) + it('routes Azure DevOps review creation through the shared provider contract', async () => { + createAzureDevOpsPullRequestMock.mockResolvedValue({ + ok: true, + number: 88, + url: 'https://dev.azure.com/acme/Project/_git/orca/pullrequest/88' + }) + + const provider = getForgeProviderById('azure-devops') + await expect( + provider.createReview?.( + '/repo', + { + provider: 'azure-devops', + base: 'main', + head: 'feature/provider-interface', + title: 'Add provider interface' + }, + 'ssh-1' + ) + ).resolves.toEqual({ + ok: true, + number: 88, + url: 'https://dev.azure.com/acme/Project/_git/orca/pullrequest/88' + }) + expect(createAzureDevOpsPullRequestMock).toHaveBeenCalledWith( + '/repo', + { + provider: 'azure-devops', + base: 'main', + head: 'feature/provider-interface', + title: 'Add provider interface' + }, + 'ssh-1' + ) + }) + + it('routes Gitea review creation through the shared provider contract', async () => { + createGiteaPullRequestMock.mockResolvedValue({ + ok: true, + number: 19, + url: 'https://git.example.com/team/orca/pulls/19' + }) + + const provider = getForgeProviderById('gitea') + await expect( + provider.createReview?.( + '/repo', + { + provider: 'gitea', + base: 'main', + head: 'feature/provider-interface', + title: 'Add provider interface' + }, + 'ssh-1' + ) + ).resolves.toEqual({ + ok: true, + number: 19, + url: 'https://git.example.com/team/orca/pulls/19' + }) + expect(createGiteaPullRequestMock).toHaveBeenCalledWith( + '/repo', + { + provider: 'gitea', + base: 'main', + head: 'feature/provider-interface', + title: 'Add provider interface' + }, + 'ssh-1' + ) + }) + it('adapts GitHub branch lookup through the shared provider contract', async () => { getPRForBranchMock.mockResolvedValue({ number: 7, diff --git a/src/main/source-control/forge-provider.ts b/src/main/source-control/forge-provider.ts index 41fa093792d..f9214c965a3 100644 --- a/src/main/source-control/forge-provider.ts +++ b/src/main/source-control/forge-provider.ts @@ -11,6 +11,7 @@ import { getAzureDevOpsPullRequestForBranch, getAzureDevOpsRepoSlug } from '../azure-devops/client' +import { createAzureDevOpsPullRequest } from '../azure-devops/pull-request-creation' import type { AzureDevOpsPullRequestInfo } from '../azure-devops/pull-request-mappers' import { getBitbucketPullRequest, @@ -23,6 +24,7 @@ import { getGiteaPullRequestForBranch, getGiteaRepoSlug } from '../gitea/client' +import { createGiteaPullRequest } from '../gitea/pull-request-creation' import type { GiteaPullRequestInfo } from '../gitea/pull-request-mappers' import { createGitHubPullRequest, getPRForBranch, getRepoSlug } from '../github/client' import { getMergeRequest, getMergeRequestForBranch, getProjectSlug } from '../gitlab/client' @@ -80,6 +82,7 @@ function mapGitLabReview(mr: MRInfo): HostedReviewInfo { updatedAt: mr.updatedAt, mergeable: mr.mergeable, ...(mr.headSha ? { headSha: mr.headSha } : {}), + ...(mr.baseRefName ? { baseRefName: mr.baseRefName } : {}), ...(mr.conflictSummary ? { conflictSummary: mr.conflictSummary } : {}) } } @@ -198,7 +201,7 @@ const bitbucketForgeProvider = { const azureDevOpsForgeProvider = { id: 'azure-devops', - supportsReviewCreation: false, + supportsReviewCreation: true, resolveRepository: ({ repoPath, connectionId }) => getAzureDevOpsRepoSlug(repoPath, connectionId), async getReviewForBranch(input) { const pr = await getAzureDevOpsPullRequestForBranch( @@ -212,12 +215,13 @@ const azureDevOpsForgeProvider = { async getReviewByNumber(input) { const pr = await getAzureDevOpsPullRequest(input.repoPath, input.number, input.connectionId) return pr ? mapAzureDevOpsReview(pr) : null - } + }, + createReview: createAzureDevOpsPullRequest } satisfies ForgeProvider const giteaForgeProvider = { id: 'gitea', - supportsReviewCreation: false, + supportsReviewCreation: true, resolveRepository: ({ repoPath, connectionId }) => getGiteaRepoSlug(repoPath, connectionId), async getReviewForBranch(input) { const pr = await getGiteaPullRequestForBranch( @@ -231,7 +235,8 @@ const giteaForgeProvider = { async getReviewByNumber(input) { const pr = await getGiteaPullRequest(input.repoPath, input.number, input.connectionId) return pr ? mapGiteaReview(pr) : null - } + }, + createReview: createGiteaPullRequest } satisfies ForgeProvider // Why: provider order preserves existing branch-status behavior when remotes diff --git a/src/main/source-control/hosted-review-api-request.ts b/src/main/source-control/hosted-review-api-request.ts new file mode 100644 index 00000000000..90e4d197ad8 --- /dev/null +++ b/src/main/source-control/hosted-review-api-request.ts @@ -0,0 +1,48 @@ +export class HostedReviewApiRequestError extends Error { + readonly status: number | null + readonly timedOut: boolean + + constructor(message: string, options: { status?: number | null; timedOut?: boolean } = {}) { + super(message) + this.name = 'HostedReviewApiRequestError' + this.status = options.status ?? null + this.timedOut = options.timedOut ?? false + } +} + +async function readResponseText(response: Response): Promise { + try { + return await response.text() + } catch { + return '' + } +} + +export async function requestHostedReviewJson( + url: URL, + init: Omit, + timeoutMs: number +): Promise { + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), timeoutMs) + try { + const response = await fetch(url, { ...init, signal: controller.signal }) + if (!response.ok) { + const body = await readResponseText(response) + throw new HostedReviewApiRequestError(body || response.statusText, { + status: response.status + }) + } + return (await response.json()) as T + } catch (error) { + if (error instanceof HostedReviewApiRequestError) { + throw error + } + if (error instanceof Error && error.name === 'AbortError') { + throw new HostedReviewApiRequestError('Request timed out', { timedOut: true }) + } + throw error + } finally { + clearTimeout(timeout) + } +} diff --git a/src/main/source-control/hosted-review-creation.test.ts b/src/main/source-control/hosted-review-creation.test.ts index 117f6ffcbf8..77a3d60b8e2 100644 --- a/src/main/source-control/hosted-review-creation.test.ts +++ b/src/main/source-control/hosted-review-creation.test.ts @@ -4,6 +4,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const { createGitHubPullRequestMock, createGitLabMergeRequestMock, + createAzureDevOpsPullRequestMock, + createGiteaPullRequestMock, + isAzureDevOpsReviewCreationAuthenticatedMock, + isGiteaReviewCreationAuthenticatedMock, getRepoSlugMock, getProjectSlugMock, getBitbucketRepoSlugMock, @@ -18,6 +22,10 @@ const { } = vi.hoisted(() => ({ createGitHubPullRequestMock: vi.fn(), createGitLabMergeRequestMock: vi.fn(), + createAzureDevOpsPullRequestMock: vi.fn(), + createGiteaPullRequestMock: vi.fn(), + isAzureDevOpsReviewCreationAuthenticatedMock: vi.fn(), + isGiteaReviewCreationAuthenticatedMock: vi.fn(), getRepoSlugMock: vi.fn(), getProjectSlugMock: vi.fn(), getBitbucketRepoSlugMock: vi.fn(), @@ -59,12 +67,22 @@ vi.mock('../azure-devops/client', () => ({ getAzureDevOpsPullRequest: vi.fn() })) +vi.mock('../azure-devops/pull-request-creation', () => ({ + createAzureDevOpsPullRequest: createAzureDevOpsPullRequestMock, + isAzureDevOpsReviewCreationAuthenticated: isAzureDevOpsReviewCreationAuthenticatedMock +})) + vi.mock('../gitea/client', () => ({ getGiteaRepoSlug: getGiteaRepoSlugMock, getGiteaPullRequestForBranch: vi.fn(), getGiteaPullRequest: vi.fn() })) +vi.mock('../gitea/pull-request-creation', () => ({ + createGiteaPullRequest: createGiteaPullRequestMock, + isGiteaReviewCreationAuthenticated: isGiteaReviewCreationAuthenticatedMock +})) + vi.mock('../github/gh-utils', () => ({ acquire: vi.fn(), release: vi.fn(), @@ -98,6 +116,10 @@ function resetMocks(): void { for (const mock of [ createGitHubPullRequestMock, createGitLabMergeRequestMock, + createAzureDevOpsPullRequestMock, + createGiteaPullRequestMock, + isAzureDevOpsReviewCreationAuthenticatedMock, + isGiteaReviewCreationAuthenticatedMock, getRepoSlugMock, getProjectSlugMock, getBitbucketRepoSlugMock, @@ -130,6 +152,34 @@ function mockGitLabProvider(): void { getGiteaRepoSlugMock.mockResolvedValue(null) } +function mockAzureDevOpsProvider(): void { + getProjectSlugMock.mockResolvedValue(null) + getRepoSlugMock.mockResolvedValue(null) + getBitbucketRepoSlugMock.mockResolvedValue(null) + getAzureDevOpsRepoSlugMock.mockResolvedValue({ + host: 'dev.azure.com', + project: 'Project', + repository: 'orca', + apiBaseUrl: 'https://dev.azure.com/acme/Project', + webBaseUrl: 'https://dev.azure.com/acme/Project/_git/orca' + }) + getGiteaRepoSlugMock.mockResolvedValue(null) +} + +function mockGiteaProvider(): void { + getProjectSlugMock.mockResolvedValue(null) + getRepoSlugMock.mockResolvedValue(null) + getBitbucketRepoSlugMock.mockResolvedValue(null) + getAzureDevOpsRepoSlugMock.mockResolvedValue(null) + getGiteaRepoSlugMock.mockResolvedValue({ + host: 'git.example.com', + owner: 'acme', + repo: 'orca', + apiBaseUrl: 'https://git.example.com/api/v1', + webBaseUrl: 'https://git.example.com' + }) +} + describe('createHostedReview', () => { beforeEach(() => { resetMocks() @@ -169,6 +219,18 @@ describe('createHostedReview', () => { number: 44, url: 'https://gitlab.com/acme/orca/-/merge_requests/44' }) + createAzureDevOpsPullRequestMock.mockResolvedValue({ + ok: true, + number: 88, + url: 'https://dev.azure.com/acme/Project/_git/orca/pullrequest/88' + }) + createGiteaPullRequestMock.mockResolvedValue({ + ok: true, + number: 19, + url: 'https://git.example.com/acme/orca/pulls/19' + }) + isAzureDevOpsReviewCreationAuthenticatedMock.mockReturnValue(true) + isGiteaReviewCreationAuthenticatedMock.mockReturnValue(true) }) it('revalidates ahead commits before creating a GitHub pull request', async () => { @@ -266,6 +328,66 @@ describe('createHostedReview', () => { expect(createGitHubPullRequestMock).not.toHaveBeenCalled() }) + it('creates an Azure DevOps pull request after fresh main-process validation passes', async () => { + mockAzureDevOpsProvider() + + await expect( + createHostedReview('/repo', { + provider: 'azure-devops', + base: 'main', + head: 'feature', + title: 'Feature' + }) + ).resolves.toEqual({ + ok: true, + number: 88, + url: 'https://dev.azure.com/acme/Project/_git/orca/pullrequest/88' + }) + + expect(createAzureDevOpsPullRequestMock).toHaveBeenCalledWith( + '/repo', + { + provider: 'azure-devops', + base: 'main', + head: 'feature', + title: 'Feature' + }, + undefined + ) + expect(createGitHubPullRequestMock).not.toHaveBeenCalled() + expect(createGitLabMergeRequestMock).not.toHaveBeenCalled() + }) + + it('creates a Gitea pull request after fresh main-process validation passes', async () => { + mockGiteaProvider() + + await expect( + createHostedReview('/repo', { + provider: 'gitea', + base: 'main', + head: 'feature', + title: 'Feature' + }) + ).resolves.toEqual({ + ok: true, + number: 19, + url: 'https://git.example.com/acme/orca/pulls/19' + }) + + expect(createGiteaPullRequestMock).toHaveBeenCalledWith( + '/repo', + { + provider: 'gitea', + base: 'main', + head: 'feature', + title: 'Feature' + }, + undefined + ) + expect(createGitHubPullRequestMock).not.toHaveBeenCalled() + expect(createGitLabMergeRequestMock).not.toHaveBeenCalled() + }) + it('uses the SSH git provider for remote hosted-review preflight', async () => { const remoteGit = { getStatus: vi.fn(async () => ({ entries: [], conflictOperation: 'unknown' })), @@ -375,6 +497,8 @@ describe('getHostedReviewCreationEligibility', () => { getHostedReviewForBranchMock.mockResolvedValue(null) ghExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) gitExecFileAsyncMock.mockResolvedValue({ stdout: 'Feature title\n', stderr: '' }) + isAzureDevOpsReviewCreationAuthenticatedMock.mockReturnValue(true) + isGiteaReviewCreationAuthenticatedMock.mockReturnValue(true) }) it('treats short remote base refs as the default branch name', async () => { @@ -511,4 +635,54 @@ describe('getHostedReviewCreationEligibility', () => { { cwd: '/repo' } ) }) + + it('enables creation for clean, in-sync, token-configured Azure DevOps feature branches', async () => { + mockAzureDevOpsProvider() + + await expect( + getHostedReviewCreationEligibility({ + repoPath: '/repo', + branch: 'feature/azure', + base: 'main', + hasUncommittedChanges: false, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + ).resolves.toMatchObject({ + provider: 'azure-devops', + canCreate: true, + blockedReason: null, + nextAction: null, + head: 'feature/azure' + }) + expect(isAzureDevOpsReviewCreationAuthenticatedMock).toHaveBeenCalledOnce() + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() + expect(glabExecFileAsyncMock).not.toHaveBeenCalled() + }) + + it('enables creation for clean, in-sync, token-configured Gitea feature branches', async () => { + mockGiteaProvider() + + await expect( + getHostedReviewCreationEligibility({ + repoPath: '/repo', + branch: 'feature/gitea', + base: 'main', + hasUncommittedChanges: false, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + ).resolves.toMatchObject({ + provider: 'gitea', + canCreate: true, + blockedReason: null, + nextAction: null, + head: 'feature/gitea' + }) + expect(isGiteaReviewCreationAuthenticatedMock).toHaveBeenCalledOnce() + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() + expect(glabExecFileAsyncMock).not.toHaveBeenCalled() + }) }) diff --git a/src/main/source-control/hosted-review-creation.ts b/src/main/source-control/hosted-review-creation.ts index 2dbde4b03ec..7b38d7c073b 100644 --- a/src/main/source-control/hosted-review-creation.ts +++ b/src/main/source-control/hosted-review-creation.ts @@ -12,6 +12,12 @@ import { normalizeHostedReviewBaseRef, normalizeHostedReviewHeadRef } from '../../shared/hosted-review-refs' +import { + supportsHostedReviewCreation, + type HostedReviewCreationProvider +} from '../../shared/hosted-review-creation-providers' +import { isAzureDevOpsReviewCreationAuthenticated } from '../azure-devops/pull-request-creation' +import { isGiteaReviewCreationAuthenticated } from '../gitea/pull-request-creation' import { acquire, ghExecFileAsync, gitExecFileAsync, release } from '../github/gh-utils' import { isNoUpstreamError, normalizeGitErrorMessage } from '../../shared/git-remote-error' import type { GitUpstreamStatus } from '../../shared/types' @@ -160,22 +166,56 @@ async function getHostedReviewUpstreamStatus( function reviewCopy(provider: HostedReviewProvider): { shortLabel: 'PR' | 'MR' reviewLabel: 'pull request' | 'merge request' - providerName: 'GitHub' | 'GitLab' - authCommand: 'gh auth login' | 'glab auth login' + providerName: string + authInstruction: string } { - return provider === 'gitlab' - ? { - shortLabel: 'MR', - reviewLabel: 'merge request', - providerName: 'GitLab', - authCommand: 'glab auth login' - } - : { - shortLabel: 'PR', - reviewLabel: 'pull request', - providerName: 'GitHub', - authCommand: 'gh auth login' - } + if (provider === 'gitlab') { + return { + shortLabel: 'MR', + reviewLabel: 'merge request', + providerName: 'GitLab', + authInstruction: 'Run glab auth login' + } + } + if (provider === 'azure-devops') { + return { + shortLabel: 'PR', + reviewLabel: 'pull request', + providerName: 'Azure DevOps', + authInstruction: 'Set ORCA_AZURE_DEVOPS_TOKEN' + } + } + if (provider === 'gitea') { + return { + shortLabel: 'PR', + reviewLabel: 'pull request', + providerName: 'Gitea', + authInstruction: 'Set ORCA_GITEA_TOKEN' + } + } + return { + shortLabel: 'PR', + reviewLabel: 'pull request', + providerName: 'GitHub', + authInstruction: 'Run gh auth login' + } +} + +async function isProviderAuthenticated( + provider: HostedReviewCreationProvider, + repoPath: string, + connectionId?: string | null +): Promise { + if (provider === 'gitlab') { + return isGitLabAuthenticated(repoPath, connectionId) + } + if (provider === 'azure-devops') { + return isAzureDevOpsReviewCreationAuthenticated() + } + if (provider === 'gitea') { + return isGiteaReviewCreationAuthenticated() + } + return isGitHubAuthenticated(repoPath, connectionId) } function blockedCreateResultForReason( @@ -187,7 +227,7 @@ function blockedCreateResultForReason( auth_required: { ok: false, code: 'auth_required', - error: `Create ${copy.shortLabel} failed: ${copy.providerName} is not authenticated. Next step: run ${copy.authCommand} in this environment.` + error: `Create ${copy.shortLabel} failed: ${copy.providerName} is not authenticated. Next step: ${copy.authInstruction} in this environment.` }, unsupported_provider: { ok: false, @@ -346,7 +386,7 @@ export async function getHostedReviewCreationEligibility( nextAction: 'open_existing_review' } } - if (provider !== 'github' && provider !== 'gitlab') { + if (!supportsHostedReviewCreation(provider)) { return { ...baseResult, canCreate: false, @@ -369,10 +409,7 @@ export async function getHostedReviewCreationEligibility( if ((args.behind ?? 0) > 0) { return { ...baseResult, canCreate: false, blockedReason: 'needs_sync', nextAction: 'sync' } } - const authenticated = - provider === 'gitlab' - ? await isGitLabAuthenticated(args.repoPath, args.connectionId) - : await isGitHubAuthenticated(args.repoPath, args.connectionId) + const authenticated = await isProviderAuthenticated(provider, args.repoPath, args.connectionId) if (!authenticated) { return { ...baseResult, @@ -392,7 +429,7 @@ export async function createHostedReview( input: CreateHostedReviewInput, connectionId?: string | null ): Promise { - if (input.provider !== 'github' && input.provider !== 'gitlab') { + if (!supportsHostedReviewCreation(input.provider)) { return { ok: false, code: 'unsupported_provider', diff --git a/src/main/source-control/pull-request-template.ts b/src/main/source-control/pull-request-template.ts new file mode 100644 index 00000000000..47e0b7e3920 --- /dev/null +++ b/src/main/source-control/pull-request-template.ts @@ -0,0 +1,82 @@ +import { readFile } from 'fs/promises' +import { join } from 'path' +import type { HostedReviewProvider } from '../../shared/hosted-review' +import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' +import { joinWorktreeRelativePath } from '../runtime/runtime-relative-paths' + +const PULL_REQUEST_TEMPLATE_CANDIDATES = [ + '.github/pull_request_template.md', + '.github/PULL_REQUEST_TEMPLATE.md', + '.azuredevops/pull_request_template.md', + '.azuredevops/PULL_REQUEST_TEMPLATE.md', + '.gitea/pull_request_template.md', + '.gitea/PULL_REQUEST_TEMPLATE.md', + 'pull_request_template.md', + 'PULL_REQUEST_TEMPLATE.md', + 'docs/pull_request_template.md', + 'docs/PULL_REQUEST_TEMPLATE.md' +] + +const MERGE_REQUEST_TEMPLATE_CANDIDATES = [ + '.gitlab/merge_request_templates/Default.md', + '.gitlab/merge_request_templates/default.md', + '.gitlab/merge_request_template.md', + '.gitlab/MERGE_REQUEST_TEMPLATE.md' +] + +function getTemplateCandidates(provider?: HostedReviewProvider | null): string[] { + if (provider === 'gitlab') { + return [...MERGE_REQUEST_TEMPLATE_CANDIDATES, ...PULL_REQUEST_TEMPLATE_CANDIDATES] + } + return PULL_REQUEST_TEMPLATE_CANDIDATES +} + +export async function readHostedPullRequestTemplate( + repoPath: string, + connectionId?: string | null +): Promise { + return readHostedReviewTemplate(repoPath, connectionId) +} + +export async function readHostedReviewTemplate( + repoPath: string, + connectionId?: string | null, + provider?: HostedReviewProvider | null +): Promise { + const remoteProvider = connectionId ? getSshFilesystemProvider(connectionId) : undefined + if (connectionId && !remoteProvider) { + return '' + } + for (const relativeCandidate of getTemplateCandidates(provider)) { + try { + if (remoteProvider) { + const result = await remoteProvider.readFile( + joinWorktreeRelativePath(repoPath, relativeCandidate) + ) + if (result.isBinary) { + continue + } + return result.content + } + return await readFile(join(repoPath, relativeCandidate), 'utf8') + } catch { + // Try the next conventional hosted-review template path. + } + } + return '' +} + +export async function resolveHostedReviewBodyForGeneration(args: { + body: string + repoPath: string + connectionId?: string | null + provider?: HostedReviewProvider | null + useTemplate?: boolean +}): Promise { + if (!args.useTemplate || args.body.trim()) { + return args.body + } + // Why: generated non-empty bodies bypass provider-side template fallback, so + // preload the template into the AI context when the user asked to use it. + return readHostedReviewTemplate(args.repoPath, args.connectionId, args.provider) +} diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index d2094e4bc23..59823b5fb45 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -5,7 +5,8 @@ import type { HostedReviewCreationEligibility, HostedReviewCreationEligibilityArgs, HostedReviewForBranchArgs, - HostedReviewInfo + HostedReviewInfo, + HostedReviewProvider } from '../shared/hosted-review' import type { NativeFileDropPayload } from '../shared/native-file-drop' import type { AppIdentity } from '../shared/app-identity' @@ -938,6 +939,7 @@ export type PreloadApi = { repoId: string prNumber: number headRefName?: string + baseRefName?: string isCrossRepository?: boolean }) => Promise /** GitLab parallel of resolvePrBase. For same-project MRs returns @@ -947,8 +949,12 @@ export type PreloadApi = { repoId: string mrIid: number sourceBranch?: string + targetBranch?: string isCrossRepository?: boolean - }) => Promise<{ baseBranch: string; pushTarget?: GitPushTarget } | { error: string }> + }) => Promise< + | { baseBranch: string; compareBaseRef?: string; pushTarget?: GitPushTarget } + | { error: string } + > remove: (args: { worktreeId: string force?: boolean @@ -2240,6 +2246,8 @@ export type PreloadApi = { title: string body: string draft: boolean + provider?: HostedReviewProvider + useTemplate?: boolean connectionId?: string sourceControlAiResolvedParams?: ResolvedSourceControlAiGenerationParams sourceControlAi?: SourceControlAiSettings diff --git a/src/preload/index.ts b/src/preload/index.ts index a12f33d828a..1e3c7c7a0b4 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2660,6 +2660,8 @@ const api = { title: string body: string draft: boolean + provider?: unknown + useTemplate?: boolean connectionId?: string sourceControlAiResolvedParams?: unknown sourceControlAi?: unknown diff --git a/src/relay/pty-shell-utils.test.ts b/src/relay/pty-shell-utils.test.ts index e308bb88c38..61180b88975 100644 --- a/src/relay/pty-shell-utils.test.ts +++ b/src/relay/pty-shell-utils.test.ts @@ -30,6 +30,21 @@ function mockExecFile( ) } +async function withProcessPlatform( + platform: NodeJS.Platform, + run: () => T | Promise +): Promise { + const descriptor = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + try { + return await run() + } finally { + if (descriptor) { + Object.defineProperty(process, 'platform', descriptor) + } + } +} + beforeEach(() => { execFileMock.mockReset() }) @@ -115,66 +130,106 @@ describe('getForegroundProcessName', () => { }) it('recognizes SSH relay node-wrapped agents from descendant command lines', async () => { - mockExecFile((_command, args) => { - if (args[0] === '-axo') { - return { - stdout: ['100 99 Ss bash -l', '101 100 S+ node /home/dev/.local/bin/codex'].join('\n') + await withProcessPlatform('linux', async () => { + mockExecFile((_command, args) => { + if (args[0] === '-axo') { + return { + stdout: ['100 99 Ss bash -l', '101 100 S+ node /home/dev/.local/bin/codex'].join( + '\n' + ) + } } - } - return new Error('unexpected command') - }) + return new Error('unexpected command') + }) - await expect(getForegroundProcessName(100, 'node')).resolves.toBe('codex') + await expect(getForegroundProcessName(100, 'node')).resolves.toBe('codex') + }) + }) + + it('recognizes Windows SSH relay shell-rooted agent descendants', async () => { + await withProcessPlatform('win32', async () => { + mockExecFile((command) => { + if (command === 'powershell.exe') { + return { + stdout: JSON.stringify([ + { + CommandLine: 'powershell.exe', + ExecutablePath: 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', + Name: 'powershell.exe', + ParentProcessId: 99, + ProcessId: 100 + }, + { + CommandLine: 'node C:\\Users\\dev\\AppData\\Roaming\\npm\\codex.cmd', + ExecutablePath: 'C:\\Program Files\\nodejs\\node.exe', + Name: 'node.exe', + ParentProcessId: 100, + ProcessId: 101 + } + ]) + } + } + return new Error('unexpected command') + }) + + await expect(getForegroundProcessName(100, 'powershell.exe')).resolves.toBe('codex') + }) }) it('recognizes SSH relay wrapped agents when no foreground marker is available', async () => { - mockExecFile((_command, args) => { - if (args[0] === '-axo') { - return { - stdout: [ - '100 99 Ss bash -l', - '101 100 S node /home/dev/.local/bin/node_modules/@google/gemini-cli/bundle/gemini.mjs' - ].join('\n') + await withProcessPlatform('linux', async () => { + mockExecFile((_command, args) => { + if (args[0] === '-axo') { + return { + stdout: [ + '100 99 Ss bash -l', + '101 100 S node /home/dev/.local/bin/node_modules/@google/gemini-cli/bundle/gemini.mjs' + ].join('\n') + } } - } - return new Error('unexpected command') - }) + return new Error('unexpected command') + }) - await expect(getForegroundProcessName(100, 'node')).resolves.toBe('gemini') + await expect(getForegroundProcessName(100, 'node')).resolves.toBe('gemini') + }) }) it('does not guess when SSH relay wrapper descendants are ambiguous', async () => { - mockExecFile((_command, args) => { - if (args[0] === '-axo') { - return { - stdout: [ - '100 99 Ss bash -l', - '101 100 S node /home/dev/project/server.js', - '102 100 S node /home/dev/.local/bin/node_modules/@openai/codex/bin/codex.js' - ].join('\n') + await withProcessPlatform('linux', async () => { + mockExecFile((_command, args) => { + if (args[0] === '-axo') { + return { + stdout: [ + '100 99 Ss bash -l', + '101 100 S node /home/dev/project/server.js', + '102 100 S node /home/dev/.local/bin/node_modules/@openai/codex/bin/codex.js' + ].join('\n') + } } - } - return new Error('unexpected command') - }) + return new Error('unexpected command') + }) - await expect(getForegroundProcessName(100, 'node')).resolves.toBe('node') + await expect(getForegroundProcessName(100, 'node')).resolves.toBe('node') + }) }) it('does not report a stopped SSH relay agent when another process has foreground', async () => { - mockExecFile((_command, args) => { - if (args[0] === '-axo') { - return { - stdout: [ - '100 99 Ss bash -l', - '101 100 T node /home/dev/.local/bin/codex', - '102 100 S+ vim notes.txt' - ].join('\n') + await withProcessPlatform('linux', async () => { + mockExecFile((_command, args) => { + if (args[0] === '-axo') { + return { + stdout: [ + '100 99 Ss bash -l', + '101 100 T node /home/dev/.local/bin/codex', + '102 100 S+ vim notes.txt' + ].join('\n') + } } - } - return new Error('unexpected command') - }) + return new Error('unexpected command') + }) - await expect(getForegroundProcessName(100, 'node')).resolves.toBe('node') + await expect(getForegroundProcessName(100, 'node')).resolves.toBe('node') + }) }) it('falls back to the root process command when descendant inspection fails', async () => { diff --git a/src/relay/pty-shell-utils.ts b/src/relay/pty-shell-utils.ts index 4a14742a7fd..af5ad0d3cc7 100644 --- a/src/relay/pty-shell-utils.ts +++ b/src/relay/pty-shell-utils.ts @@ -10,6 +10,10 @@ import { recognizeAgentProcessFromCommandLine } from '../shared/agent-process-recognition' import { isShellProcess } from '../shared/shell-process-detection' +import { + resolveWindowsAgentForegroundProcess, + shouldInspectWindowsAgentForeground +} from '../main/providers/windows-agent-foreground-process' const execFile = promisify(execFileCb) @@ -259,6 +263,14 @@ export async function getForegroundProcessName( if (fallbackRecognition) { return fallbackRecognition.processName } + if (process.platform === 'win32') { + if (!shouldInspectWindowsAgentForeground(fallbackProcess)) { + return fallbackProcess + } + return ( + (await resolveWindowsAgentForegroundProcess(pid, fallbackProcess, {})) ?? fallbackProcess + ) + } if (!isShellProcess(fallbackProcess) && !isAgentForegroundWrapperProcess(fallbackProcess)) { return fallbackProcess } diff --git a/src/renderer/src/assets/main.css b/src/renderer/src/assets/main.css index 5663c90012d..cfed3767a26 100644 --- a/src/renderer/src/assets/main.css +++ b/src/renderer/src/assets/main.css @@ -17,6 +17,12 @@ @custom-variant dark (&:is(.dark *)); +/* Why: hover-reveal controls (sidebar/row action buttons) hide with `opacity-0` + and fade in via `group-hover`. Touch devices have no hover, so those controls + stay invisible. Gate the hide behind `can-hover:` so touch (hover: none) keeps + them visible while pointer devices preserve the reveal-on-hover behavior. */ +@custom-variant can-hover (@media (hover: hover)); + .theme-transition-disabled *, .theme-transition-disabled *::before, .theme-transition-disabled *::after { diff --git a/src/renderer/src/components/activity/ActivityPrototypePage.tsx b/src/renderer/src/components/activity/ActivityPrototypePage.tsx index 5d9f2e6ea4b..5355913981a 100644 --- a/src/renderer/src/components/activity/ActivityPrototypePage.tsx +++ b/src/renderer/src/components/activity/ActivityPrototypePage.tsx @@ -1193,7 +1193,7 @@ function ThreadRow({ 'Mark thread unread' )} > - + @@ -1216,14 +1216,13 @@ function ThreadRow({ {/* Why (Jump-to-workspace lives on the secondary row): the bell slot on the title row already holds the unread/Mark-unread state, so the navigation action gets its own slot down here aligned with - the worktree name. Reserved layout via `invisible` + - `pointer-events-none` keeps the worktree-name's flex-1 width - stable across hover. */} + the worktree name. On hover-capable pointers, the hidden state + keeps the worktree-name's flex-1 width stable across hover. */} {canJump ? ( diff --git a/src/renderer/src/components/browser-pane/BrowserPane.tsx b/src/renderer/src/components/browser-pane/BrowserPane.tsx index abd7e1687d8..fded30fc515 100644 --- a/src/renderer/src/components/browser-pane/BrowserPane.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPane.tsx @@ -5232,7 +5232,7 @@ function BrowserPagePane({ - )} - {/* Why: timestamp and dismiss-X share a single slot so passive - rows show "time ago" and hovered rows swap in the X — no - reserved-space gap, no competing columns. Grid stacks both - children in cell 1,1 so the slot width is the larger of the - two (usually the timestamp, e.g. "just now" / "12m ago"), - which keeps the chevron's column stable whether or not the - row is hovered. Using opacity (not display:none) lets us - fade the crossfade instead of snapping, and keyboard focus - on the hidden X still activates it because `opacity-0` - doesn't remove it from the tab order. */} - {!sendTargetStatus && (startedAt !== null || doneAt !== null) && ( - - - {doneAt !== null - ? formatTimeAgo(doneAt, now) - : startedAt !== null - ? formatTimeAgo(startedAt, now) - : null} - - - - )} - {/* Why: when there is no timestamp yet (fresh agent, never - reported), the grid slot above does not render — show the X - as a standalone hover-only control so dismiss is still - reachable. Rare path; most rows have a timestamp the moment - they start. */} - {!sendTargetStatus && startedAt === null && doneAt === null && ( - - )} - {/* Why: chevron points down when collapsed (content below is - available) and rotates 180° to point up when expanded - (content is showing above the fold line). Single glyph - with a transform animates smoothly; swapping between two - glyphs (ChevronDown / ChevronUp) would snap because the - old node unmounts. Invisible placeholder keeps vertical - alignment stable across rows when nothing is expandable - so the row-trailing edge stays stable. */} - {!hideExpand && ( - - )} - + void + onToggleExpanded: () => void + onSendTargetClick?: (paneKey: string) => void +} + +export function DashboardAgentRowTrailingControls({ + paneKey, + relativeTimestamp, + expanded, + hideExpand, + sendTargetStatus, + onDismiss, + onToggleExpanded, + onSendTargetClick +}: DashboardAgentRowTrailingControlsProps): React.JSX.Element { + // Why: stop propagation so clicking nested row controls does not also + // activate the agent row or parent worktree card. + const stopMouseDown = useCallback((event: React.MouseEvent) => { + event.stopPropagation() + }, []) + const stopKeyDown = useCallback((event: React.KeyboardEvent) => { + if (event.key === 'Enter' || event.key === ' ') { + event.stopPropagation() + } + }, []) + const handleDismiss = useCallback( + (event: React.MouseEvent) => { + event.stopPropagation() + onDismiss(paneKey) + }, + [onDismiss, paneKey] + ) + const handleToggleExpand = useCallback( + (event: React.MouseEvent) => { + event.preventDefault() + event.stopPropagation() + onToggleExpanded() + }, + [onToggleExpanded] + ) + const handleInlineSendTargetClick = useCallback( + (event: React.MouseEvent) => { + event.preventDefault() + event.stopPropagation() + if (sendTargetStatus === 'eligible') { + onSendTargetClick?.(paneKey) + } + }, + [onSendTargetClick, paneKey, sendTargetStatus] + ) + + return ( + + {(sendTargetStatus === 'eligible' || sendTargetStatus === 'sending') && ( + + )} + {/* Why: timestamp and dismiss-X share one slot. On no-hover devices the X + is visible by default, so the timestamp must yield there too. */} + {!sendTargetStatus && relativeTimestamp !== null && ( + + + {relativeTimestamp} + + + + )} + {!sendTargetStatus && relativeTimestamp === null && ( + + )} + {!hideExpand && ( + + )} + + ) +} diff --git a/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx b/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx index db8dbcade8d..754837a6707 100644 --- a/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx +++ b/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx @@ -1,10 +1,13 @@ import React from 'react' import { ExternalLink, LoaderCircle, RefreshCw } from 'lucide-react' +import { toast } from 'sonner' import { Button } from '@/components/ui/button' import CommentMarkdown from '@/components/sidebar/CommentMarkdown' import type { PRCheckDetail, PRCheckRunDetails } from '../../../../shared/types' import { CheckJobLogTail } from '@/components/right-sidebar/check-job-log-tail' +import { SourceControlFixSplitButton } from '@/components/right-sidebar/source-control-fix-split-button' import { translate } from '@/i18n/i18n' +import { useCheckRunDetailsFixWithAI } from './check-run-details-fix-with-ai' function formatCheckTimestamp(value: string | null | undefined): string | null { if (!value) { @@ -52,6 +55,7 @@ export function CheckRunDetailsPanel({ loading, error, openUrl, + worktreeId, onRefresh }: { check: PRCheckDetail @@ -59,8 +63,28 @@ export function CheckRunDetailsPanel({ loading: boolean error: string | null openUrl: string | null | undefined + worktreeId: string | null onRefresh?: () => void }): React.JSX.Element { + const { + canFixWithAI, + disabledReason, + isFixing, + fixPrompt, + repoId, + connectionId, + launchPlatform, + savedAgentId, + savedCommandInputTemplate, + savedAgentArgs, + saveLaunchActionDefault, + openSourceControlAiSettings, + fixWithAI + } = useCheckRunDetailsFixWithAI({ + worktreeId, + check, + details + }) const startedAt = formatCheckTimestamp(details?.startedAt) const completedAt = formatCheckTimestamp(details?.completedAt) const detailsStatusCheck: PRCheckDetail = { @@ -85,19 +109,86 @@ export function CheckRunDetailsPanel({

{check.name}

- {onRefresh && ( - - )} +
+ {canFixWithAI && ( + + toast.success( + translate( + 'auto.components.editor.check.run.details.fix.with.ai.2ef90c9819', + 'Started an AI agent for this check.' + ) + ) + } + /> + )} + {onRefresh && ( + + )} +
diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index ebd4f46b646..d28540badd6 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -1617,7 +1617,7 @@ export default function CombinedDiffViewer({ comments={diffCommentsForWorktree} filePath={section.path} showFileScope - triggerClassName="p-0.5 opacity-0 group-hover:opacity-100" + triggerClassName="p-0.5 can-hover:opacity-0 group-hover:opacity-100" /> ) : null }} diff --git a/src/renderer/src/components/editor/DiffSectionHeader.tsx b/src/renderer/src/components/editor/DiffSectionHeader.tsx index c69600a8b24..8df224e7c9d 100644 --- a/src/renderer/src/components/editor/DiffSectionHeader.tsx +++ b/src/renderer/src/components/editor/DiffSectionHeader.tsx @@ -72,7 +72,7 @@ export function DiffSectionHeader({
{trailingContent}
) })} -
+
{row.content.url ? ( diff --git a/src/renderer/src/components/hover-reveal-touch-action-visibility.test.ts b/src/renderer/src/components/hover-reveal-touch-action-visibility.test.ts new file mode 100644 index 00000000000..c8398ccf308 --- /dev/null +++ b/src/renderer/src/components/hover-reveal-touch-action-visibility.test.ts @@ -0,0 +1,87 @@ +import { readFileSync } from 'fs' +import { resolve } from 'path' +import { describe, expect, it } from 'vitest' + +// Why: hover-reveal action buttons hide with `opacity-0` and reveal on +// `group-hover`. Touch devices have no hover, so a bare hide leaves the icon +// permanently invisible while the control stays clickable. + +const MAIN_CSS = resolve(__dirname, '../assets/main.css') + +const HOVER_REVEAL_FILES = [ + resolve(__dirname, 'activity/ActivityPrototypePage.tsx'), + resolve(__dirname, 'browser-pane/BrowserPane.tsx'), + resolve(__dirname, 'dashboard/DashboardAgentRow.tsx'), + resolve(__dirname, 'dashboard/DashboardAgentRowTrailingControls.tsx'), + resolve(__dirname, 'editor/CombinedDiffViewer.tsx'), + resolve(__dirname, 'editor/DiffSectionHeader.tsx'), + resolve(__dirname, 'github-project/ProjectPicker.tsx'), + resolve(__dirname, 'github-project/ProjectRow.tsx'), + resolve(__dirname, 'right-sidebar/AiVaultSessionRow.tsx'), + resolve(__dirname, 'right-sidebar/ChecksPanel.tsx'), + resolve(__dirname, 'right-sidebar/PortsPanel.tsx'), + resolve(__dirname, 'right-sidebar/SourceControl.tsx'), + resolve(__dirname, 'right-sidebar/checks-panel-content.tsx'), + resolve(__dirname, 'settings/MobilePairingQrSection.tsx'), + resolve(__dirname, 'settings/ShortcutBindingRow.tsx'), + resolve(__dirname, 'settings/VoiceSpeechModelSection.tsx'), + resolve(__dirname, 'sidebar/HostSectionHeaderMenu.tsx'), + resolve(__dirname, 'sidebar/PendingWorktreeRow.tsx'), + resolve(__dirname, 'sidebar/WorkspaceKanbanStatusLane.tsx'), + resolve(__dirname, 'sidebar/WorktreeCardPorts.tsx'), + resolve(__dirname, 'sidebar/WorktreeList.tsx'), + resolve(__dirname, 'status-bar/ResourceUsageStatusSegment.tsx'), + resolve(__dirname, 'status-bar/ports-status-popover-rows.tsx'), + resolve(__dirname, 'tab-bar/TabBarQuickCommandsMenu.tsx') +] + +function lineHasBareOpacityZero(line: string): boolean { + for (const match of line.matchAll(/opacity-0\b/g)) { + const previousChar = match.index! > 0 ? line[match.index! - 1] : '' + if (previousChar !== ':') { + return true + } + } + + return false +} + +describe('hover-reveal action button touch visibility', () => { + it('declares the can-hover variant so touch devices skip the hover-only hide', () => { + const css = readFileSync(MAIN_CSS, 'utf8') + expect(css).toMatch(/@custom-variant\s+can-hover\s+\(@media\s*\(hover:\s*hover\)\)/) + }) + + it('keeps hover-reveal controls visible on touch devices', () => { + const offenders: string[] = [] + + HOVER_REVEAL_FILES.forEach((file) => { + const source = readFileSync(file, 'utf8') + const lines = source.split('\n') + + lines.forEach((line, index) => { + if (!line.includes('opacity-0') || !lineHasBareOpacityZero(line)) { + return + } + + const revealContext = [line, lines[index + 1] ?? '', lines[index + 2] ?? ''].join(' ') + const isHoverReveal = + /(group-hover|group-focus-within|focus-visible|focus:|data-\[state=open\]|data-\[selected=true\]):opacity-100/.test( + revealContext + ) + const hasTouchOverride = revealContext.includes('[@media(hover:none)]:opacity-100') + const isPassiveSwap = + line.includes('[@media(hover:none)]:opacity-0') && line.includes('pointer-events-none') + const isPointerlessPlaceholder = + line.includes('pointer-events-none') && + !/(group-hover|group-focus-within)/.test(revealContext) + + if (isHoverReveal && !hasTouchOverride && !isPassiveSwap && !isPointerlessPlaceholder) { + offenders.push(`${file}:${index + 1}: ${line.trim()}`) + } + }) + }) + + expect(offenders).toEqual([]) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx b/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx index 751afcf20b8..85e68bddea1 100644 --- a/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx +++ b/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx @@ -127,7 +127,12 @@ export function VaultSessionRow({ event.stopPropagation() onResume() }} - className="opacity-0 transition-opacity group-hover:pointer-events-auto group-hover:opacity-100 group-focus-within:pointer-events-auto group-focus-within:opacity-100" + // Why: the wrapper is `pointer-events-none`; this control only + // re-enables pointer events on hover/focus. On touch (no hover) + // it is visible via `can-hover:opacity-0`, so it must also be + // tappable — keep base `pointer-events-auto` and only disable it + // on hover-capable devices where the reveal gates interaction. + className="pointer-events-auto can-hover:pointer-events-none can-hover:opacity-0 transition-opacity group-hover:pointer-events-auto group-hover:opacity-100 group-focus-within:pointer-events-auto group-focus-within:opacity-100" > diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 77a42e7c5d9..9e8ef50b6ed 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -70,6 +70,7 @@ import type { HostedReviewCreationEligibility, HostedReviewProvider } from '../../../../shared/hosted-review' +import { resolveHostedReviewCreationProvider } from '../../../../shared/hosted-review-creation-providers' import { normalizeHostedReviewHeadRef } from '../../../../shared/hosted-review-refs' import { getHostedReviewCacheKey, refreshHostedReviewCard } from '@/store/slices/hosted-review' import { toast } from 'sonner' @@ -694,8 +695,9 @@ export default function ChecksPanel(): React.JSX.Element { hostedReviewCreationSnapshot?.requestKey === hostedReviewCreationRequestKey ? hostedReviewCreationSnapshot.data : null - const hostedReviewCreateProvider: HostedReviewProvider = - hostedReviewCreation?.provider === 'gitlab' ? 'gitlab' : 'github' + const hostedReviewCreateProvider = resolveHostedReviewCreationProvider( + hostedReviewCreation?.provider + ) const hostedReviewCreateCopy = localizedHostedReviewCopy(hostedReviewCreateProvider) const handleBranchChangedByPullRequestGeneration = useCallback(async (): Promise => { if (!activeWorktreeId || !activeWorktree?.path) { @@ -2669,17 +2671,27 @@ export default function ChecksPanel(): React.JSX.Element { if (activeWorktreeId && result.provider === 'gitlab') { await updateWorktreeMeta(activeWorktreeId, { linkedGitLabMR: result.number }) } + if (activeWorktreeId && result.provider === 'azure-devops') { + await updateWorktreeMeta(activeWorktreeId, { linkedAzureDevOpsPR: result.number }) + } + if (activeWorktreeId && result.provider === 'gitea') { + await updateWorktreeMeta(activeWorktreeId, { linkedGiteaPR: result.number }) + } + const linkedReviewNumbers = { + linkedGitHubPR: result.provider === 'github' ? result.number : linkedPR, + fallbackGitHubPR: fallbackGitHubPRNumber, + linkedGitLabMR: result.provider === 'gitlab' ? result.number : linkedGitLabMR, + linkedBitbucketPR, + linkedAzureDevOpsPR: + result.provider === 'azure-devops' ? result.number : linkedAzureDevOpsPR, + linkedGiteaPR: result.provider === 'gitea' ? result.number : linkedGiteaPR + } if (result.provider === 'gitlab') { const refreshedReview = await refreshHostedReviewCard(fetchHostedReviewForBranch, { repoPath: repo.path, repoId: repo.id, branch, - linkedGitHubPR: linkedPR, - fallbackGitHubPR: fallbackGitHubPRNumber, - linkedGitLabMR: result.number, - linkedBitbucketPR, - linkedAzureDevOpsPR, - linkedGiteaPR + ...linkedReviewNumbers }) const refreshedGitLabReview = refreshedReview?.provider === 'gitlab' ? refreshedReview : null @@ -2690,6 +2702,15 @@ export default function ChecksPanel(): React.JSX.Element { }) return } + if (result.provider !== 'github') { + await refreshHostedReviewCard(fetchHostedReviewForBranch, { + repoPath: repo.path, + repoId: repo.id, + branch, + ...linkedReviewNumbers + }) + return + } await refreshLinkedGitHubPullRequest(result.number) } catch { // The success toast keeps the hosted URL available; Checks can be refreshed manually. @@ -2703,6 +2724,7 @@ export default function ChecksPanel(): React.JSX.Element { linkedAzureDevOpsPR, linkedBitbucketPR, linkedGiteaPR, + linkedGitLabMR, linkedPR, refreshLinkedGitHubPullRequest, repo, @@ -3103,7 +3125,7 @@ export default function ChecksPanel(): React.JSX.Element { {activeReview.title} - +
)} diff --git a/src/renderer/src/components/right-sidebar/CommitArea.test.tsx b/src/renderer/src/components/right-sidebar/CommitArea.test.tsx index 32317c593ee..7aeaf80bef8 100644 --- a/src/renderer/src/components/right-sidebar/CommitArea.test.tsx +++ b/src/renderer/src/components/right-sidebar/CommitArea.test.tsx @@ -1,7 +1,10 @@ import { describe, expect, it, vi } from 'vitest' import { renderToStaticMarkup } from 'react-dom/server' import { CommitArea, ConflictSummaryCard, OperationBanner } from './SourceControl' -import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action' +import { + resolveCommitAreaPrimaryAction, + type PrimaryActionInputs +} from './source-control-primary-action' import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items' import { TooltipProvider } from '@/components/ui/tooltip' @@ -39,7 +42,7 @@ function baseProps(overrides: Partial = {}) { hasUnresolvedConflicts: inputs.hasUnresolvedConflicts, isRemoteOperationActive: inputs.isRemoteOperationActive, inFlightRemoteOpKind: inputs.inFlightRemoteOpKind ?? null, - primaryAction: resolvePrimaryAction(inputs), + primaryAction: resolveCommitAreaPrimaryAction(inputs), dropdownItems: resolveDropdownItems(inputs), onCommitMessageChange: vi.fn(), onGenerate: vi.fn(), @@ -50,7 +53,7 @@ function baseProps(overrides: Partial = {}) { } } -function renderCommitArea(props: ReturnType): string { +function renderCommitArea(props: Parameters[0]): string { return renderToStaticMarkup( @@ -327,6 +330,96 @@ describe('CommitArea', () => { expect(button).toContain('animate-spin') expect(button).not.toContain('lucide-check') }) + + it('keeps Stage All as the commit-area primary when review prep can stage changes', () => { + const input = buildInputs({ + stagedCount: 0, + hasUnstagedChanges: true, + hasStageableChanges: true, + hasPartiallyStagedChanges: false, + hasMessage: false, + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit' + } + }) + const markup = renderCommitArea(baseProps(input)) + + const stageAllButton = firstButton(markup) + expect(stageAllButton).toContain('Stage All') + expect(stageAllButton).not.toContain('disabled=""') + expect(stageAllButton).toContain('lucide-plus') + expect(stageAllButton).toContain('rounded-r-none') + expect(markup).toContain('aria-label="More commit and remote actions"') + expect(markup).toContain('Stage all changes') + expect( + (markup.match(//g) ?? []).some((button) => + button.includes('Commit') + ) + ).toBe(false) + }) + + it('keeps Push as the commit-area primary when review prep can create after pushing', () => { + const input = buildInputs({ + stagedCount: 0, + hasUnstagedChanges: false, + hasStageableChanges: false, + hasPartiallyStagedChanges: false, + hasMessage: false, + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push' + } + }) + const markup = renderCommitArea(baseProps(input)) + + const pushButton = firstButton(markup) + expect(pushButton).toContain('Push') + expect(pushButton).not.toContain('disabled=""') + expect(pushButton).toContain('lucide-arrow-up') + expect(pushButton).toContain('rounded-r-none') + expect(markup).toContain('aria-label="More commit and remote actions"') + }) + + it('hides the composer generate affordance while Create PR intent is in flight', () => { + const markup = renderCommitArea({ + ...baseProps(), + aiEnabled: true, + aiAgentConfigured: true, + isGenerating: true, + isCreatePrIntentInFlight: true, + createPrIntentNotice: { + tone: 'muted', + message: 'Generating commit message…' + } + }) + + expect(markup).not.toContain('lucide-sparkles') + expect(markup).not.toContain('animate-spin') + expect(markup).toContain('Generating commit message…') + }) + + it('renders Create PR failures in the visible inline notice', () => { + const markup = renderCommitArea({ + ...baseProps(), + createPrIntentNotice: { + tone: 'destructive', + message: 'Create PR failed: push this branch first.' + } + }) + + expect(markup).toContain('id="commit-area-create-pr-intent"') + expect(markup).toContain('role="alert"') + expect(markup).toContain('Create PR failed: push this branch first.') + }) }) describe('ConflictSummaryCard', () => { diff --git a/src/renderer/src/components/right-sidebar/CreatePullRequestDialog.tsx b/src/renderer/src/components/right-sidebar/CreatePullRequestDialog.tsx index a1e78944061..1b6114ae7f6 100644 --- a/src/renderer/src/components/right-sidebar/CreatePullRequestDialog.tsx +++ b/src/renderer/src/components/right-sidebar/CreatePullRequestDialog.tsx @@ -15,6 +15,7 @@ import type { HostedReviewCreationEligibility, HostedReviewProvider } from '../../../../shared/hosted-review' +import { resolveHostedReviewCreationProvider } from '../../../../shared/hosted-review-creation-providers' import { normalizeHostedReviewHeadRef } from '../../../../shared/hosted-review-refs' import { stripBaseRef, useCreatePullRequestDialogFields } from './useCreatePullRequestDialogFields' import { @@ -68,7 +69,7 @@ export function CreatePullRequestDialog({ const submitInFlightRef = useRef(false) const [submitting, setSubmitting] = useState(false) const [error, setError] = useState(null) - const provider = eligibility?.provider === 'gitlab' ? 'gitlab' : 'github' + const provider = resolveHostedReviewCreationProvider(eligibility?.provider) const copy = reviewCopy(provider) const prCreationDefaults = React.useMemo(() => { if (!settings) { diff --git a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx index bc9966b7e87..13796a63b79 100644 --- a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx +++ b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx @@ -114,6 +114,8 @@ export function FolderWorkspacePrChecksRow({ checksLoading={row.isRefreshing} checkDetailsContextKey={row.refreshIdentity} onLoadCheckDetails={onLoadCheckDetails} + worktreeId={row.worktree.id} + detailsStickySurface="card" />
) : null} diff --git a/src/renderer/src/components/right-sidebar/PortsPanel.tsx b/src/renderer/src/components/right-sidebar/PortsPanel.tsx index 1226e4e6e13..13881765cdc 100644 --- a/src/renderer/src/components/right-sidebar/PortsPanel.tsx +++ b/src/renderer/src/components/right-sidebar/PortsPanel.tsx @@ -594,7 +594,7 @@ function LocalPortRow({
-
+
-
+
))} - {hostedReview && ( + {(visibleCreatePrHeaderAction || hostedReview) && (
- - + {visibleCreatePrHeaderAction && ( + + + + + + + + {visibleCreatePrHeaderAction.title} + + + )} + {hostedReview && ( + <> + + + + )}
)}
@@ -4071,7 +5082,7 @@ function SourceControlInner(): React.JSX.Element { ) : null} {shouldRenderCommitArea(scope, unresolvedConflicts.length, conflictOperation) && - (primaryAction.kind === 'create_pr' ? ( + (directCreatePrAction ? ( { + void handleCreatePullRequest() + }} onDropdownAction={handleActionInvoke} /> ) : ( @@ -4112,8 +5127,11 @@ function SourceControlInner(): React.JSX.Element { commitError={commitError} commitFailureRecoveryPrompt={commitFailureRecoveryPrompt} remoteActionError={remoteActionError?.message ?? null} + createPrIntentNotice={createPrIntentNotice} isCommitting={isCommitting} isFixingCommitFailureWithAI={isLaunchingCommitFailureAgent} + isCreatingPr={isCreatingPr || isCreatePrIntentInFlight} + isCreatePrIntentInFlight={isCreatePrIntentInFlight} groupId={activeGroupId ?? activeWorktreeId} showComposer={!(scope === 'all' && showGenericEmptyState)} aiEnabled={resolvedCommitMessageAi?.ok === true} @@ -4131,7 +5149,7 @@ function SourceControlInner(): React.JSX.Element { if (!activeWorktreeId) { return } - setCommitDrafts((prev) => + updateCommitDrafts((prev) => writeCommitDraftForWorktree(prev, activeWorktreeId, value) ) }} @@ -4196,7 +5214,7 @@ function SourceControlInner(): React.JSX.Element { focusing any action reveals all three siblings — otherwise keyboard users tab into an invisible next stop. */} -
+
{canRevertAll && ( { if (baseRefOwnedByWorktree && activeWorktreeId) { void updateWorktreeMeta(activeWorktreeId, { baseRef: ref }) @@ -4829,9 +5847,11 @@ type CommitAreaProps = { commitError: string | null commitFailureRecoveryPrompt: string | null remoteActionError: string | null + createPrIntentNotice?: CreatePrIntentNotice | null isCommitting: boolean isFixingCommitFailureWithAI: boolean isCreatingPr?: boolean + isCreatePrIntentInFlight?: boolean showComposer?: boolean aiEnabled: boolean aiAgentConfigured: boolean @@ -4868,9 +5888,11 @@ export function CommitArea({ commitError, commitFailureRecoveryPrompt, remoteActionError, + createPrIntentNotice, isCommitting, isFixingCommitFailureWithAI, isCreatingPr = false, + isCreatePrIntentInFlight = false, showComposer = true, aiEnabled, aiAgentConfigured, @@ -4897,9 +5919,9 @@ export function CommitArea({ // the existing style) — the browser scrolls internally past 12 rows. const rows = Math.min(12, Math.max(2, commitMessage.split('\n').length)) // Why: only spin the primary when its label matches what's actually - // running. resolvePrimaryAction overrides the primary kind to mirror the - // in-flight op (e.g. user picks Sync from the dropdown → primary becomes - // "Sync"), so the equality check spins the button for any primary- + // running. The commit-area resolver overrides the primary kind to mirror + // the in-flight op (e.g. user picks Sync from the dropdown → primary + // becomes "Sync"), so the equality check spins the button for any primary- // eligible remote op the user triggered. Background ops the primary // doesn't show (Fetch) leave primaryAction.kind unchanged and the // mismatch keeps the spinner off — the disabled state alone is enough @@ -4909,7 +5931,7 @@ export function CommitArea({ primaryAction.kind === inFlightRemoteOpKind || (primaryAction.kind === 'push' && inFlightRemoteOpKind === 'force_push') const showSpinner = - primaryAction.kind === 'create_pr' + primaryAction.kind === 'create_pr' || primaryAction.kind === 'create_pr_intent' ? isCreatingPr : primaryAction.kind === 'commit' ? isCommitting @@ -4989,6 +6011,7 @@ export function CommitArea({ const describedBy = [ commitError ? 'commit-area-error' : null, remoteActionError ? 'commit-area-remote-error' : null, + createPrIntentNotice ? 'commit-area-create-pr-intent' : null, generateError ? 'commit-area-generate-error' : null ] .filter(Boolean) @@ -4996,7 +6019,10 @@ export function CommitArea({ // Why: only render Generate when it has a runnable path; otherwise the // composer should stay focused on the normal Commit action. - const showGenerate = showComposer && aiEnabled && (aiAgentConfigured || isGenerating) + // Why: Create PR intent owns message generation and surfaces status via the + // inline notice; a second composer spinner stacks on the primary spinner. + const showGenerate = + showComposer && aiEnabled && !isCreatePrIntentInFlight && (aiAgentConfigured || isGenerating) let generateDisabledReason: string | undefined if (isGenerating) { generateDisabledReason = 'Generating commit message…' @@ -5016,6 +6042,55 @@ export function CommitArea({ stagedCount === 0 || hasMessage || hasUnresolvedConflicts + const moreCommitAndRemoteActionsLabel = translate( + 'auto.components.right.sidebar.SourceControl.cc199ccc5f', + 'More commit and remote actions' + ) + const moreActionsLabel = translate( + 'auto.components.right.sidebar.SourceControl.4d6e1fd7f3', + 'More actions' + ) + const dropdownMenuContent = ( + + {dropdownItems.map((entry, index) => + entry.kind === 'separator' ? ( + + ) : ( + + +
+ { + if (entry.disabled) { + event.preventDefault() + return + } + onDropdownAction(entry.kind) + }} + > + + {entry.label} + {entry.hint ? ( + + {entry.hint} + + ) : null} + + +
+ + + {entry.title} + + + ) + )} + + ) return (
@@ -5096,124 +6171,77 @@ export function CommitArea({ ))}
) : null} - {/* Why: primary + chevron sit together as a visual split button so the - edit → commit → push loop stays in a single vertical band. The - chevron exposes the full action surface (fetch, pull, sync, - publish, compound commits) without forcing morphing labels to - carry every possible intent. */} -
- {/* Why: match the hosted-review action buttons in Checks - (size="xs", px-3 text-[11px]) so the sidebar has a consistent - action-button shape across Source Control and Checks. The primary - and chevron share a single rounded rectangle — rounded-r-none on - the primary and rounded-l-none + border-l on the chevron make the - pair read as one split button instead of two detached buttons. */} - - - - - - - - {primaryAction.title} - - - + {/* Why: the current manual action + chevron sit together as a visual + split button so the edit → commit → push loop stays in a single + vertical band. The chevron exposes the full action surface without + forcing morphing labels to carry every possible intent. */} +
+
+ {/* Why: match the hosted-review action buttons in Checks + (size="xs", px-3 text-[11px]) so the sidebar has a consistent + action-button shape across Source Control and Checks. */} - - - - + + - - {translate( - 'auto.components.right.sidebar.SourceControl.cc199ccc5f', - 'More commit and remote actions' - )} + + {primaryAction.title} - - {dropdownItems.map((entry, index) => - entry.kind === 'separator' ? ( - - ) : ( - - -
- { - if (entry.disabled) { - event.preventDefault() - return - } - onDropdownAction(entry.kind) - }} - > - - {entry.label} - {entry.hint ? ( - - {entry.hint} - - ) : null} - - -
- - - {entry.title} - - - ) - )} - - + + + + + + + + + + + {moreCommitAndRemoteActionsLabel} + + + {dropdownMenuContent} + +
{commitError && ( // Why: role="alert" + aria-live="polite" lets screen readers announce @@ -5353,6 +6381,33 @@ export function CommitArea({ {remoteActionError}

)} + {createPrIntentNotice && ( +
+ {createPrIntentNotice.message} + {createPrIntentNotice.action === 'settings' && onOpenSourceControlAiSettings ? ( + + ) : null} +
+ )} {generateError && (

+ ) +} + function CheckRunDetails({ check, state, - checkDetailsContextKey + checkDetailsContextKey, + worktreeId, + detailsStickySurface = 'sidebar' }: { check: PRCheckDetail state: CheckDetailsLoadState | undefined checkDetailsContextKey: string + worktreeId: string | null + detailsStickySurface?: CheckDetailsStickySurface }): React.JSX.Element { - const activeWorktree = useActiveWorktree() const openCheckRunDetails = useAppStore((s) => s.openCheckRunDetails) const details = state?.details const startedAt = formatCheckTimestamp(details?.startedAt) @@ -499,19 +530,49 @@ function CheckRunDetails({ const hasJobs = jobs.length > 0 const hasLogTail = jobs.some((job) => Boolean(job.logTail)) + // Why: wait until inline details finish loading before switching to the logs label + // so the sticky button does not resize mid-fetch. + const fullDetailsLabel = + !state?.loading && hasLogTail + ? translate('auto.components.right.sidebar.checks.panel.content.b8c4e2a1f7', 'View full logs') + : translate( + 'auto.components.right.sidebar.checks.panel.content.e4e3af15ee', + 'View full details' + ) + const openFullDetailsTab = (): void => { - if (!activeWorktree) { + if (!worktreeId) { return } - openCheckRunDetails(activeWorktree.id, checkDetailsContextKey, check, { + openCheckRunDetails(worktreeId, checkDetailsContextKey, check, { details: state?.details ?? null, loading: state?.loading ?? false, error: state?.error ?? null }) } + const handleOpenFullDetails = (event: React.MouseEvent): void => { + event.stopPropagation() + openFullDetailsTab() + } + return (

+ {worktreeId && ( + // Why: inline check details can be long; pinning the affordance keeps it + // visible while scrolling through annotations and job output. +
+ + {check.name} + + +
+ )} {state?.loading ? (
@@ -521,25 +582,6 @@ function CheckRunDetails({ 'Loading check details…' )}
- {activeWorktree && ( -
- -
- )}
) : (
@@ -744,26 +786,6 @@ function CheckRunDetails({ )}
)} - -
- {activeWorktree && ( - - )} -
)}
@@ -777,14 +799,20 @@ export function ChecksList({ checks, checksLoading, checkDetailsContextKey, - onLoadCheckDetails + onLoadCheckDetails, + worktreeId: worktreeIdOverride, + detailsStickySurface = 'sidebar' }: { checks: PRCheckDetail[] checksLoading: boolean checkDetailsContextKey: string onLoadCheckDetails?: (check: PRCheckDetail) => Promise + /** Why: folder-workspace PR checks render rows for attached worktrees, not the active one. */ + worktreeId?: string + detailsStickySurface?: CheckDetailsStickySurface }): React.JSX.Element { const activeWorktree = useActiveWorktree() + const resolvedWorktreeId = worktreeIdOverride ?? activeWorktree?.id ?? null const patchOpenCheckRunDetails = useAppStore((s) => s.patchOpenCheckRunDetails) const [checksExpanded, setChecksExpanded] = useState(true) const [expandedCheckKeys, setExpandedCheckKeys] = useState>(new Set()) @@ -952,7 +980,7 @@ export function ChecksList({ }, [checksExpanded, detailsByCheckKey, expandedCheckKeys, requestCheckDetails, rows]) useEffect(() => { - if (!activeWorktree) { + if (!resolvedWorktreeId) { return } for (const row of rows) { @@ -960,13 +988,19 @@ export function ChecksList({ if (!detailsState) { continue } - patchOpenCheckRunDetails(activeWorktree.id, checkDetailsContextKey, row.check, { + patchOpenCheckRunDetails(resolvedWorktreeId, checkDetailsContextKey, row.check, { details: detailsState.details ?? null, loading: detailsState.loading ?? false, error: detailsState.error ?? null }) } - }, [activeWorktree, checkDetailsContextKey, detailsByCheckKey, patchOpenCheckRunDetails, rows]) + }, [ + checkDetailsContextKey, + detailsByCheckKey, + patchOpenCheckRunDetails, + resolvedWorktreeId, + rows + ]) const toggleCheckExpanded = useCallback( (row: { check: PRCheckDetail; key: string }) => { @@ -1124,6 +1158,8 @@ export function ChecksList({ check={check} state={detailsByCheckKey[row.key]} checkDetailsContextKey={checkDetailsContextKey} + worktreeId={resolvedWorktreeId} + detailsStickySurface={detailsStickySurface} /> )}
@@ -1501,7 +1537,7 @@ function CommentRow({
{!editing && resolveSelectionAction} {!editing && ( -
+
{showResolve && comment.threadId != null && onResolve && ( | null + worktreeId: string + worktreePath: string + connectionId?: string + pushTarget?: GitPushTarget + deps: Omit & { + fetchUpstreamStatus?: GitStatusRefreshDeps['fetchUpstreamStatus'] + } +}): Promise<{ status: GitStatusResult; upstreamStatus: GitUpstreamStatus }> { + const status = (await getRuntimeGitStatus({ + settings, + worktreeId, + worktreePath, + connectionId + })) as GitStatusResult + + deps.setGitStatus(worktreeId, status) + // Why: branch switches can happen inside a terminal. `git status --branch` + // gives us the new identity without a separate worktree-list poll. + deps.updateWorktreeGitIdentity(worktreeId, { + head: status.head, + // Why: detached HEAD reports a head oid and no branch. Pass null as an + // explicit clear signal so stale branch names don't linger in the UI. + branch: status.branch ?? (status.head ? null : undefined) + }) + if (pushTarget) { + // Why: porcelain status reports Git's configured upstream. Source Control + // actions for PR-created worktrees must instead reconcile with Orca's + // explicit publish target. + const upstreamStatus = await getRuntimeGitUpstreamStatus( + { settings, worktreeId, worktreePath, connectionId }, + pushTarget + ) + deps.setUpstreamStatus(worktreeId, upstreamStatus) + return { status, upstreamStatus } + } + if (status.upstreamStatus) { + if ( + status.upstreamStatus.ahead > 0 && + status.upstreamStatus.behind > 0 && + status.upstreamStatus.behindCommitsArePatchEquivalent === undefined + ) { + // Why: porcelain status has counts but cannot tell stale post-rebase + // upstream commits from real remote work. Writing it first makes the + // primary action flicker between Sync and Force Push on every poll. + const upstreamStatus = await getRuntimeGitUpstreamStatus( + { settings, worktreeId, worktreePath, connectionId }, + undefined + ) + deps.setUpstreamStatus(worktreeId, upstreamStatus) + return { status, upstreamStatus } + } + deps.setUpstreamStatus(worktreeId, status.upstreamStatus) + return { status, upstreamStatus: status.upstreamStatus } + } + const upstreamStatus = await getRuntimeGitUpstreamStatus( + { settings, worktreeId, worktreePath, connectionId }, + undefined + ) + deps.setUpstreamStatus(worktreeId, upstreamStatus) + return { status, upstreamStatus } +} diff --git a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.test.ts b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.test.ts new file mode 100644 index 00000000000..f04530b8511 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, it, vi } from 'vitest' +import { + createCreatePrIntentRunToken, + createPrIntentGitStatusMatchesToken, + createPrIntentRunTokenMatches, + getCreatePrIntentStagePaths, + resolveCreatePrIntentRemoteStep +} from './source-control-create-pr-intent-flow' +import type { GitStatusEntry } from '../../../../shared/types' + +describe('source-control Create PR intent flow helpers', () => { + it('matches async completions only to the original repo, worktree, path, and branch', () => { + const now = vi.spyOn(Date, 'now').mockReturnValue(123) + try { + const token = createCreatePrIntentRunToken({ + repoId: 'repo-1', + worktreeId: 'wt-1', + worktreePath: '/repo', + branch: 'feature' + }) + + expect(token.startedAt).toBe(123) + expect(createPrIntentRunTokenMatches(token, token)).toBe(true) + expect(createPrIntentRunTokenMatches(token, { ...token, branch: 'other' })).toBe(false) + expect(createPrIntentRunTokenMatches(token, { ...token, worktreeId: 'wt-2' })).toBe(false) + } finally { + now.mockRestore() + } + }) + + it('matches strict git status snapshots to the original branch', () => { + const token = createCreatePrIntentRunToken({ + repoId: 'repo-1', + worktreeId: 'wt-1', + worktreePath: '/repo', + branch: 'feature/pr' + }) + + expect(createPrIntentGitStatusMatchesToken(token, { branch: 'refs/heads/feature/pr' })).toBe( + true + ) + expect(createPrIntentGitStatusMatchesToken(token, { branch: 'feature/pr' })).toBe(true) + expect(createPrIntentGitStatusMatchesToken(token, { branch: 'refs/heads/other' })).toBe(false) + expect(createPrIntentGitStatusMatchesToken(token, { branch: null })).toBe(false) + }) + + it('stages only safe unstaged and untracked paths', () => { + const unresolved = { + path: 'conflicted.ts', + status: 'modified', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + } satisfies GitStatusEntry + + expect( + getCreatePrIntentStagePaths({ + unstaged: [{ path: 'safe.ts', status: 'modified', area: 'unstaged' }, unresolved], + untracked: [{ path: 'new.ts', status: 'untracked', area: 'untracked' }] + }) + ).toEqual(['safe.ts', 'new.ts']) + }) + + it('resolves safe remote steps for publish, push, and patch-equivalent force-push', () => { + expect( + resolveCreatePrIntentRemoteStep({ + upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }, + branchCommitsAhead: 2, + hasCurrentBranch: true, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'no_upstream', + nextAction: 'publish' + } + }) + ).toBe('publish') + + expect( + resolveCreatePrIntentRemoteStep({ + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hasCurrentBranch: true, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push' + } + }) + ).toBe('push') + + expect( + resolveCreatePrIntentRemoteStep({ + upstreamStatus: { + hasUpstream: true, + ahead: 3, + behind: 2, + behindCommitsArePatchEquivalent: true + }, + branchCommitsAhead: 3, + hasCurrentBranch: true, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_sync', + nextAction: 'sync' + } + }) + ).toBe('force_push') + }) + + it('blocks ordinary diverged branches and unpublished branches without commits', () => { + expect( + resolveCreatePrIntentRemoteStep({ + upstreamStatus: { hasUpstream: true, ahead: 1, behind: 1 }, + hasCurrentBranch: true, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_sync', + nextAction: 'sync' + } + }) + ).toBe('blocked') + + expect( + resolveCreatePrIntentRemoteStep({ + upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }, + branchCommitsAhead: 0, + hasCurrentBranch: true, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'no_upstream', + nextAction: 'publish' + } + }) + ).toBe('blocked') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.ts b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.ts new file mode 100644 index 00000000000..9b6e54bc201 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.ts @@ -0,0 +1,93 @@ +import { shouldForcePushWithLeaseForUpstream } from '../../../../shared/git-upstream-status' +import type { HostedReviewCreationEligibility } from '../../../../shared/hosted-review' +import { normalizeHostedReviewHeadRef } from '../../../../shared/hosted-review-refs' +import type { GitStatusEntry, GitUpstreamStatus } from '../../../../shared/types' +import { getStageAllPaths } from './discard-all-sequence' + +export type CreatePrIntentRemoteStep = 'publish' | 'push' | 'force_push' | 'blocked' | 'none' + +export type CreatePrIntentRunToken = { + repoId: string + worktreeId: string + worktreePath: string + branch: string + startedAt: number +} + +export type CreatePrIntentCurrentTarget = { + repoId?: string | null + worktreeId?: string | null + worktreePath?: string | null + branch?: string | null +} + +export function createCreatePrIntentRunToken(input: Omit) { + return { ...input, startedAt: Date.now() } +} + +export function createPrIntentRunTokenMatches( + token: CreatePrIntentRunToken, + current: CreatePrIntentCurrentTarget +): boolean { + return ( + token.repoId === current.repoId && + token.worktreeId === current.worktreeId && + token.worktreePath === current.worktreePath && + token.branch === current.branch + ) +} + +export function createPrIntentGitStatusMatchesToken( + token: CreatePrIntentRunToken, + status: { branch?: string | null } +): boolean { + const branch = normalizeHostedReviewHeadRef(status.branch ?? '') + return branch.length > 0 && branch === token.branch +} + +export function getCreatePrIntentStagePaths(grouped: { + unstaged: GitStatusEntry[] + untracked: GitStatusEntry[] +}): string[] { + return [ + ...getStageAllPaths(grouped.unstaged, 'unstaged'), + ...getStageAllPaths(grouped.untracked, 'untracked') + ] +} + +export function resolveCreatePrIntentRemoteStep({ + upstreamStatus, + hostedReviewCreation, + branchCommitsAhead, + hasCurrentBranch +}: { + upstreamStatus: GitUpstreamStatus | undefined + hostedReviewCreation?: HostedReviewCreationEligibility | null + branchCommitsAhead?: number + hasCurrentBranch: boolean +}): CreatePrIntentRemoteStep { + if (!hasCurrentBranch || !hostedReviewCreation || hostedReviewCreation.canCreate) { + return 'none' + } + + if (hostedReviewCreation.blockedReason === 'no_upstream') { + return branchCommitsAhead && branchCommitsAhead > 0 ? 'publish' : 'blocked' + } + + if (hostedReviewCreation.blockedReason === 'needs_push') { + return 'push' + } + + if ( + hostedReviewCreation.blockedReason === 'needs_sync' && + shouldForcePushWithLeaseForUpstream(upstreamStatus) + ) { + return 'force_push' + } + + if (hostedReviewCreation.blockedReason === 'needs_sync') { + return 'blocked' + } + + return 'none' +} diff --git a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-state.test.ts b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-state.test.ts new file mode 100644 index 00000000000..4861ad586d7 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-state.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest' +import { resolveVisibleCreatePrHeaderAction } from './source-control-create-pr-intent-state' +import type { PrimaryAction } from './source-control-primary-action-types' + +const createPrIntentAction: PrimaryAction = { + kind: 'create_pr_intent', + label: 'Create PR', + title: 'Preparing branch for review…', + disabled: true +} + +const createPrAction: PrimaryAction = { + kind: 'create_pr', + label: 'Create PR', + title: 'Create a pull request for this branch', + disabled: false +} + +describe('resolveVisibleCreatePrHeaderAction', () => { + it('hides the header when the hosted-review composer owns direct Create PR', () => { + expect( + resolveVisibleCreatePrHeaderAction({ + createPrHeaderAction: createPrAction, + directCreatePrAction: createPrAction, + isCreatePrIntentInFlight: false, + primaryActionKind: 'create_pr' + }) + ).toBeNull() + }) + + it('hides the header while Create PR intent is in flight on the commit-area primary', () => { + expect( + resolveVisibleCreatePrHeaderAction({ + createPrHeaderAction: createPrIntentAction, + directCreatePrAction: null, + isCreatePrIntentInFlight: true, + primaryActionKind: 'create_pr_intent' + }) + ).toBeNull() + }) + + it('keeps the header visible when intent is in flight but the primary is a prerequisite action', () => { + expect( + resolveVisibleCreatePrHeaderAction({ + createPrHeaderAction: createPrIntentAction, + directCreatePrAction: null, + isCreatePrIntentInFlight: true, + primaryActionKind: 'publish' + }) + ).toEqual(createPrIntentAction) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-state.ts b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-state.ts new file mode 100644 index 00000000000..aedeec06c21 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-state.ts @@ -0,0 +1,98 @@ +import { shouldForcePushWithLeaseForUpstream } from '../../../../shared/git-upstream-status' +import type { HostedReviewCreationEligibility } from '../../../../shared/hosted-review' +import { supportsHostedReviewCreation } from '../../../../shared/hosted-review-creation-providers' +import type { GitUpstreamStatus } from '../../../../shared/types' +import type { PrimaryAction } from './source-control-primary-action-types' + +export type CreatePrIntentKind = + | 'dirty' + | 'message_required' + | 'no_upstream' + | 'needs_push' + | 'force_push' + +export type CreatePrIntentEligibility = { + eligible: boolean + kind: CreatePrIntentKind | null +} + +export function resolveCreatePrIntentEligibility({ + stagedCount, + hasStageableChanges, + hasMessage, + hasUnresolvedConflicts, + upstreamStatus, + hostedReviewCreation, + branchCommitsAhead, + hasCurrentBranch = true +}: { + stagedCount: number + hasStageableChanges: boolean + hasMessage: boolean + hasUnresolvedConflicts: boolean + upstreamStatus: GitUpstreamStatus | undefined + hostedReviewCreation?: HostedReviewCreationEligibility | null + branchCommitsAhead?: number + hasCurrentBranch?: boolean +}): CreatePrIntentEligibility { + if ( + hasUnresolvedConflicts || + !hasCurrentBranch || + !hostedReviewCreation || + hostedReviewCreation.canCreate || + !supportsHostedReviewCreation(hostedReviewCreation.provider) + ) { + return { eligible: false, kind: null } + } + + if (hostedReviewCreation.blockedReason === 'dirty') { + if (stagedCount > 0 && !hasMessage) { + return { eligible: true, kind: 'message_required' } + } + return { eligible: stagedCount > 0 || hasStageableChanges, kind: 'dirty' } + } + + if (hostedReviewCreation.blockedReason === 'no_upstream') { + const hasPublishableCommits = branchCommitsAhead === undefined ? false : branchCommitsAhead > 0 + return { + eligible: hasPublishableCommits || stagedCount > 0 || hasStageableChanges, + kind: 'no_upstream' + } + } + + if (hostedReviewCreation.blockedReason === 'needs_push') { + return { eligible: true, kind: 'needs_push' } + } + + if ( + hostedReviewCreation.blockedReason === 'needs_sync' && + shouldForcePushWithLeaseForUpstream(upstreamStatus) + ) { + return { eligible: true, kind: 'force_push' } + } + + return { eligible: false, kind: null } +} + +export function resolveVisibleCreatePrHeaderAction({ + createPrHeaderAction, + directCreatePrAction, + isCreatePrIntentInFlight, + primaryActionKind +}: { + createPrHeaderAction: PrimaryAction | null + directCreatePrAction: PrimaryAction | null + isCreatePrIntentInFlight: boolean + primaryActionKind: PrimaryAction['kind'] +}): PrimaryAction | null { + if (directCreatePrAction) { + return null + } + // Why: CommitArea already mirrors in-flight Create PR intent on the primary; + // keeping a second spinning header button stacks redundant spinners once + // message generation also shows one. + if (isCreatePrIntentInFlight && primaryActionKind === 'create_pr_intent') { + return null + } + return createPrHeaderAction +} diff --git a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts index 6d9cbeb2a69..21484eed1d5 100644 --- a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts @@ -596,6 +596,54 @@ describe('resolveDropdownItems', () => { expect(byKind.push_create_pr.disabled).toBe(false) }) + it.each(['azure-devops', 'gitea'] as const)( + 'enables push-before-PR recovery for %s review creation', + (provider) => { + const items = resolveDropdownItems( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hostedReviewCreation: { + provider, + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push' + } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.create_pr.label).toBe('Create PR') + expect(byKind.create_pr.hint).toBe('Push first') + expect(byKind.push_create_pr.label).toBe('Push before PR') + expect(byKind.push_create_pr.title).toBe('Push local commits before creating a pull request') + expect(byKind.push_create_pr.disabled).toBe(false) + } + ) + + it.each([ + ['azure-devops', 'Set ORCA_AZURE_DEVOPS_TOKEN in this environment'], + ['gitea', 'Set ORCA_GITEA_TOKEN in this environment'] + ] as const)('uses token auth copy when %s PR creation needs authentication', (provider, hint) => { + const items = resolveDropdownItems( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: { + provider, + review: null, + canCreate: false, + blockedReason: 'auth_required', + nextAction: 'authenticate' + } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.create_pr.hint).toBe(hint) + }) + it('uses GitLab auth copy when MR creation needs authentication', () => { const items = resolveDropdownItems( inputs({ diff --git a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts index 13f7114b4bd..6c58cd3dd78 100644 --- a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts +++ b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.ts @@ -4,6 +4,7 @@ import type { PrimaryActionInputs } from './source-control-primary-action' import type { GitConflictOperation } from '../../../../shared/types' import { shouldForcePushWithLeaseForUpstream } from '../../../../shared/git-upstream-status' +import { supportsHostedReviewCreation } from '../../../../shared/hosted-review-creation-providers' import { translate } from '@/i18n/i18n' import { localizedHostedReviewCopy, @@ -96,11 +97,19 @@ function formatRebaseBaseRef(baseRef: string): string { function reviewCopy( provider: NonNullable['provider'] | undefined ): ReturnType & { - authCommand: 'gh auth login' | 'glab auth login' + authInstruction: string } { + const authInstruction = + provider === 'gitlab' + ? 'Run glab auth login' + : provider === 'azure-devops' + ? 'Set ORCA_AZURE_DEVOPS_TOKEN' + : provider === 'gitea' + ? 'Set ORCA_GITEA_TOKEN' + : 'Run gh auth login' return { ...localizedHostedReviewCopy(resolveSupportedHostedReviewCopyProvider(provider)), - authCommand: provider === 'gitlab' ? 'glab auth login' : 'gh auth login' + authInstruction } } @@ -491,7 +500,7 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr case 'needs_sync': return shouldForcePushWithLease ? 'Force Push first' : 'Sync first' case 'auth_required': - return `Run ${createReviewCopy.authCommand} in this environment` + return `${createReviewCopy.authInstruction} in this environment` case 'unsupported_provider': return 'Unsupported provider' case 'existing_review': @@ -521,7 +530,7 @@ export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntr const canPushAndCreate = !globalBusy && !upstreamLoading && - (hostedReviewCreation?.provider === 'github' || hostedReviewCreation?.provider === 'gitlab') && + supportsHostedReviewCreation(hostedReviewCreation?.provider) && (hostedReviewCreation.blockedReason === 'needs_push' || (hostedReviewCreation.blockedReason === 'needs_sync' && shouldForcePushWithLease)) const pushCreatePRItem: DropdownItem = { diff --git a/src/renderer/src/components/right-sidebar/source-control-fix-split-button.tsx b/src/renderer/src/components/right-sidebar/source-control-fix-split-button.tsx new file mode 100644 index 00000000000..26fed796035 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-fix-split-button.tsx @@ -0,0 +1,171 @@ +import React, { useState } from 'react' +import { ChevronDown, RefreshCw, SlidersHorizontal, Sparkle } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger +} from '@/components/ui/dropdown-menu' +import { cn } from '@/lib/utils' +import type { + SourceControlActionRecipe, + SourceControlLaunchActionId +} from '../../../../shared/source-control-ai-actions' +import type { TuiAgent } from '../../../../shared/types' +import type { LaunchSource } from '../../../../shared/telemetry-events' +import type { SourceControlAiWriteTarget } from '../../../../shared/source-control-ai-recipe-save' +import { translate } from '@/i18n/i18n' +import { SourceControlAgentActionDialog } from './SourceControlAgentActionDialog' + +export type SourceControlFixSplitButtonProps = { + label: string + actionId: SourceControlLaunchActionId + dialogTitle: string + dialogDescription: string + launchSource: LaunchSource + contextUnavailableLabel: string + primaryTitle: string + primaryAriaLabel: string + chevronTitle: string + chevronAriaLabel: string + worktreeId: string | null + groupId: string | null + connectionId?: string | null + repoId?: string | null + launchPlatform?: NodeJS.Platform + prompt: string | null + isLaunching: boolean + disabledReason?: string + variant: React.ComponentProps['variant'] + size: React.ComponentProps['size'] + iconClassName: string + primaryClassName?: string + chevronClassName?: string + savedAgentId?: TuiAgent | null + savedCommandInputTemplate?: string | null + savedAgentArgs?: string | null + onSaveAgentDefault?: ( + target: SourceControlAiWriteTarget, + actionId: SourceControlLaunchActionId, + recipe: SourceControlActionRecipe + ) => void | Promise + onOpenSettings?: () => void + onFixWithDefaultAgent: (promptOverride?: string) => Promise | boolean + onPromptDelivered?: () => void +} + +export function SourceControlFixSplitButton({ + label, + actionId, + dialogTitle, + dialogDescription, + launchSource, + contextUnavailableLabel, + primaryTitle, + primaryAriaLabel, + chevronTitle, + chevronAriaLabel, + worktreeId, + groupId, + connectionId, + repoId, + launchPlatform, + prompt, + isLaunching, + disabledReason, + variant, + size, + iconClassName, + primaryClassName, + chevronClassName, + savedAgentId, + savedCommandInputTemplate, + savedAgentArgs, + onSaveAgentDefault, + onOpenSettings, + onFixWithDefaultAgent, + onPromptDelivered +}: SourceControlFixSplitButtonProps): React.JSX.Element { + const [composerOpen, setComposerOpen] = useState(false) + const canLaunch = Boolean(worktreeId && groupId && prompt && !disabledReason) + const dividerClass = variant === 'default' ? 'border-primary-foreground/20' : 'border-border' + + return ( + <> + +
+ + + + +
+ + {worktreeId && groupId && prompt && !disabledReason ? ( + setComposerOpen(true)} + className="gap-2 rounded-[7px] px-2 py-1.5 text-[12px] leading-5 font-medium" + > + + {translate( + 'auto.components.right.sidebar.SourceControl.f0a2dc9e46', + 'Customize launch...' + )} + + ) : ( + {contextUnavailableLabel} + )} + + + {worktreeId && groupId && prompt ? ( + + ) : null} + + ) +} diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-action-types.ts b/src/renderer/src/components/right-sidebar/source-control-primary-action-types.ts index 726729a2305..ca3ee189766 100644 --- a/src/renderer/src/components/right-sidebar/source-control-primary-action-types.ts +++ b/src/renderer/src/components/right-sidebar/source-control-primary-action-types.ts @@ -15,6 +15,7 @@ export type PrimaryActionKind = | 'pull' | 'sync' | 'publish' + | 'create_pr_intent' | 'create_pr' // Why: the in-flight remote op tracker stores which action the user actually @@ -65,6 +66,7 @@ export type PrimaryActionInputs = { // Why: detached HEAD can look like an unpublished branch from upstream // status alone, but it has no branch ref that Publish Branch can push. hasCurrentBranch?: boolean + isPrIntentInFlight?: boolean } export const PRIMARY_LABEL_BY_KIND: Record, string> = { @@ -73,5 +75,6 @@ export const PRIMARY_LABEL_BY_KIND: Record, pull: 'Pull', sync: 'Sync', publish: 'Publish Branch', + create_pr_intent: 'Create PR', create_pr: 'Create PR' } diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-action.create-pr-intent.test.ts b/src/renderer/src/components/right-sidebar/source-control-primary-action.create-pr-intent.test.ts new file mode 100644 index 00000000000..62338e0ba6f --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-primary-action.create-pr-intent.test.ts @@ -0,0 +1,281 @@ +import { describe, expect, it } from 'vitest' +import { + resolveCommitAreaPrimaryAction, + resolvePrimaryAction, + type PrimaryActionInputs +} from './source-control-primary-action' +import { resolveCreatePrHeaderAction } from './source-control-primary-create-pr-intent-action' + +function inputs(overrides: Partial = {}): PrimaryActionInputs { + return { + stagedCount: 0, + hasUnstagedChanges: false, + hasStageableChanges: false, + hasPartiallyStagedChanges: false, + hasMessage: false, + hasUnresolvedConflicts: false, + isCommitting: false, + isRemoteOperationActive: false, + upstreamStatus: undefined, + ...overrides + } +} + +const upstreamInSync = { + hasUpstream: true, + upstreamName: 'origin/main', + ahead: 0, + behind: 0 +} + +describe('resolvePrimaryAction Create PR intent', () => { + it('returns Create PR intent for an unpublished clean branch with commits to publish', () => { + const result = resolvePrimaryAction( + inputs({ + upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }, + branchCommitsAhead: 2, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'no_upstream', + nextAction: 'publish' + } + }) + ) + expect(result.kind).toBe('create_pr_intent') + expect(result.disabled).toBe(false) + }) + + it('returns Create PR intent for patch-equivalent force-push before review', () => { + const result = resolvePrimaryAction( + inputs({ + branchCommitsAhead: 4, + upstreamStatus: { + hasUpstream: true, + upstreamName: 'origin/feature', + ahead: 14, + behind: 3, + behindCommitsArePatchEquivalent: true + }, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_sync', + nextAction: 'sync' + } + }) + ) + expect(result.kind).toBe('create_pr_intent') + expect(result.disabled).toBe(false) + }) + + it('returns Create PR intent for a branch that needs a safe push before review', () => { + const input = inputs({ + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push' + } + }) + const result = resolvePrimaryAction(input) + expect(result.kind).toBe('create_pr_intent') + expect(result.disabled).toBe(false) + expect(resolveCreatePrHeaderAction(input)).toEqual(result) + expect(resolveCommitAreaPrimaryAction(input)).toEqual({ + kind: 'push', + label: 'Push', + title: 'Push 2 commits', + disabled: false + }) + }) + + it('returns Create PR intent for a dirty tree when hosted review prep can commit changes', () => { + const result = resolvePrimaryAction( + inputs({ + hasUnstagedChanges: true, + hasStageableChanges: true, + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit' + } + }) + ) + expect(result).toEqual({ + kind: 'create_pr_intent', + label: 'Create PR', + title: 'Prepare this branch and create a pull request', + disabled: false + }) + }) + + it('returns Create PR intent for staged changes without a message so the flow can request one', () => { + const result = resolvePrimaryAction( + inputs({ + stagedCount: 1, + hasMessage: false, + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit' + } + }) + ) + expect(result.kind).toBe('create_pr_intent') + expect(result.disabled).toBe(false) + }) + + it('returns Create MR intent with provider copy for a GitLab dirty branch', () => { + const result = resolvePrimaryAction( + inputs({ + stagedCount: 1, + hasMessage: true, + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider: 'gitlab', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit' + } + }) + ) + expect(result.kind).toBe('create_pr_intent') + expect(result.label).toBe('Create MR') + expect(result.title).toBe('Prepare this branch and create a merge request') + }) + + it('keeps in-flight Create MR intent copy provider-aware', () => { + const input = inputs({ + isPrIntentInFlight: true, + hostedReviewCreation: { + provider: 'gitlab', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit' + } + }) + + expect(resolvePrimaryAction(input)).toEqual({ + kind: 'create_pr_intent', + label: 'Create MR', + title: 'Preparing branch for review…', + disabled: true + }) + expect(resolveCreatePrHeaderAction(input)).toEqual({ + kind: 'create_pr_intent', + label: 'Create MR', + title: 'Preparing branch for review…', + disabled: true + }) + }) + + it.each(['azure-devops', 'gitea'] as const)( + 'returns Create PR intent for a %s branch that needs a safe push before review', + (provider) => { + const result = resolvePrimaryAction( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hostedReviewCreation: { + provider, + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push' + } + }) + ) + expect(result).toEqual({ + kind: 'create_pr_intent', + label: 'Create PR', + title: 'Prepare this branch and create a pull request', + disabled: false + }) + } + ) + + it('separates Publish Branch from the Create PR header action for unpublished commits', () => { + const input = inputs({ + upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }, + branchCommitsAhead: 2, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'no_upstream', + nextAction: 'publish' + } + }) + + expect(resolveCreatePrHeaderAction(input)?.kind).toBe('create_pr_intent') + expect(resolveCommitAreaPrimaryAction(input)).toEqual({ + kind: 'publish', + label: 'Publish Branch', + title: 'Publish this branch to origin', + disabled: false + }) + }) + + it('separates Force Push from the Create PR header action for patch-equivalent divergence', () => { + const input = inputs({ + branchCommitsAhead: 4, + upstreamStatus: { + hasUpstream: true, + upstreamName: 'origin/feature', + ahead: 14, + behind: 3, + behindCommitsArePatchEquivalent: true + }, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_sync', + nextAction: 'sync' + } + }) + + expect(resolveCreatePrHeaderAction(input)?.kind).toBe('create_pr_intent') + expect(resolveCommitAreaPrimaryAction(input)).toEqual({ + kind: 'push', + label: 'Force Push', + title: + 'Remote only has older copies of local commits. Force push 4 branch commits with lease to update origin/feature.', + disabled: false + }) + }) + + it('returns direct Create PR as a header action when the branch is ready', () => { + expect( + resolveCreatePrHeaderAction( + inputs({ + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: true, + blockedReason: null, + nextAction: null + } + }) + ) + ).toEqual({ + kind: 'create_pr', + label: 'Create PR', + title: 'Create a pull request for this branch', + disabled: false + }) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts b/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts index e32f9e8ae26..f43d94ea090 100644 --- a/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-primary-action.test.ts @@ -1,6 +1,9 @@ -/* eslint-disable max-lines -- Why: this state-machine table intentionally keeps every primary-action priority case together so merge regressions are visible in one file. */ import { describe, expect, it } from 'vitest' -import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action' +import { + resolveCommitAreaPrimaryAction, + resolvePrimaryAction, + type PrimaryActionInputs +} from './source-control-primary-action' // Why: a shared defaults object keeps each case row terse while making the // "this is the one knob that differs from the baseline" intent obvious. @@ -380,6 +383,58 @@ describe('resolvePrimaryAction', () => { expect(result.disabled).toBe(false) }) + it('keeps Stage All available in the commit area when Create PR intent is additive', () => { + const input = inputs({ + stagedCount: 0, + hasUnstagedChanges: true, + hasStageableChanges: true, + hasPartiallyStagedChanges: false, + hasMessage: false, + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit' + } + }) + + expect(resolvePrimaryAction(input).kind).toBe('create_pr_intent') + expect(resolveCommitAreaPrimaryAction(input)).toEqual({ + kind: 'stage', + label: 'Stage All', + title: 'Stage all changes', + disabled: false + }) + }) + + it('keeps the partial-staging reason on the additive commit-area Stage All action', () => { + const input = inputs({ + stagedCount: 1, + hasUnstagedChanges: true, + hasStageableChanges: true, + hasPartiallyStagedChanges: true, + hasMessage: true, + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit' + } + }) + + expect(resolvePrimaryAction(input).kind).toBe('create_pr_intent') + expect(resolveCommitAreaPrimaryAction(input)).toEqual({ + kind: 'stage', + label: 'Stage All', + title: 'Stage all changes before committing partially staged files', + disabled: false + }) + }) + it('still resolves to Commit when staged and unrelated unstaged files exist', () => { const result = resolvePrimaryAction( inputs({ @@ -486,4 +541,28 @@ describe('resolvePrimaryAction', () => { disabled: false }) }) + + it.each(['azure-devops', 'gitea'] as const)( + 'returns Create PR when a clean tracked %s branch is eligible for review creation', + (provider) => { + const result = resolvePrimaryAction( + inputs({ + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider, + review: null, + canCreate: true, + blockedReason: null, + nextAction: null + } + }) + ) + expect(result).toEqual({ + kind: 'create_pr', + label: 'Create PR', + title: 'Create a pull request for this branch', + disabled: false + }) + } + ) }) diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-action.ts b/src/renderer/src/components/right-sidebar/source-control-primary-action.ts index db1fdb6518f..05eff4ae0df 100644 --- a/src/renderer/src/components/right-sidebar/source-control-primary-action.ts +++ b/src/renderer/src/components/right-sidebar/source-control-primary-action.ts @@ -14,6 +14,11 @@ import { describePushCount, describeSyncCounts } from './source-control-primary-action-titles' +import { + resolveCreatePrIntentInFlightPrimaryAction, + resolveCreatePrIntentPrimaryAction +} from './source-control-primary-create-pr-intent-action' +import { resolveUnpublishedPrimaryAction } from './source-control-primary-unpublished-action' export type { PrimaryActionKind, @@ -34,13 +39,15 @@ export type { * 1. In-flight commit locks the primary to a disabled "Commit". * 2. In-flight remote operation keeps the current label but disables it. * 3. Unresolved conflicts block the commit path entirely. - * 4. Has partially staged files → "Stage All" to avoid hook-time partial + * 4. Create PR intent can own the primary; manual prerequisites are + * exposed as a visible sibling action by CommitArea. + * 5. Has partially staged files → "Stage All" to avoid hook-time partial * stash conflicts. - * 5. Has staged files + message → plain "Commit" (compound flows live in - * the dropdown; after the commit lands, step 7 rotates the primary to - * the appropriate single remote action). - * 6. Has staged files + no message → disabled "Commit" with a reason. - * 7. Clean tree → adaptive remote action (or disabled "Commit" no-op). + * 6. Has staged files + message → plain "Commit" (compound flows live in + * the dropdown; after the commit lands, the clean-tree rung rotates + * the primary to the appropriate single remote action). + * 7. Has staged files + no message → disabled "Commit" with a reason. + * 8. Clean tree → adaptive remote action (or disabled "Commit" no-op). * * An undefined upstream status means fetchUpstreamStatus has not resolved * yet for this worktree. We return a disabled Commit so the button has a @@ -62,9 +69,14 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction isPRStateLoading, hostedReviewCreation, branchCommitsAhead, - hasCurrentBranch = true + hasCurrentBranch = true, + isPrIntentInFlight = false } = inputs + if (isPrIntentInFlight) { + return resolveCreatePrIntentInFlightPrimaryAction(inputs) + } + // 1. Commit in flight — lock the primary no matter what else is true. if (isCommitting) { return { @@ -101,11 +113,15 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction } } + const createPrIntent = resolveCreatePrIntentPrimaryAction(inputs) + if (createPrIntent) { + return createPrIntent + } + const hasStaged = stagedCount > 0 - // 4. A path with both staged and unstaged edits can make lint-staged's - // partial-stash restore fail after formatters rewrite the staged copy. Push - // the user through Stage All first so the index matches the worktree. + // Why: partial staging can break hook-time restores during the intent flow; + // keep Stage All visible as a sibling prerequisite without replacing Create PR. if (hasStaged && hasPartiallyStagedChanges) { return { kind: 'stage', @@ -194,78 +210,12 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction } if (!upstreamStatus.hasUpstream) { - if (!hasCurrentBranch) { - return { - kind: 'commit', - label: translate( - 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', - 'Commit' - ), - title: translate( - 'auto.components.right.sidebar.source.control.primary.action.e61b0d7a3c', - 'Check out a branch before publishing commits.' - ), - disabled: true - } - } - - if (branchCommitsAhead === 0) { - return { - kind: 'commit', - label: translate( - 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', - 'Commit' - ), - title: translate( - 'auto.components.right.sidebar.source.control.primary.action.acce237921', - 'Nothing to commit. Branch has no changes to publish.' - ), - disabled: true - } - } - - if (isPRStateLoading) { - return { - kind: 'commit', - label: translate( - 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', - 'Commit' - ), - title: translate( - 'auto.components.right.sidebar.source.control.primary.action.41d4bcf157', - 'Checking PR status…' - ), - disabled: true - } - } - - if (prState === 'merged') { - return { - kind: 'commit', - label: translate( - 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', - 'Commit' - ), - title: translate( - 'auto.components.right.sidebar.source.control.primary.action.3d5dccef0b', - 'Nothing to commit. PR is already merged.' - ), - disabled: true - } - } - - return { - kind: 'publish', - label: translate( - 'auto.components.right.sidebar.source.control.primary.action.7b4d02e6b8', - 'Publish Branch' - ), - title: translate( - 'auto.components.right.sidebar.source.control.primary.action.1884cf34af', - 'Publish this branch to origin' - ), - disabled: false - } + return resolveUnpublishedPrimaryAction({ + hasCurrentBranch, + branchCommitsAhead, + isPRStateLoading, + prState + }) } if (upstreamStatus.ahead > 0 && upstreamStatus.behind > 0) { @@ -347,3 +297,13 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction disabled: true } } + +export function resolveCommitAreaPrimaryAction(inputs: PrimaryActionInputs): PrimaryAction { + // Why: review creation is additive chrome. The commit area should keep the + // same local/remote primary action it would have without review eligibility. + return resolvePrimaryAction({ + ...inputs, + hostedReviewCreation: null, + isPrIntentInFlight: false + }) +} diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-create-pr-intent-action.ts b/src/renderer/src/components/right-sidebar/source-control-primary-create-pr-intent-action.ts new file mode 100644 index 00000000000..04901c43096 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-primary-create-pr-intent-action.ts @@ -0,0 +1,96 @@ +import { translate } from '@/i18n/i18n' +import { + localizedHostedReviewCopy, + resolveSupportedHostedReviewCopyProvider +} from '@/i18n/hosted-review-localized-copy' +import type { PrimaryAction, PrimaryActionInputs } from './source-control-primary-action-types' +import { resolveCreatePrIntentEligibility } from './source-control-create-pr-intent-state' + +export function resolveCreatePrIntentInFlightPrimaryAction( + inputs?: Pick +): PrimaryAction { + const copy = localizedHostedReviewCopy( + resolveSupportedHostedReviewCopyProvider(inputs?.hostedReviewCreation?.provider) + ) + + return { + kind: 'create_pr_intent', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.e7ffa46946', + 'Create {{value0}}', + { value0: copy.shortLabel } + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.d37e68f61d', + 'Preparing branch for review…' + ), + disabled: true + } +} + +export function resolveCreatePrIntentPrimaryAction( + inputs: PrimaryActionInputs +): PrimaryAction | null { + const createPrIntent = resolveCreatePrIntentEligibility({ + stagedCount: inputs.stagedCount, + hasStageableChanges: inputs.hasStageableChanges, + hasMessage: inputs.hasMessage, + hasUnresolvedConflicts: inputs.hasUnresolvedConflicts, + upstreamStatus: inputs.upstreamStatus, + hostedReviewCreation: inputs.hostedReviewCreation, + branchCommitsAhead: inputs.branchCommitsAhead, + hasCurrentBranch: inputs.hasCurrentBranch + }) + if (!createPrIntent.eligible) { + return null + } + const copy = localizedHostedReviewCopy( + resolveSupportedHostedReviewCopyProvider(inputs.hostedReviewCreation?.provider) + ) + return { + kind: 'create_pr_intent', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.e7ffa46946', + 'Create {{value0}}', + { value0: copy.shortLabel } + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.c72e5e65d1', + 'Prepare this branch and create a {{value0}}', + { value0: copy.reviewLabel } + ), + disabled: false + } +} + +export function resolveCreatePrHeaderAction(inputs: PrimaryActionInputs): PrimaryAction | null { + if (inputs.isPrIntentInFlight) { + return resolveCreatePrIntentInFlightPrimaryAction(inputs) + } + + if (inputs.isCommitting || inputs.isRemoteOperationActive || inputs.hasUnresolvedConflicts) { + return null + } + + if (inputs.hostedReviewCreation?.canCreate) { + const copy = localizedHostedReviewCopy( + resolveSupportedHostedReviewCopyProvider(inputs.hostedReviewCreation.provider) + ) + return { + kind: 'create_pr', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.e7ffa46946', + 'Create {{value0}}', + { value0: copy.shortLabel } + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.946a8a05ea', + 'Create a {{value0}} for this branch', + { value0: copy.reviewLabel } + ), + disabled: false + } + } + + return resolveCreatePrIntentPrimaryAction(inputs) +} diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-unpublished-action.ts b/src/renderer/src/components/right-sidebar/source-control-primary-unpublished-action.ts new file mode 100644 index 00000000000..d7f1821613e --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-primary-unpublished-action.ts @@ -0,0 +1,88 @@ +import { translate } from '@/i18n/i18n' +import type { PrimaryAction } from './source-control-primary-action-types' +import type { PRState } from '../../../../shared/types' + +export function resolveUnpublishedPrimaryAction({ + hasCurrentBranch, + branchCommitsAhead, + isPRStateLoading, + prState +}: { + hasCurrentBranch: boolean + branchCommitsAhead?: number + isPRStateLoading?: boolean + prState?: PRState | null +}): PrimaryAction { + if (!hasCurrentBranch) { + return { + kind: 'commit', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', + 'Commit' + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.e61b0d7a3c', + 'Check out a branch before publishing commits.' + ), + disabled: true + } + } + + if (branchCommitsAhead === 0) { + return { + kind: 'commit', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', + 'Commit' + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.acce237921', + 'Nothing to commit. Branch has no changes to publish.' + ), + disabled: true + } + } + + if (isPRStateLoading) { + return { + kind: 'commit', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', + 'Commit' + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.41d4bcf157', + 'Checking PR status…' + ), + disabled: true + } + } + + if (prState === 'merged') { + return { + kind: 'commit', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.ed93b4f14f', + 'Commit' + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.3d5dccef0b', + 'Nothing to commit. PR is already merged.' + ), + disabled: true + } + } + + return { + kind: 'publish', + label: translate( + 'auto.components.right.sidebar.source.control.primary.action.7b4d02e6b8', + 'Publish Branch' + ), + title: translate( + 'auto.components.right.sidebar.source.control.primary.action.1884cf34af', + 'Publish this branch to origin' + ), + disabled: false + } +} diff --git a/src/renderer/src/components/right-sidebar/useCreatePullRequestDialogFields.ts b/src/renderer/src/components/right-sidebar/useCreatePullRequestDialogFields.ts index 4be16c0ba73..48c6e31112c 100644 --- a/src/renderer/src/components/right-sidebar/useCreatePullRequestDialogFields.ts +++ b/src/renderer/src/components/right-sidebar/useCreatePullRequestDialogFields.ts @@ -369,7 +369,9 @@ export function useCreatePullRequestDialogFields({ base: stripBaseRef(base.trim()), title, body, - draft + draft, + provider: eligibility?.provider, + useTemplate: resolvedPrDefaults.useTemplate }, overrides ) @@ -417,9 +419,11 @@ export function useCreatePullRequestDialogFields({ draft, effectiveGenerating, applyGeneratedFields, + eligibility?.provider, generation, generateDisabled, onBranchChangedByGeneration, + resolvedPrDefaults.useTemplate, settings, title, worktreeId, diff --git a/src/renderer/src/components/settings/MobilePairingQrSection.tsx b/src/renderer/src/components/settings/MobilePairingQrSection.tsx index a3fed5d771b..e09fc669f8f 100644 --- a/src/renderer/src/components/settings/MobilePairingQrSection.tsx +++ b/src/renderer/src/components/settings/MobilePairingQrSection.tsx @@ -81,7 +81,7 @@ export function MobilePairingQrSection({ )} className="size-48" /> - + {endpoint && {endpoint}}

diff --git a/src/renderer/src/components/settings/RepositoryHostSetupsSection.test.tsx b/src/renderer/src/components/settings/RepositoryHostSetupsSection.test.tsx index 465e3b0478e..2ebc332f7a1 100644 --- a/src/renderer/src/components/settings/RepositoryHostSetupsSection.test.tsx +++ b/src/renderer/src/components/settings/RepositoryHostSetupsSection.test.tsx @@ -3,7 +3,7 @@ import React, { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { getLocalExecutionHostLabel, toSshExecutionHostId } from '../../../../shared/execution-host' +import { getExecutionHostLabel, toSshExecutionHostId } from '../../../../shared/execution-host' import { PROJECT_HOST_SETUP_RUNTIME_CAPABILITY, RUNTIME_PROTOCOL_VERSION, @@ -15,7 +15,8 @@ import { RepositoryHostSetupsSection } from './RepositoryHostSetupsSection' let container: HTMLDivElement let root: Root -const localHostLabel = getLocalExecutionHostLabel() + +const LOCAL_HOST_LABEL = getExecutionHostLabel('local') function makeRepo(overrides: Partial & Pick): Repo { return { @@ -143,7 +144,7 @@ describe('RepositoryHostSetupsSection', () => { renderSection(localRepo) expect(container.textContent).toContain('Viewing host') - expect(container.textContent).toContain(localHostLabel) + expect(container.textContent).toContain(LOCAL_HOST_LABEL) }) it('opens the selected host setup settings pane through the setup repo id', () => { diff --git a/src/renderer/src/components/settings/ShortcutBindingRow.tsx b/src/renderer/src/components/settings/ShortcutBindingRow.tsx index fb1bc35b9bd..b2abc41379c 100644 --- a/src/renderer/src/components/settings/ShortcutBindingRow.tsx +++ b/src/renderer/src/components/settings/ShortcutBindingRow.tsx @@ -160,7 +160,7 @@ export function ShortcutBindingRow({ {/* Reset/Disable reveal on hover or keyboard focus to keep the row calm; they stay reachable via focus-within for keyboard users. */} {hasBinding ? ( -

+
{modified ? ( diff --git a/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx b/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx index 27bb214040d..870b498b5c0 100644 --- a/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx +++ b/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx @@ -162,12 +162,12 @@ export function VoiceSpeechModelSection({ ) ) }} - className="shrink-0 p-1 text-muted-foreground opacity-0 group-hover:opacity-100 hover:text-destructive transition-all rounded" + className="shrink-0 p-1 text-muted-foreground can-hover:opacity-0 group-hover:opacity-100 hover:text-destructive transition-all rounded" > ) : !isCloud && !isReady && !isDownloading ? ( - + ) : null} diff --git a/src/renderer/src/components/settings/cli-source-control-integration-cards.test.tsx b/src/renderer/src/components/settings/cli-source-control-integration-cards.test.tsx index aa9ed66320e..c5965f17289 100644 --- a/src/renderer/src/components/settings/cli-source-control-integration-cards.test.tsx +++ b/src/renderer/src/components/settings/cli-source-control-integration-cards.test.tsx @@ -3,12 +3,14 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it, vi } from 'vitest' -import { getLocalExecutionHostLabel } from '../../../../shared/execution-host' +import { getExecutionHostLabel } from '../../../../shared/execution-host' import { GitHubIntegrationCard, GitLabIntegrationCard } from './cli-source-control-integration-cards' +const LOCAL_HOST_LABEL = getExecutionHostLabel('local') + type StoreState = { settings: { activeRuntimeEnvironmentId: string | null } openSettingsPage: () => void @@ -48,7 +50,6 @@ vi.mock('./source-control-preflight-card-status', () => ({ let root: Root | null = null let container: HTMLDivElement | null = null -const localHostLabel = getLocalExecutionHostLabel() async function renderCard(card: React.ReactNode): Promise { container = document.createElement('div') @@ -90,7 +91,7 @@ describe('CLI source-control integration card account scope', () => { expect(rendered.textContent).toContain('GitHub') expect(rendered.textContent).toContain('Connected') - expect(rendered.textContent).toContain(`Account scope: ${localHostLabel}`) + expect(rendered.textContent).toContain(`Account scope: ${LOCAL_HOST_LABEL}`) expect(rendered.textContent).toContain( 'Credentials and account checks for this provider are owned by this desktop client. Use Settings > Remote Orca Servers > Advanced to edit server-owned credentials.' ) diff --git a/src/renderer/src/components/settings/provider-account-scope.test.ts b/src/renderer/src/components/settings/provider-account-scope.test.ts index 1216f93ed10..7727cc41401 100644 --- a/src/renderer/src/components/settings/provider-account-scope.test.ts +++ b/src/renderer/src/components/settings/provider-account-scope.test.ts @@ -1,11 +1,13 @@ import { describe, expect, it } from 'vitest' -import { getLocalExecutionHostLabel } from '../../../../shared/execution-host' +import { getExecutionHostLabel } from '../../../../shared/execution-host' import { getProviderAccountScope, getProviderRateLimitScope } from './provider-account-scope' +const LOCAL_HOST_LABEL = getExecutionHostLabel('local') + describe('getProviderAccountScope', () => { it('describes provider accounts as client-owned without an active runtime', () => { expect(getProviderAccountScope({ activeRuntimeEnvironmentId: null })).toEqual({ - label: getLocalExecutionHostLabel(), + label: LOCAL_HOST_LABEL, description: 'Credentials and account checks for this provider are owned by this desktop client. Use Settings > Remote Orca Servers > Advanced to edit server-owned credentials.' }) @@ -21,7 +23,7 @@ describe('getProviderAccountScope', () => { it('describes provider API budgets as host-scoped', () => { expect(getProviderRateLimitScope({ activeRuntimeEnvironmentId: null }, 'GitHub')).toEqual({ - label: getLocalExecutionHostLabel(), + label: LOCAL_HOST_LABEL, description: 'GitHub API budget is fetched from the CLI on this desktop client. Use Settings > Remote Orca Servers > Advanced to view server-owned budgets.' }) diff --git a/src/renderer/src/components/settings/provider-rate-limit-scope-panels.test.tsx b/src/renderer/src/components/settings/provider-rate-limit-scope-panels.test.tsx index 28a8924691a..389964b4b55 100644 --- a/src/renderer/src/components/settings/provider-rate-limit-scope-panels.test.tsx +++ b/src/renderer/src/components/settings/provider-rate-limit-scope-panels.test.tsx @@ -1,8 +1,10 @@ import { renderToStaticMarkup } from 'react-dom/server' import { describe, expect, it, vi } from 'vitest' +import { getExecutionHostLabel } from '../../../../shared/execution-host' import { GitHubRateLimitPanel } from '@/components/github/github-rate-limit-display' import { GitLabRateLimitPanel } from '@/components/gitlab/gitlab-rate-limit-display' -import { getLocalExecutionHostLabel } from '../../../../shared/execution-host' + +const LOCAL_HOST_LABEL = getExecutionHostLabel('local') type StoreState = { settings: { activeRuntimeEnvironmentId: string | null } @@ -19,7 +21,6 @@ const mocks = vi.hoisted(() => ({ } as StoreState } })) -const localHostLabel = getLocalExecutionHostLabel() vi.mock('@/store', () => ({ useAppStore: (selector: (state: StoreState) => unknown) => selector(mocks.store.current) @@ -35,7 +36,7 @@ describe('provider rate-limit panels account scope', () => { const markup = renderToStaticMarkup() - expect(markup).toContain(`Budget scope: ${localHostLabel}`) + expect(markup).toContain(`Budget scope: ${LOCAL_HOST_LABEL}`) expect(markup).toContain( 'GitHub API budget is fetched from the CLI on this desktop client. Use Settings > Remote Orca Servers > Advanced to view server-owned budgets.' ) diff --git a/src/renderer/src/components/settings/task-tracker-integration-cards.test.tsx b/src/renderer/src/components/settings/task-tracker-integration-cards.test.tsx index 778e1c369f0..0dd26191914 100644 --- a/src/renderer/src/components/settings/task-tracker-integration-cards.test.tsx +++ b/src/renderer/src/components/settings/task-tracker-integration-cards.test.tsx @@ -3,10 +3,12 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it, vi } from 'vitest' -import { getLocalExecutionHostLabel } from '../../../../shared/execution-host' +import { getExecutionHostLabel } from '../../../../shared/execution-host' import { getProviderRuntimeContextKey } from '@/lib/provider-runtime-context' import { LinearIntegrationCard } from './task-tracker-integration-cards' +const LOCAL_HOST_LABEL = getExecutionHostLabel('local') + type StoreState = { linearStatus: { connected: boolean @@ -46,7 +48,6 @@ vi.mock('@/components/linear-api-key-dialog', () => ({ let root: Root | null = null let container: HTMLDivElement | null = null -const localHostLabel = getLocalExecutionHostLabel() function installStore( connected: boolean, @@ -108,7 +109,7 @@ describe('LinearIntegrationCard account scope', () => { const rendered = await renderCard() - expect(rendered.textContent).toContain(`Account scope: ${localHostLabel}`) + expect(rendered.textContent).toContain(`Account scope: ${LOCAL_HOST_LABEL}`) expect(rendered.textContent).toContain( 'Credentials and account checks for this provider are owned by this desktop client. Use Settings > Remote Orca Servers > Advanced to edit server-owned credentials.' ) diff --git a/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx b/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx index fddb48137b5..5c944fa3a1e 100644 --- a/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx +++ b/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx @@ -187,7 +187,7 @@ export function HostSectionHeaderMenu({ row }: { row: HostHeaderRow }): React.JS variant="ghost" size="icon-xs" type="button" - className="size-5 shrink-0 text-muted-foreground opacity-0 transition-opacity focus-visible:opacity-100 group-hover/host-header:opacity-100 data-[state=open]:opacity-100" + className="size-5 shrink-0 text-muted-foreground can-hover:opacity-0 transition-opacity focus-visible:opacity-100 group-hover/host-header:opacity-100 data-[state=open]:opacity-100" aria-label={translate( 'auto.components.sidebar.HostSectionHeaderMenu.4f2c8a9b10', 'Host actions for {{value0}}', diff --git a/src/renderer/src/components/sidebar/PendingWorktreeRow.tsx b/src/renderer/src/components/sidebar/PendingWorktreeRow.tsx index 38b66950e49..4c40dfea829 100644 --- a/src/renderer/src/components/sidebar/PendingWorktreeRow.tsx +++ b/src/renderer/src/components/sidebar/PendingWorktreeRow.tsx @@ -84,7 +84,7 @@ export function PendingWorktreeRow({ onClick={() => useAppStore.getState().removePendingWorktreeCreation(creationId)} className={cn( 'mr-1 flex size-5 shrink-0 items-center justify-center rounded text-muted-foreground transition-opacity hover:bg-sidebar-accent hover:text-foreground focus-visible:opacity-100', - isError ? 'opacity-100' : 'opacity-0 group-hover:opacity-100' + isError ? 'opacity-100' : 'can-hover:opacity-0 group-hover:opacity-100' )} > diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx index aef39b12e1d..033350a5fe7 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx @@ -182,7 +182,7 @@ export default function WorkspaceKanbanStatusLane({ variant="secondary" size="xs" className={cn( - 'mt-2 h-7 w-full opacity-0 transition-opacity', + 'mt-2 h-7 w-full can-hover:opacity-0 transition-opacity', 'group-hover/lane:opacity-100 group-focus-within/lane:opacity-100' )} aria-label={createTooltip} diff --git a/src/renderer/src/components/sidebar/WorktreeCardPorts.tsx b/src/renderer/src/components/sidebar/WorktreeCardPorts.tsx index ba30c1f2791..6f543521629 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardPorts.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardPorts.tsx @@ -242,7 +242,7 @@ function WorktreePortRow({ port }: { port: WorkspacePort }): React.JSX.Element { -
+
) : ( - + )} diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 804f60223fd..a7f49a267f8 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -571,8 +571,8 @@ function getHostHeaderDetail(row: HostHeaderRow): { text: string; isWarning: boo isWarning: false } } - // Why: the transport suffix only earns space on remote hosts; "This - // computer" on Local Mac is noise. + // Why: the transport suffix only earns space on remote hosts; repeating + // "This computer" under the local host label is noise. if (row.kind !== 'local') { return { text: row.detail, isWarning: false } } @@ -651,7 +651,7 @@ function HostSectionHeader({ ) : null}
-
+
@@ -3507,7 +3507,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
-
+
{ it('hides host controls for local-only workspaces', () => { const hosts = buildSidebarHostOptions({ @@ -19,7 +21,7 @@ describe('sidebar host options', () => { expect(hosts).toEqual([ { id: 'local', - label: getLocalExecutionHostLabel(), + label: LOCAL_HOST_LABEL, detail: 'This computer', kind: 'local', health: 'local', @@ -174,13 +176,8 @@ describe('sidebar host options', () => { }) expect(buildSidebarHostScopeOptions(hosts)).toMatchObject([ - { - id: 'all', - label: 'All hosts', - detail: `${getLocalExecutionHostLabel()}, Builder`, - health: 'mixed' - }, - { id: 'local', label: getLocalExecutionHostLabel(), health: 'local' }, + { id: 'all', label: 'All hosts', detail: `${LOCAL_HOST_LABEL}, Builder`, health: 'mixed' }, + { id: 'local', label: LOCAL_HOST_LABEL, health: 'local' }, { id: 'ssh:ssh-1', label: 'Builder', health: 'disconnected' } ]) }) diff --git a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts index effb71bda96..269c5af47cc 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts @@ -2,7 +2,7 @@ import { readFileSync } from 'node:fs' import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' -import { getLocalExecutionHostLabel } from '../../../../shared/execution-host' +import { getExecutionHostLabel } from '../../../../shared/execution-host' import { ALL_GROUP_META, buildRows, @@ -24,7 +24,7 @@ import type { WorktreeLineage } from '../../../../shared/types' -const localHostLabel = getLocalExecutionHostLabel() +const LOCAL_HOST_LABEL = getExecutionHostLabel('local') const repo: Repo = { id: 'repo-1', @@ -349,7 +349,7 @@ describe('buildRows with pinned worktrees', () => { expect(rows).toMatchObject([ { type: 'header', key: 'project:github:stablyai/orca', label: 'Orca', count: 2 }, - { type: 'item', worktree: { id: worktree.id }, hostContextLabel: localHostLabel }, + { type: 'item', worktree: { id: worktree.id }, hostContextLabel: LOCAL_HOST_LABEL }, { type: 'item', worktree: { id: remoteWorktree.id }, hostContextLabel: 'gpu-vm' } ]) }) @@ -464,14 +464,14 @@ describe('buildRows with pinned worktrees', () => { { projects: [project], projectHostSetups: [projectHostSetups[0]!, runtimeSetup] }, [], new Map([ - ['local', localHostLabel], + ['local', LOCAL_HOST_LABEL], ['runtime:03ef704c-b180-4b10-998d-e28fbd5de9a3', 'dev box'] ]) ) expect(rows).toMatchObject([ { type: 'header', key: 'project:github:stablyai/orca', label: 'Orca', count: 2 }, - { type: 'item', worktree: { id: worktree.id }, hostContextLabel: localHostLabel }, + { type: 'item', worktree: { id: worktree.id }, hostContextLabel: LOCAL_HOST_LABEL }, { type: 'item', worktree: { id: runtimeWorktree.id }, hostContextLabel: 'dev box' } ]) }) diff --git a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx index 75c68e658d2..4f97e0ec764 100644 --- a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx @@ -414,7 +414,7 @@ function SessionRow({ className={cn( 'rounded p-0.5 text-muted-foreground transition-opacity hover:bg-destructive/10 hover:text-destructive', session.bound && - 'opacity-0 group-hover/sessrow:opacity-100 group-focus-within/sessrow:opacity-100 focus-visible:opacity-100' + 'can-hover:opacity-0 group-hover/sessrow:opacity-100 group-focus-within/sessrow:opacity-100 focus-visible:opacity-100' )} aria-label={translate( 'auto.components.status.bar.ResourceUsageStatusSegment.fa6d36758d', @@ -528,18 +528,20 @@ function WorktreeRow({
+ {/* Why: no-hover devices show the action overlay by default, so + the sparkline yields there just like it does during hover. */} {showWorktreeActions && ( -
+
diff --git a/src/renderer/src/components/status-bar/ports-status-popover-rows.tsx b/src/renderer/src/components/status-bar/ports-status-popover-rows.tsx index e949b9f306d..b0a4103379b 100644 --- a/src/renderer/src/components/status-bar/ports-status-popover-rows.tsx +++ b/src/renderer/src/components/status-bar/ports-status-popover-rows.tsx @@ -217,7 +217,7 @@ export function PortRow({ {processLabel} -
+
- +