feat(source-control): commit, push, pull, and sync actions in panel (#1211)

Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Alexander Saavedra <mralexsaavedra@gmail.com>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: brennanb2025 <brennankbenson@gmail.com>
This commit is contained in:
Alexander Saavedra
2026-05-06 15:03:52 -07:00
committed by GitHub
co-authored by Orca Neil Brennan Benson Jinjing github-actions[bot] brennanb2025
parent 5c7e5f2f1d
commit 9f8bf81c38
26 changed files with 3387 additions and 115 deletions
+134
View File
@@ -0,0 +1,134 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { gitExecFileAsyncMock } = vi.hoisted(() => ({
gitExecFileAsyncMock: vi.fn()
}))
vi.mock('./runner', () => ({
gitExecFileAsync: gitExecFileAsyncMock
}))
import { gitFetch, gitPull, gitPush } from './remote'
describe('git remote operations', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
})
it('pushes with --set-upstream regardless of publish flag', async () => {
// Why: every push uses --set-upstream so worktrees that were created
// tracking the BASE ref (origin/main) get their upstream repointed to
// origin/<branch> on first push. Without that the local branch keeps
// tracking origin/main forever and the UI's ahead/behind read via
// @{u} measures "ahead of base" rather than "ahead of remote branch".
// Both publish=true and publish=false take the same path now; the
// parameter is preserved in the signature for IPC compatibility but
// is no longer load-bearing.
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
await gitPush('/repo', true)
await gitPush('/repo', false)
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
1,
['push', '--set-upstream', 'origin', 'HEAD'],
{ cwd: '/repo' }
)
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
2,
['push', '--set-upstream', 'origin', 'HEAD'],
{ cwd: '/repo' }
)
})
it('maps non-fast-forward push failures to an actionable message', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('remote rejected: non-fast-forward'))
await expect(gitPush('/repo', false)).rejects.toThrow(
'Push rejected: remote has newer commits (non-fast-forward). Please pull or sync first.'
)
})
it('passes through clean tail line when push error does not match known patterns', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(
new Error('Command failed: git push\nfatal: something obscure happened')
)
await expect(gitPush('/repo', false)).rejects.toThrow('fatal: something obscure happened')
})
it('strips embedded credentials from push error messages', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(
new Error(
'Command failed: git push\nhttps://x-access-token:ghp_abc@github.com/foo/bar.git\nfatal: remote error'
)
)
let caught: Error | undefined
try {
await gitPush('/repo', false)
} catch (error) {
caught = error as Error
}
expect(caught).toBeInstanceOf(Error)
expect(caught?.message).not.toContain('ghp_abc')
expect(caught?.message).not.toContain('x-access-token')
})
it('strips token-only credentials (https://TOKEN@host) from push error messages', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(
new Error(
'Command failed: git push\nhttps://ghp_onlyToken@github.com/foo/bar.git\nfatal: remote error'
)
)
let caught: Error | undefined
try {
await gitPush('/repo', false)
} catch (error) {
caught = error as Error
}
expect(caught).toBeInstanceOf(Error)
expect(caught?.message).not.toContain('ghp_onlyToken')
})
it('falls back to a generic message for non-Error rejections', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce('string')
await expect(gitPush('/repo', false)).rejects.toThrow('Git remote operation failed.')
})
it("runs pull with the user's configured strategy", async () => {
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
await gitPull('/repo')
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['pull'], { cwd: '/repo' })
})
it('normalizes pull authentication errors to a friendly message', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('Authentication failed'))
await expect(gitPull('/repo')).rejects.toThrow(
'Authentication failed. Check your remote credentials.'
)
})
it('runs fetch with prune', async () => {
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
await gitFetch('/repo')
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['fetch', '--prune'], { cwd: '/repo' })
})
it('normalizes fetch authentication errors to a friendly message', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('Authentication failed'))
await expect(gitFetch('/repo')).rejects.toThrow(
'Authentication failed. Check your remote credentials.'
)
})
})
+46
View File
@@ -0,0 +1,46 @@
import { normalizeGitErrorMessage } from '../../shared/git-remote-error'
import { gitExecFileAsync } from './runner'
export async function gitPush(worktreePath: string, _publish = false): Promise<void> {
try {
// Why: always pass --set-upstream so that worktrees Orca creates with
// `git worktree add --track -b <name> <dir> <baseRef>` (which initially
// track the BASE — e.g. origin/main) get their upstream repointed to
// origin/<branch> on first push. Without this the local branch keeps
// tracking origin/main forever, so ahead/behind reads via @{u} measure
// "ahead of base" rather than "ahead of remote branch", and the primary
// button never rotates from "Push" to "Commit" after a successful push.
//
// The `publish` flag becomes redundant under this strategy — every push
// sets upstream, including the first. We keep the parameter in the
// signature so callers don't need to change, but it's no longer
// load-bearing. On an already-published branch --set-upstream is a
// no-op for the tracking config and a regular push otherwise.
//
// Branch-vs-base reporting (the "Committed on Branch" section) is
// unaffected because it uses branchCompare against an explicit baseRef
// from worktree config, not the upstream relationship.
await gitExecFileAsync(['push', '--set-upstream', 'origin', 'HEAD'], { cwd: worktreePath })
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'push'))
}
}
export async function gitPull(worktreePath: string): Promise<void> {
// Why: plain `git pull` uses the user's configured pull strategy (merge by
// default) so diverged branches reconcile instead of erroring out. Conflicts
// surface through the existing conflict-resolution flow.
try {
await gitExecFileAsync(['pull'], { cwd: worktreePath })
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'pull'))
}
}
export async function gitFetch(worktreePath: string): Promise<void> {
try {
await gitExecFileAsync(['fetch', '--prune'], { cwd: worktreePath })
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
}
}
+62
View File
@@ -0,0 +1,62 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { gitExecFileAsyncMock } = vi.hoisted(() => ({
gitExecFileAsyncMock: vi.fn()
}))
vi.mock('./runner', () => ({
gitExecFileAsync: gitExecFileAsyncMock
}))
import { getUpstreamStatus } from './upstream'
const missingTrackingRefError = new Error(
"fatal: ambiguous argument 'HEAD@{u}': unknown revision or path not in the working tree.\n" +
"Use '--' to separate paths from revisions, like this:\n" +
"'git <command> [<revision>...] -- [<file>...]'"
)
describe('getUpstreamStatus', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
})
it('returns upstream and ahead/behind counts when tracking is configured', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'origin/main\n' })
.mockResolvedValueOnce({ stdout: '2\t3\n' })
const result = await getUpstreamStatus('/repo')
expect(result).toEqual({
hasUpstream: true,
upstreamName: 'origin/main',
ahead: 2,
behind: 3
})
})
it('returns hasUpstream=false when upstream is missing', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('fatal: no upstream configured'))
const result = await getUpstreamStatus('/repo')
expect(result).toEqual({
hasUpstream: false,
ahead: 0,
behind: 0
})
})
it('returns hasUpstream=false when the configured tracking ref is missing', async () => {
gitExecFileAsyncMock.mockRejectedValueOnce(missingTrackingRefError)
const result = await getUpstreamStatus('/repo')
expect(result).toEqual({
hasUpstream: false,
ahead: 0,
behind: 0
})
})
})
+61
View File
@@ -0,0 +1,61 @@
import type { GitUpstreamStatus } from '../../shared/types'
import { isNoUpstreamError, normalizeGitErrorMessage } from '../../shared/git-remote-error'
import { gitExecFileAsync } from './runner'
export async function getUpstreamStatus(worktreePath: string): Promise<GitUpstreamStatus> {
try {
const { stdout: upstreamStdout } = await gitExecFileAsync(
['rev-parse', '--abbrev-ref', 'HEAD@{u}'],
{
cwd: worktreePath
}
)
const upstreamName = upstreamStdout.trim()
if (!upstreamName) {
return { hasUpstream: false, ahead: 0, behind: 0 }
}
const { stdout: countsStdout } = await gitExecFileAsync(
['rev-list', '--left-right', '--count', 'HEAD...@{u}'],
{
cwd: worktreePath
}
)
const tokens = countsStdout.trim().split(/\s+/)
if (tokens.length !== 2) {
// Why: 'rev-list --left-right --count HEAD...@{u}' must emit exactly two
// tokens; anything else (empty stdout, truncation, unexpected locale) is a
// real failure and must not be silently reported as "in sync" 0/0.
throw new Error(`Unexpected git rev-list output: ${JSON.stringify(countsStdout)}`)
}
const ahead = Number.parseInt(tokens[0]!, 10)
const behind = Number.parseInt(tokens[1]!, 10)
if (!Number.isFinite(ahead) || !Number.isFinite(behind) || ahead < 0 || behind < 0) {
throw new Error(`Unparseable git rev-list counts: ${JSON.stringify(countsStdout)}`)
}
return {
hasUpstream: true,
upstreamName,
ahead,
behind
}
} catch (error) {
// Why: we only swallow clearly-no-upstream signals — that's an expected
// state, not a failure. Other errors (auth, corruption, "not a git
// repository", sparse-checkout) should surface to the user so they can
// act on them. The shared isNoUpstreamError helper intentionally omits
// broad phrases like "no such branch" to avoid masking real errors.
if (isNoUpstreamError(error)) {
return {
hasUpstream: false,
ahead: 0,
behind: 0
}
}
// Why: parity with gitPush/gitPull/gitFetch — normalize before crossing
// the IPC boundary so renderers don't see execFile stderr preambles or local paths.
throw new Error(normalizeGitErrorMessage(error, 'upstream'))
}
}
+73
View File
@@ -11,6 +11,7 @@ import type {
GitBranchCompareResult,
GitConflictOperation,
GitDiffResult,
GitUpstreamStatus,
GitStatusResult,
MarkdownDocument,
SearchOptions,
@@ -37,6 +38,8 @@ import {
getBranchCompare,
getBranchDiff
} from '../git/status'
import { getUpstreamStatus } from '../git/upstream'
import { gitFetch, gitPull, gitPush } from '../git/remote'
import { getRemoteFileUrl } from '../git/repo'
import {
resolveAuthorizedPath,
@@ -546,6 +549,76 @@ export function registerFilesystemHandlers(store: Store): void {
}
)
ipcMain.handle(
'git:upstreamStatus',
async (
_event,
args: { worktreePath: string; connectionId?: string }
): Promise<GitUpstreamStatus> => {
if (args.connectionId) {
const provider = getSshGitProvider(args.connectionId)
if (!provider) {
throw new Error(`No git provider for connection "${args.connectionId}"`)
}
return provider.getUpstreamStatus(args.worktreePath)
}
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
return getUpstreamStatus(worktreePath)
}
)
ipcMain.handle(
'git:fetch',
async (_event, args: { worktreePath: string; connectionId?: string }): Promise<void> => {
if (args.connectionId) {
const provider = getSshGitProvider(args.connectionId)
if (!provider) {
throw new Error(`No git provider for connection "${args.connectionId}"`)
}
return provider.fetchRemote(args.worktreePath)
}
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
await gitFetch(worktreePath)
}
)
ipcMain.handle(
'git:push',
async (
_event,
args: { worktreePath: string; publish?: boolean; connectionId?: string }
): Promise<void> => {
// Why: coerce to strict boolean at the IPC boundary so a malformed
// renderer payload (e.g. string 'false') can't silently enable
// --set-upstream mode. Mirrors the relay handler in src/relay/git-handler.ts.
const publish = args.publish === true
if (args.connectionId) {
const provider = getSshGitProvider(args.connectionId)
if (!provider) {
throw new Error(`No git provider for connection "${args.connectionId}"`)
}
return provider.pushBranch(args.worktreePath, publish)
}
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
await gitPush(worktreePath, publish)
}
)
ipcMain.handle(
'git:pull',
async (_event, args: { worktreePath: string; connectionId?: string }): Promise<void> => {
if (args.connectionId) {
const provider = getSshGitProvider(args.connectionId)
if (!provider) {
throw new Error(`No git provider for connection "${args.connectionId}"`)
}
return provider.pullBranch(args.worktreePath)
}
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
await gitPull(worktreePath)
}
)
ipcMain.handle(
'git:branchDiff',
async (
@@ -128,6 +128,39 @@ describe('SshGitProvider', () => {
expect(result).toEqual(compareResult)
})
it('getUpstreamStatus sends git.upstreamStatus request', async () => {
const upstreamResult = { hasUpstream: true, upstreamName: 'origin/main', ahead: 1, behind: 0 }
mux.request.mockResolvedValue(upstreamResult)
const result = await provider.getUpstreamStatus('/home/user/repo')
expect(mux.request).toHaveBeenCalledWith('git.upstreamStatus', {
worktreePath: '/home/user/repo'
})
expect(result).toEqual(upstreamResult)
})
it('pushBranch sends git.push request and forwards publish mode', async () => {
await provider.pushBranch('/home/user/repo', true)
expect(mux.request).toHaveBeenCalledWith('git.push', {
worktreePath: '/home/user/repo',
publish: true
})
})
it('pullBranch sends git.pull request', async () => {
await provider.pullBranch('/home/user/repo')
expect(mux.request).toHaveBeenCalledWith('git.pull', {
worktreePath: '/home/user/repo'
})
})
it('fetchRemote sends git.fetch request', async () => {
await provider.fetchRemote('/home/user/repo')
expect(mux.request).toHaveBeenCalledWith('git.fetch', {
worktreePath: '/home/user/repo'
})
})
it('getBranchDiff sends git.branchDiff request', async () => {
const diffs = [{ kind: 'text', originalContent: '', modifiedContent: 'new' }]
mux.request.mockResolvedValue(diffs)
+19
View File
@@ -6,6 +6,7 @@ import type {
GitDiffResult,
GitBranchCompareResult,
GitConflictOperation,
GitUpstreamStatus,
GitWorktreeInfo
} from '../../shared/types'
@@ -83,6 +84,24 @@ export class SshGitProvider implements IGitProvider {
})) as GitBranchCompareResult
}
async getUpstreamStatus(worktreePath: string): Promise<GitUpstreamStatus> {
return (await this.mux.request('git.upstreamStatus', {
worktreePath
})) as GitUpstreamStatus
}
async pushBranch(worktreePath: string, publish = false): Promise<void> {
await this.mux.request('git.push', { worktreePath, publish })
}
async pullBranch(worktreePath: string): Promise<void> {
await this.mux.request('git.pull', { worktreePath })
}
async fetchRemote(worktreePath: string): Promise<void> {
await this.mux.request('git.fetch', { worktreePath })
}
async getBranchDiff(
worktreePath: string,
baseRef: string,
+5
View File
@@ -5,6 +5,7 @@ import type {
GitDiffResult,
GitBranchCompareResult,
GitConflictOperation,
GitUpstreamStatus,
GitWorktreeInfo,
SearchOptions,
SearchResult
@@ -146,6 +147,10 @@ export type IGitProvider = {
discardChanges(worktreePath: string, filePath: string): Promise<void>
detectConflictOperation(worktreePath: string): Promise<GitConflictOperation>
getBranchCompare(worktreePath: string, baseRef: string): Promise<GitBranchCompareResult>
getUpstreamStatus(worktreePath: string): Promise<GitUpstreamStatus>
pushBranch(worktreePath: string, publish?: boolean): Promise<void>
pullBranch(worktreePath: string): Promise<void>
fetchRemote(worktreePath: string): Promise<void>
getBranchDiff(
worktreePath: string,
baseRef: string,
+12
View File
@@ -20,6 +20,7 @@ import type {
GitConflictOperation,
GitDiffResult,
GitStatusResult,
GitUpstreamStatus,
GitHubAssignableUser,
GitHubPRFile,
GitHubPRFileContents,
@@ -915,6 +916,17 @@ export type PreloadApi = {
baseRef: string
connectionId?: string
}) => Promise<GitBranchCompareResult>
upstreamStatus: (args: {
worktreePath: string
connectionId?: string
}) => Promise<GitUpstreamStatus>
fetch: (args: { worktreePath: string; connectionId?: string }) => Promise<void>
push: (args: {
worktreePath: string
publish?: boolean
connectionId?: string
}) => Promise<void>
pull: (args: { worktreePath: string; connectionId?: string }) => Promise<void>
branchDiff: (args: {
worktreePath: string
compare: {
+14
View File
@@ -17,6 +17,7 @@ import type {
GitHubAssignableUser,
GitHubCommentResult,
GitHubWorkItem,
GitUpstreamStatus,
GhosttyImportPreview,
ListWorkItemsResult,
MemorySnapshot,
@@ -1523,6 +1524,19 @@ const api = {
baseRef: string
connectionId?: string
}): Promise<unknown> => ipcRenderer.invoke('git:branchCompare', args),
upstreamStatus: (args: {
worktreePath: string
connectionId?: string
}): Promise<GitUpstreamStatus> => ipcRenderer.invoke('git:upstreamStatus', args),
fetch: (args: { worktreePath: string; connectionId?: string }): Promise<void> =>
ipcRenderer.invoke('git:fetch', args),
push: (args: {
worktreePath: string
publish?: boolean
connectionId?: string
}): Promise<void> => ipcRenderer.invoke('git:push', args),
pull: (args: { worktreePath: string; connectionId?: string }): Promise<void> =>
ipcRenderer.invoke('git:pull', args),
branchDiff: (args: {
worktreePath: string
compare: { baseRef: string; baseOid: string; headOid: string; mergeBase: string }
+122
View File
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
import { describe, expect, it, beforeEach, afterEach } from 'vitest'
import { GitHandler } from './git-handler'
import { RelayContext } from './context'
@@ -43,6 +44,10 @@ describe('GitHandler', () => {
expect(methods).toContain('git.discard')
expect(methods).toContain('git.conflictOperation')
expect(methods).toContain('git.branchCompare')
expect(methods).toContain('git.upstreamStatus')
expect(methods).toContain('git.fetch')
expect(methods).toContain('git.push')
expect(methods).toContain('git.pull')
expect(methods).toContain('git.branchDiff')
expect(methods).toContain('git.listWorktrees')
expect(methods).toContain('git.addWorktree')
@@ -245,6 +250,123 @@ describe('GitHandler', () => {
})
})
describe('remote operations', () => {
it('returns upstream divergence for tracked branches', async () => {
gitInit(tmpDir)
writeFileSync(path.join(tmpDir, 'base.txt'), 'base')
gitCommit(tmpDir, 'initial')
const result = (await dispatcher.callRequest('git.upstreamStatus', {
worktreePath: tmpDir
})) as { hasUpstream: boolean; upstreamName?: string; ahead: number; behind: number }
expect(result.hasUpstream).toBe(false)
expect(result.ahead).toBe(0)
expect(result.behind).toBe(0)
})
it('reports ahead/behind counts against a real upstream remote', async () => {
// Why: the upstream branch exists but isn't configured — exercise the
// full path through `git rev-parse HEAD@{u}` + `rev-list --left-right`
// so a future refactor can't silently break the happy-path roundtrip
// the no-upstream test doesn't cover.
const bareDir = mkdtempSync(path.join(tmpdir(), 'relay-git-bare-'))
try {
execFileSync('git', ['init', '--bare'], { cwd: bareDir, stdio: 'pipe' })
gitInit(tmpDir)
writeFileSync(path.join(tmpDir, 'base.txt'), 'base')
gitCommit(tmpDir, 'initial')
const firstSha = execFileSync('git', ['rev-parse', 'HEAD'], {
cwd: tmpDir,
encoding: 'utf-8'
}).trim()
const branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], {
cwd: tmpDir,
encoding: 'utf-8'
}).trim()
execFileSync('git', ['remote', 'add', 'origin', bareDir], {
cwd: tmpDir,
stdio: 'pipe'
})
execFileSync('git', ['push', '--set-upstream', 'origin', branch], {
cwd: tmpDir,
stdio: 'pipe'
})
// Add two local commits (ahead=2), then reset behind the remote tip
// and add one different commit so we end up ahead=1, behind=0 vs.
// upstream; then reset to first commit to produce behind=1 ahead=0.
writeFileSync(path.join(tmpDir, 'ahead1.txt'), 'a1')
gitCommit(tmpDir, 'ahead1')
writeFileSync(path.join(tmpDir, 'ahead2.txt'), 'a2')
gitCommit(tmpDir, 'ahead2')
// Push so remote is at ahead2 (so after we reset below, we are behind).
execFileSync('git', ['push', 'origin', branch], { cwd: tmpDir, stdio: 'pipe' })
// Reset local back to the first commit: 0 ahead, 2 behind.
execFileSync('git', ['reset', '--hard', firstSha], { cwd: tmpDir, stdio: 'pipe' })
const result = (await dispatcher.callRequest('git.upstreamStatus', {
worktreePath: tmpDir
})) as { hasUpstream: boolean; upstreamName?: string; ahead: number; behind: number }
expect(result.hasUpstream).toBe(true)
expect(result.upstreamName).toBe(`origin/${branch}`)
expect(result.ahead).toBe(0)
expect(result.behind).toBe(2)
} finally {
await fs.rm(bareDir, { recursive: true, force: true })
}
})
it('fetches from a configured remote without throwing', async () => {
const bareDir = mkdtempSync(path.join(tmpdir(), 'relay-git-bare-'))
try {
execFileSync('git', ['init', '--bare'], { cwd: bareDir, stdio: 'pipe' })
gitInit(tmpDir)
writeFileSync(path.join(tmpDir, 'base.txt'), 'base')
gitCommit(tmpDir, 'initial')
const branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], {
cwd: tmpDir,
encoding: 'utf-8'
}).trim()
execFileSync('git', ['remote', 'add', 'origin', bareDir], {
cwd: tmpDir,
stdio: 'pipe'
})
execFileSync('git', ['push', '--set-upstream', 'origin', branch], {
cwd: tmpDir,
stdio: 'pipe'
})
await expect(
dispatcher.callRequest('git.fetch', { worktreePath: tmpDir })
).resolves.not.toThrow()
// FETCH_HEAD is created by any successful fetch, confirming the
// remote was actually contacted (not just silently no-op'd).
await expect(fs.access(path.join(tmpDir, '.git', 'FETCH_HEAD'))).resolves.toBeUndefined()
} finally {
await fs.rm(bareDir, { recursive: true, force: true })
}
})
it('rethrows upstreamStatus failures that are not "no upstream configured"', async () => {
// Why: the handler's catch is narrowed to only swallow the expected
// "no upstream" signal. A non-repo path should surface its error rather
// than silently returning hasUpstream=false, which would mask auth or
// corruption failures in production.
const nonRepoDir = path.join(tmpDir, 'not-a-repo')
await fs.mkdir(nonRepoDir, { recursive: true })
await expect(
dispatcher.callRequest('git.upstreamStatus', { worktreePath: nonRepoDir })
).rejects.toThrow(/not a git repository/i)
})
})
describe('listWorktrees', () => {
it('lists worktrees for a repo', async () => {
gitInit(tmpDir)
+103
View File
@@ -14,6 +14,7 @@ import {
} from './git-handler-ops'
import { commitChangesRelay, addWorktreeOp, removeWorktreeOp } from './git-handler-worktree-ops'
import { detectConflictOperation, getStatusOp } from './git-handler-status-ops'
import { normalizeGitErrorMessage, isNoUpstreamError } from '../shared/git-remote-error'
const execFileAsync = promisify(execFile)
const MAX_GIT_BUFFER = 10 * 1024 * 1024
@@ -40,6 +41,10 @@ export class GitHandler {
this.dispatcher.onRequest('git.discard', (p) => this.discard(p))
this.dispatcher.onRequest('git.conflictOperation', (p) => this.conflictOperation(p))
this.dispatcher.onRequest('git.branchCompare', (p) => this.branchCompare(p))
this.dispatcher.onRequest('git.upstreamStatus', (p) => this.upstreamStatus(p))
this.dispatcher.onRequest('git.fetch', (p) => this.fetch(p))
this.dispatcher.onRequest('git.push', (p) => this.push(p))
this.dispatcher.onRequest('git.pull', (p) => this.pull(p))
this.dispatcher.onRequest('git.branchDiff', (p) => this.branchDiff(p))
this.dispatcher.onRequest('git.listWorktrees', (p) => this.listWorktrees(p))
this.dispatcher.onRequest('git.addWorktree', (p) => this.addWorktree(p))
@@ -193,6 +198,104 @@ export class GitHandler {
})
}
private async upstreamStatus(params: Record<string, unknown>) {
const worktreePath = params.worktreePath as string
this.context.validatePath(worktreePath)
try {
const { stdout: upstreamStdout } = await this.git(
['rev-parse', '--abbrev-ref', 'HEAD@{u}'],
worktreePath
)
const upstreamName = upstreamStdout.trim()
if (!upstreamName) {
return { hasUpstream: false, ahead: 0, behind: 0 }
}
const { stdout: countsStdout } = await this.git(
['rev-list', '--left-right', '--count', 'HEAD...@{u}'],
worktreePath
)
const tokens = countsStdout.trim().split(/\s+/)
if (tokens.length !== 2) {
// Why: 'rev-list --left-right --count HEAD...@{u}' must emit exactly two
// tokens; anything else (empty stdout, SSH transport truncation, unexpected
// locale) is a real failure and must not be silently reported as "in sync" 0/0.
throw new Error(`Unexpected git rev-list output: ${JSON.stringify(countsStdout)}`)
}
const ahead = Number.parseInt(tokens[0]!, 10)
const behind = Number.parseInt(tokens[1]!, 10)
if (!Number.isFinite(ahead) || !Number.isFinite(behind) || ahead < 0 || behind < 0) {
throw new Error(`Unparseable git rev-list counts: ${JSON.stringify(countsStdout)}`)
}
return {
hasUpstream: true,
upstreamName,
ahead,
behind
}
} catch (error) {
// Why: we only swallow the 'no upstream configured' error — that's an
// expected state, not a failure. Other errors (auth, corruption, network)
// should surface to the user so they can act on them.
if (isNoUpstreamError(error)) {
return { hasUpstream: false, ahead: 0, behind: 0 }
}
// Why: match fetch/push/pull normalization so execFile preamble and local
// paths don't leak to the renderer.
throw new Error(normalizeGitErrorMessage(error, 'upstream'))
}
}
private async fetch(params: Record<string, unknown>) {
const worktreePath = params.worktreePath as string
this.context.validatePath(worktreePath)
try {
await this.git(['fetch', '--prune'], worktreePath)
} catch (error) {
// Why: mirror the local gitFetch normalization so SSH users see the same
// actionable messages instead of raw git stderr (which varies across
// versions/locales and may embed credentials).
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
}
}
private async push(params: Record<string, unknown>) {
const worktreePath = params.worktreePath as string
this.context.validatePath(worktreePath)
// Why: always pass --set-upstream (mirrors src/main/git/remote.ts).
// Orca's worktrees initially track the BASE ref (origin/main) because
// they're created via `git worktree add --track -b <name> <dir>
// <baseRef>` — without --set-upstream the local branch keeps tracking
// the base after the first push, so ahead/behind via @{u} measures
// "ahead of base" instead of "ahead of remote branch", and the UI's
// primary button never rotates from "Push" to "Commit". The `publish`
// flag is preserved in the param shape for IPC compatibility but is no
// longer load-bearing. On an already-published branch --set-upstream is
// a no-op for the tracking config.
void params.publish
try {
await this.git(['push', '--set-upstream', 'origin', 'HEAD'], worktreePath)
} catch (error) {
// Why: mirror the local gitPush normalization so SSH users see the same
// "non-fast-forward / pull first" guidance instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'push'))
}
}
private async pull(params: Record<string, unknown>) {
const worktreePath = params.worktreePath as string
this.context.validatePath(worktreePath)
// Why: plain `git pull` uses the user's configured pull strategy (merge by
// default) so diverged branches reconcile instead of erroring out.
try {
await this.git(['pull'], worktreePath)
} catch (error) {
// Why: mirror the local gitPull normalization so SSH users see the same
// actionable messages instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'pull'))
}
}
private async branchDiff(params: Record<string, unknown>) {
const worktreePath = params.worktreePath as string
this.context.validatePath(worktreePath)
@@ -0,0 +1,166 @@
import { describe, expect, it, vi } from 'vitest'
import { RefreshCw } from 'lucide-react'
import { CommitArea } from './SourceControl'
import { Button } from '@/components/ui/button'
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
// Why: split out from CommitArea.test.tsx so each file stays under the
// project's max-lines budget. These tests cover the chevron spinner
// behaviour for dropdown-only ops (Fetch) and the no-double-spin guard
// when the primary button already hosts the in-flight indicator.
type ReactElementLike = {
type: unknown
props: Record<string, unknown>
}
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
if (node == null || typeof node === 'string' || typeof node === 'number') {
return
}
if (Array.isArray(node)) {
node.forEach((entry) => visit(entry, cb))
return
}
const element = node as ReactElementLike
cb(element)
if (element.props?.children) {
visit(element.props.children, cb)
}
}
function findButtons(node: unknown): ReactElementLike[] {
const buttons: ReactElementLike[] = []
visit(node, (entry) => {
if (entry.type === Button) {
buttons.push(entry)
}
})
return buttons
}
function buttonHasSpinner(button: ReactElementLike): boolean {
let found = false
visit(button, (entry) => {
if (entry.type === RefreshCw) {
found = true
}
})
return found
}
function primaryHasSpinner(node: unknown): boolean {
const buttons = findButtons(node)
if (buttons.length === 0) {
throw new Error('primary button not found')
}
return buttonHasSpinner(buttons[0]!)
}
function chevronHasSpinner(node: unknown): boolean {
const buttons = findButtons(node)
if (buttons.length < 2) {
throw new Error('chevron button not found')
}
return buttonHasSpinner(buttons[1]!)
}
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 1,
hasUnstagedChanges: false,
hasMessage: true,
hasUnresolvedConflicts: false,
isCommitting: false,
isRemoteOperationActive: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
...overrides
}
}
function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
const inputs = buildInputs(overrides)
return {
commitMessage: 'feat: add commit area',
commitError: null as string | null,
isCommitting: inputs.isCommitting,
isRemoteOperationActive: inputs.isRemoteOperationActive,
inFlightRemoteOpKind: inputs.inFlightRemoteOpKind ?? null,
primaryAction: resolvePrimaryAction(inputs),
dropdownItems: resolveDropdownItems(inputs),
onCommitMessageChange: vi.fn(),
onPrimaryAction: vi.fn(),
onDropdownAction: vi.fn() as (kind: DropdownActionKind) => void
}
}
describe('CommitArea chevron spinner', () => {
// Why: when the primary can't host the in-flight op (Fetch is the
// canonical case — it's dropdown-only) the click would otherwise be
// silent: the toast only fires on failure and a no-op fetch leaves
// upstream counts unchanged. Spinning the chevron gives the user
// immediate "yes, your click did something" feedback.
it('spins the chevron while a dropdown Fetch is in flight', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
const element = CommitArea(props)
expect(chevronHasSpinner(element)).toBe(true)
expect(primaryHasSpinner(element)).toBe(false)
})
// Why: avoid double-spinning. When the primary is already spinning for
// an op it hosts (e.g. user clicked Push from the dropdown and the
// primary mirrors "Push"), the chevron stays as a chevron — one
// spinner per button surface, anchored to the action the label names.
it('does not spin the chevron when the primary already hosts the in-flight op', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'push'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(true)
expect(chevronHasSpinner(element)).toBe(false)
})
// Why: a dropdown Sync from a Push-natural state mirrors onto the
// primary (label flips to "Sync"), so the primary already carries the
// spinner. The chevron should not double-spin in that case.
it('does not spin the chevron when a dropdown op is mirrored onto the primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'sync'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(true)
expect(chevronHasSpinner(element)).toBe(false)
})
// Why: the plain-Commit primary is the scenario from the original Fetch
// bug — empty message + dropdown Fetch left both buttons static. The
// chevron must spin so the user sees feedback even though the primary
// can't (showing a spinner on a disabled "Commit" would mis-narrate).
it('spins the chevron when a dropdown remote op runs while the primary is plain Commit', () => {
const props = baseProps({
stagedCount: 1,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(false)
expect(chevronHasSpinner(element)).toBe(true)
})
})
@@ -0,0 +1,129 @@
import { describe, expect, it, vi } from 'vitest'
import { ArrowDown, ArrowDownUp, ArrowUp, CloudUpload } from 'lucide-react'
import { CommitArea } from './SourceControl'
import { Button } from '@/components/ui/button'
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
// Why: split out from CommitArea.test.tsx so each file stays under the
// project's max-lines budget. These tests cover the directional-icon
// mapping for primary action kinds (push / pull / sync / publish); the
// commit-checkmark and core CommitArea behaviour live in the sibling file.
type ReactElementLike = {
type: unknown
props: Record<string, unknown>
}
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
if (node == null || typeof node === 'string' || typeof node === 'number') {
return
}
if (Array.isArray(node)) {
node.forEach((entry) => visit(entry, cb))
return
}
const element = node as ReactElementLike
cb(element)
if (element.props?.children) {
visit(element.props.children, cb)
}
}
function findPrimaryButton(node: unknown): ReactElementLike {
const buttons: ReactElementLike[] = []
visit(node, (entry) => {
if (entry.type === Button) {
buttons.push(entry)
}
})
if (buttons.length === 0) {
throw new Error('primary button not found')
}
return buttons[0]
}
function primaryHasIcon(node: unknown, icon: unknown): boolean {
const primary = findPrimaryButton(node)
let found = false
visit(primary, (entry) => {
if (entry.type === icon) {
found = true
}
})
return found
}
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 1,
hasUnstagedChanges: false,
hasMessage: true,
hasUnresolvedConflicts: false,
isCommitting: false,
isRemoteOperationActive: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
...overrides
}
}
function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
const inputs = buildInputs(overrides)
return {
commitMessage: 'feat: add commit area',
commitError: null as string | null,
isCommitting: inputs.isCommitting,
isRemoteOperationActive: inputs.isRemoteOperationActive,
inFlightRemoteOpKind: inputs.inFlightRemoteOpKind ?? null,
primaryAction: resolvePrimaryAction(inputs),
dropdownItems: resolveDropdownItems(inputs),
onCommitMessageChange: vi.fn(),
onPrimaryAction: vi.fn(),
onDropdownAction: vi.fn() as (kind: DropdownActionKind) => void
}
}
// Why: each remote primary kind is anchored by a directional icon — Push
// ↑, Pull ↓, Sync ↕, Publish ☁︎↑ — so the verb's direction is visible at a
// glance and the slot doesn't read as a row of identical pills.
describe('CommitArea primary action icons', () => {
it('renders an up-arrow on a Push primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, ArrowUp)).toBe(true)
})
it('renders a down-arrow on a Pull primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 1 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, ArrowDown)).toBe(true)
})
it('renders a bidirectional arrow on a Sync primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 1 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, ArrowDownUp)).toBe(true)
})
it('renders a cloud-up icon on a Publish primary', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }
})
const element = CommitArea(props)
expect(primaryHasIcon(element, CloudUpload)).toBe(true)
})
})
@@ -1,6 +1,9 @@
import { describe, expect, it, vi } from 'vitest'
import { Check, RefreshCw } from 'lucide-react'
import { CommitArea } from './SourceControl'
import { Button } from '@/components/ui/button'
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
type ReactElementLike = {
type: unknown
@@ -35,26 +38,64 @@ function findTextarea(node: unknown): ReactElementLike {
return found
}
function findCommitButton(node: unknown): ReactElementLike {
let found: ReactElementLike | null = null
// Why: the split button renders two Button instances back-to-back — the
// primary action and the chevron trigger. The primary is always the first
// Button encountered in a depth-first walk, so we key on that position.
function findPrimaryButton(node: unknown): ReactElementLike {
const buttons: ReactElementLike[] = []
visit(node, (entry) => {
if (entry.type === Button) {
found = entry
buttons.push(entry)
}
})
if (!found) {
throw new Error('commit button not found')
if (buttons.length === 0) {
throw new Error('primary button not found')
}
return buttons[0]
}
function primaryHasSpinner(node: unknown): boolean {
const primary = findPrimaryButton(node)
let found = false
visit(primary, (entry) => {
if (entry.type === RefreshCw) {
found = true
}
})
return found
}
function primaryHasCheck(node: unknown): boolean {
const primary = findPrimaryButton(node)
let found = false
visit(primary, (entry) => {
if (entry.type === Check) {
found = true
}
})
return found
}
function hasText(node: unknown, text: string): boolean {
let found = false
visit(node, (entry) => {
const children = entry.props?.children
if (typeof children === 'string' && children.includes(text)) {
found = true
const walk = (value: unknown): void => {
if (typeof value === 'string') {
if (value.includes(text)) {
found = true
}
return
}
if (Array.isArray(value)) {
value.forEach(walk)
return
}
const element = value as ReactElementLike | null
if (element && typeof element === 'object' && 'props' in element) {
walk(element.props?.children)
}
}
visit(node, (entry) => {
walk(entry.props?.children)
})
return found
}
@@ -63,50 +104,79 @@ function flushPromises(): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, 0))
}
const baseProps = {
stagedCount: 1,
hasUnresolvedConflicts: false,
commitMessage: 'feat: add commit area',
commitError: null as string | null,
isCommitting: false,
onCommitMessageChange: vi.fn(),
onCommitSuccess: vi.fn()
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 1,
hasUnstagedChanges: false,
hasMessage: true,
hasUnresolvedConflicts: false,
isCommitting: false,
isRemoteOperationActive: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
...overrides
}
}
function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
const inputs = buildInputs(overrides)
return {
commitMessage: 'feat: add commit area',
commitError: null as string | null,
isCommitting: inputs.isCommitting,
isRemoteOperationActive: inputs.isRemoteOperationActive,
inFlightRemoteOpKind: inputs.inFlightRemoteOpKind ?? null,
primaryAction: resolvePrimaryAction(inputs),
dropdownItems: resolveDropdownItems(inputs),
onCommitMessageChange: vi.fn(),
onPrimaryAction: vi.fn(),
onDropdownAction: vi.fn() as (kind: DropdownActionKind) => void
}
}
describe('CommitArea', () => {
it('disables commit button when no staged files', () => {
const element = CommitArea({ ...baseProps, stagedCount: 0 })
const button = findCommitButton(element)
it('disables the primary button when no staged files', () => {
const element = CommitArea(baseProps({ stagedCount: 0 }))
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
})
it('disables commit button when message is empty', () => {
const element = CommitArea({ ...baseProps, commitMessage: ' ' })
const button = findCommitButton(element)
it('disables the primary button when the commit message is empty', () => {
const props = baseProps({ hasMessage: false })
const element = CommitArea({ ...props, commitMessage: ' ' })
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
})
it('disables commit button when unresolved conflicts exist', () => {
const element = CommitArea({ ...baseProps, hasUnresolvedConflicts: true })
const button = findCommitButton(element)
it('disables the primary button when unresolved conflicts exist', () => {
const element = CommitArea(baseProps({ hasUnresolvedConflicts: true }))
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
})
it('enables commit button with staged files, message, and no conflicts', () => {
const element = CommitArea(baseProps)
const button = findCommitButton(element)
it('enables the primary button when staged + message + no conflicts', () => {
const element = CommitArea(baseProps())
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(false)
})
it('triggers commit when the button is clicked', () => {
const onCommitSuccess = vi.fn()
const element = CommitArea({ ...baseProps, onCommitSuccess })
const button = findCommitButton(element)
it('fires onPrimaryAction when the primary button is clicked', () => {
const onPrimaryAction = vi.fn()
const element = CommitArea({ ...baseProps(), onPrimaryAction })
const button = findPrimaryButton(element)
;(button.props.onClick as () => void)()
expect(onCommitSuccess).toHaveBeenCalledTimes(1)
expect(onPrimaryAction).toHaveBeenCalledTimes(1)
})
it('clears message and keeps error hidden after successful commit lifecycle', async () => {
it('keeps the textarea enabled while the commit is in flight', () => {
const element = CommitArea({
...baseProps({ isCommitting: true }),
isCommitting: true
})
const textarea = findTextarea(element)
expect(textarea.props.disabled).toBeFalsy()
})
it('clears the message and keeps error hidden after a successful commit lifecycle', async () => {
let commitMessage = 'feat: add commit area'
let commitError: string | null = null
let isCommitting = false
@@ -119,18 +189,25 @@ describe('CommitArea', () => {
isCommitting = false
})
const render = () =>
CommitArea({
...baseProps,
const render = () => {
const inputs = buildInputs({
hasMessage: commitMessage.trim().length > 0,
isCommitting
})
return CommitArea({
...baseProps(),
commitMessage,
commitError,
isCommitting,
onCommitSuccess: () => {
primaryAction: resolvePrimaryAction(inputs),
dropdownItems: resolveDropdownItems(inputs),
onPrimaryAction: () => {
void runCommit()
}
})
}
const button = findCommitButton(render())
const button = findPrimaryButton(render())
;(button.props.onClick as () => void)()
await flushPromises()
@@ -140,7 +217,7 @@ describe('CommitArea', () => {
expect(runCommit).toHaveBeenCalledTimes(1)
})
it('preserves message and shows error after failed commit lifecycle', async () => {
it('preserves the message and shows the error after a failed commit lifecycle', async () => {
const initialMessage = 'feat: add commit area'
let commitMessage = initialMessage
let commitError: string | null = null
@@ -154,18 +231,25 @@ describe('CommitArea', () => {
isCommitting = false
})
const render = () =>
CommitArea({
...baseProps,
const render = () => {
const inputs = buildInputs({
hasMessage: commitMessage.trim().length > 0,
isCommitting
})
return CommitArea({
...baseProps(),
commitMessage,
commitError,
isCommitting,
onCommitSuccess: () => {
primaryAction: resolvePrimaryAction(inputs),
dropdownItems: resolveDropdownItems(inputs),
onPrimaryAction: () => {
void runCommit()
}
})
}
const button = findCommitButton(render())
const button = findPrimaryButton(render())
;(button.props.onClick as () => void)()
await flushPromises()
@@ -175,14 +259,141 @@ describe('CommitArea', () => {
expect(runCommit).toHaveBeenCalledTimes(1)
})
it('locks the button while commit is in flight', () => {
const element = CommitArea({ ...baseProps, isCommitting: true })
const button = findCommitButton(element)
it('locks the primary button while the commit is in flight', () => {
const props = baseProps({ isCommitting: true })
const element = CommitArea({ ...props, isCommitting: true })
const button = findPrimaryButton(element)
expect(button.props.disabled).toBe(true)
})
it('shows an inline error message when commit fails', () => {
const element = CommitArea({ ...baseProps, commitError: 'pre-commit hook failed' })
it('shows an inline error message when the commit fails', () => {
const element = CommitArea({ ...baseProps(), commitError: 'pre-commit hook failed' })
expect(hasText(element, 'pre-commit hook failed')).toBe(true)
})
it('keeps the primary button labelled Commit when the tree is staged, even with commits to push', () => {
// Why: the primary never compounds ("Commit & Push"). Users commit first,
// then the primary rotates to Push. Compound flows remain in the dropdown,
// so we check only the primary button, not the whole tree.
const props = baseProps({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
const element = CommitArea(props)
const primary = findPrimaryButton(element)
expect(hasText(primary, 'Commit')).toBe(true)
expect(hasText(primary, 'Commit & Push')).toBe(false)
expect(hasText(primary, 'Commit & Sync')).toBe(false)
expect(hasText(primary, 'Commit & Publish')).toBe(false)
})
// Why: fetching from the dropdown sets isRemoteOperationActive, but the
// primary button is plain "Commit" — painting a spinner on it told the
// user their commit was running. The spinner must track the primary
// action itself, not every background remote op.
it('does not show a spinner on a plain Commit primary when a dropdown remote op is running', () => {
const props = baseProps({
// stagedCount + no message resolves to plain 'commit' kind (disabled
// because the message is empty). This is the scenario the user hit:
// a Commit button that falsely claimed their commit was in flight
// while a dropdown-triggered Fetch was the actual work.
stagedCount: 1,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isCommitting: false,
isRemoteOperationActive: true
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(false)
})
it('shows a spinner on a Commit primary while the commit itself is in flight', () => {
const props = baseProps({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
isCommitting: true
})
const element = CommitArea({ ...props, isCommitting: true })
expect(primaryHasSpinner(element)).toBe(true)
})
it('shows a spinner on a remote primary (Push) while the matching remote op is active', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'push'
})
const element = CommitArea(props)
expect(primaryHasSpinner(element)).toBe(true)
})
// Why: regression — when the user picks Sync from the dropdown, the
// primary button must mirror the action they triggered (label "Sync",
// spinner on Sync) instead of leaving a stale "Push" with a spinner that
// claims a different operation is running.
it('mirrors a dropdown-triggered Sync on the primary button while it runs', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
// Pre-click state: ahead=3, behind=0 → primary's natural label is Push.
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'sync'
})
const element = CommitArea(props)
const primary = findPrimaryButton(element)
expect(hasText(primary, 'Sync')).toBe(true)
expect(hasText(primary, 'Push')).toBe(false)
expect(primaryHasSpinner(element)).toBe(true)
})
// Why: Fetch is dropdown-only (never the primary's label). Spinning the
// primary on a fetch would mis-narrate "Push is running" while the actual
// work is fetching. Primary keeps its natural label, disabled, no spinner.
it('does not spin or relabel the primary when a dropdown Fetch is in flight', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 },
isRemoteOperationActive: true,
inFlightRemoteOpKind: 'fetch'
})
const element = CommitArea(props)
const primary = findPrimaryButton(element)
expect(hasText(primary, 'Push')).toBe(true)
expect(primary.props.disabled).toBe(true)
expect(primaryHasSpinner(element)).toBe(false)
})
// Why: the leading checkmark anchors the affirmative Commit verb so the
// button doesn't read like just another remote-state label sharing the
// slot (Push / Pull / Sync / Publish). Decorative — verified by
// presence/absence rather than label text.
it('renders a leading checkmark on a Commit primary', () => {
const element = CommitArea(baseProps())
expect(primaryHasCheck(element)).toBe(true)
})
it('omits the checkmark when the primary is a remote action', () => {
const props = baseProps({
stagedCount: 0,
hasMessage: false,
upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 }
})
const element = CommitArea(props)
expect(primaryHasCheck(element)).toBe(false)
})
// Why: while the commit is in flight the spinner replaces any leading
// icon so the user gets a single, unambiguous progress signal.
it('replaces the checkmark with a spinner while the commit is in flight', () => {
const props = baseProps({ isCommitting: true })
const element = CommitArea({ ...props, isCommitting: true })
expect(primaryHasSpinner(element)).toBe(true)
expect(primaryHasCheck(element)).toBe(false)
})
})
@@ -1,7 +1,11 @@
/* eslint-disable max-lines */
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import {
ArrowDown,
ArrowDownUp,
ArrowUp,
ChevronDown,
CloudUpload,
Minus,
Plus,
RefreshCw,
@@ -32,6 +36,23 @@ import { cn } from '@/lib/utils'
import { isFolderRepo } from '../../../../shared/repo-kind'
import { Tooltip, TooltipTrigger, TooltipContent, TooltipProvider } from '@/components/ui/tooltip'
import { Button } from '@/components/ui/button'
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuSeparator,
DropdownMenuTrigger
} from '@/components/ui/dropdown-menu'
import {
resolvePrimaryAction,
type PrimaryAction,
type RemoteOpKind
} from './source-control-primary-action'
import {
resolveDropdownItems,
type DropdownActionKind,
type DropdownEntry
} from './source-control-dropdown-items'
import { BulkActionBar } from './BulkActionBar'
import { useSourceControlSelection, type FlatEntry } from './useSourceControlSelection'
import {
@@ -70,6 +91,7 @@ import type {
GitConflictKind,
GitConflictOperation,
GitStatusEntry,
GitUpstreamStatus,
PRInfo
} from '../../../../shared/types'
import { STATUS_COLORS, STATUS_LABELS } from './status-display'
@@ -88,6 +110,22 @@ const STATUS_ICONS: Record<
copied: FilePlus
}
// Why: directional signifiers ahead of each primary action label. Commit
// (✓) is affirmative; Push (↑) and Pull (↓) point in the direction data
// flows; Sync (↕) is bidirectional; Publish gets a cloud-up to distinguish
// the first-time publish from a subsequent push. Keeping the mapping
// outside the render function avoids reallocating it on every render.
const PRIMARY_ICONS: Record<
PrimaryAction['kind'],
React.ComponentType<{ className?: string; 'aria-hidden'?: boolean | 'true' | 'false' }>
> = {
commit: Check,
push: ArrowUp,
pull: ArrowDown,
sync: ArrowDownUp,
publish: CloudUpload
}
// Why: unstaged ("Changes") is listed first so that conflict files — which
// are assigned area:'unstaged' by the parser — appear above "Staged Changes".
// This keeps unresolved conflicts visible at the top of the list where the
@@ -101,6 +139,10 @@ const SECTION_LABELS: Record<(typeof SECTION_ORDER)[number], string> = {
const BRANCH_REFRESH_INTERVAL_MS = 5000
// Why: the pure state-machine logic now lives in
// ./source-control-primary-action.ts. It is imported directly by callers
// (tests and other components) instead of going through this module.
type CommitDraftsByWorktree = Record<string, string>
export function readCommitDraftForWorktree(
@@ -144,12 +186,19 @@ function SourceControlInner(): React.JSX.Element {
const gitConflictOperationByWorktree = useAppStore((s) => s.gitConflictOperationByWorktree)
const gitBranchChangesByWorktree = useAppStore((s) => s.gitBranchChangesByWorktree)
const gitBranchCompareSummaryByWorktree = useAppStore((s) => s.gitBranchCompareSummaryByWorktree)
const remoteStatusesByWorktree = useAppStore((s) => s.remoteStatusesByWorktree)
const isRemoteOperationActive = useAppStore((s) => s.isRemoteOperationActive)
const inFlightRemoteOpKind = useAppStore((s) => s.inFlightRemoteOpKind)
const prCache = useAppStore((s) => s.prCache)
const fetchPRForBranch = useAppStore((s) => s.fetchPRForBranch)
const updateRepo = useAppStore((s) => s.updateRepo)
const beginGitBranchCompareRequest = useAppStore((s) => s.beginGitBranchCompareRequest)
const setGitBranchCompareResult = useAppStore((s) => s.setGitBranchCompareResult)
const setGitStatus = useAppStore((s) => s.setGitStatus)
const fetchUpstreamStatus = useAppStore((s) => s.fetchUpstreamStatus)
const pushBranch = useAppStore((s) => s.pushBranch)
const pullBranch = useAppStore((s) => s.pullBranch)
const syncBranch = useAppStore((s) => s.syncBranch)
const fetchBranch = useAppStore((s) => s.fetchBranch)
const revealInExplorer = useAppStore((s) => s.revealInExplorer)
const trackConflictPath = useAppStore((s) => s.trackConflictPath)
const openDiff = useAppStore((s) => s.openDiff)
@@ -245,6 +294,13 @@ function SourceControlInner(): React.JSX.Element {
const conflictOperation = activeWorktreeId
? (gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown')
: 'unknown'
// Why: leave undefined until fetchUpstreamStatus resolves for this worktree.
// Substituting a synthetic { hasUpstream: false } flashes "Publish Branch"
// on every worktree switch — resolvePrimaryAction treats it as an
// unpublished branch until the real status lands a moment later.
const remoteStatus: GitUpstreamStatus | undefined = activeWorktreeId
? remoteStatusesByWorktree[activeWorktreeId]
: undefined
const rightSidebarOpen = useAppStore((s) => s.rightSidebarOpen)
// Why: gate polling on both the active tab AND the sidebar being open.
// The sidebar now stays mounted when closed (for performance), so without
@@ -426,17 +482,19 @@ function SourceControlInner(): React.JSX.Element {
// still runs.
}, [activeWorktreeId])
const handleCommit = useCallback(async (): Promise<void> => {
// Why: returns true on success so compound actions ("Commit & Push" etc.)
// can skip the follow-up remote operation when the commit itself failed.
const handleCommit = useCallback(async (): Promise<boolean> => {
if (!activeWorktreeId || !worktreePath) {
return
return false
}
const message = commitMessage.trim()
if (!message || grouped.staged.length === 0 || unresolvedConflicts.length > 0) {
return
return false
}
if (commitInFlightRef.current[activeWorktreeId]) {
return
return false
}
commitInFlightRef.current[activeWorktreeId] = true
@@ -454,7 +512,7 @@ function SourceControlInner(): React.JSX.Element {
...prev,
[activeWorktreeId]: commitResult.error ?? 'Commit failed'
}))
return
return false
}
// Why: the textarea stays enabled during the in-flight commit (only the
@@ -472,37 +530,210 @@ function SourceControlInner(): React.JSX.Element {
return writeCommitDraftForWorktree(prev, activeWorktreeId, '')
})
setCommitErrors((prev) => ({ ...prev, [activeWorktreeId]: null }))
// Why: the commit already succeeded. If the follow-up status refresh fails
// (e.g., transient IPC error), log it but do NOT overwrite the cleared
// commitError with a misleading "Commit failed" — the existing status poll
// in useGitStatusPolling will refresh the UI shortly anyway.
try {
const status = await window.api.git.status({
worktreePath,
connectionId
})
setGitStatus(activeWorktreeId, status)
} catch (refreshError) {
console.error('[SourceControl] post-commit status refresh failed', refreshError)
// Why: flip branchSummary to 'loading' synchronously so the empty-state
// guard
// (!hasUncommittedEntries && branchSummary.status === 'ready' &&
// branchEntries.length === 0)
// doesn't briefly read true between setGitStatus clearing the
// uncommitted list and the next branchCompare poll landing the new
// commit. Without this flip "No changes on this branch" flashes for
// the full poll-interval window.
//
// Then fire-and-forget refreshBranchCompare so the "Committed on
// Branch" section repopulates as soon as the IPC returns instead of
// waiting up to 5 seconds for the next poll. Unawaited on purpose:
// compound flows (runCompoundCommitAction) need handleCommit to
// resolve immediately so the push step starts without delay. Errors
// here are best-effort — the polling tick will retry.
if (effectiveBaseRef) {
beginGitBranchCompareRequest(
activeWorktreeId,
`${activeWorktreeId}:${effectiveBaseRef}:${Date.now()}:post-commit`,
effectiveBaseRef
)
}
void refreshBranchCompareRef.current()
return true
} catch (error) {
setCommitErrors((prev) => ({
...prev,
[activeWorktreeId]: error instanceof Error ? error.message : 'Commit failed'
}))
return false
} finally {
setCommitInFlightByWorktree((prev) => ({ ...prev, [activeWorktreeId]: false }))
commitInFlightRef.current[activeWorktreeId] = false
}
}, [
activeWorktreeId,
beginGitBranchCompareRequest,
commitMessage,
effectiveBaseRef,
grouped.staged.length,
unresolvedConflicts.length,
setGitStatus,
worktreePath
])
// Why: a single dispatcher for every remote-only action the split button or
// chevron dropdown can trigger. Keeps the error-swallow pattern in one
// place — store slices already surface actionable toasts, so additional
// try/catch here would duplicate the notification.
const runRemoteAction = useCallback(
async (kind: 'push' | 'pull' | 'sync' | 'fetch' | 'publish'): Promise<void> => {
if (!activeWorktreeId || !worktreePath) {
return
}
const connectionId = getConnectionId(activeWorktreeId) ?? undefined
try {
if (kind === 'publish') {
await pushBranch(activeWorktreeId, worktreePath, true, connectionId)
return
}
if (kind === 'push') {
await pushBranch(activeWorktreeId, worktreePath, false, connectionId)
return
}
if (kind === 'pull') {
await pullBranch(activeWorktreeId, worktreePath, connectionId)
return
}
if (kind === 'fetch') {
await fetchBranch(activeWorktreeId, worktreePath, connectionId)
return
}
await syncBranch(activeWorktreeId, worktreePath, connectionId)
} catch {
// Why: remote action failures are surfaced by editor-slice actions to keep
// one consistent toast path and avoid duplicate notifications in the UI.
}
},
[activeWorktreeId, fetchBranch, pullBranch, pushBranch, syncBranch, worktreePath]
)
// Why: compound actions must commit first and only run the follow-up remote
// op when the commit succeeds. handleCommit's return value carries that
// signal — a failure leaves commitError populated and short-circuits here
// so we never push a commit the user didn't actually land. The primary
// button never takes this path (it always emits a single-action kind);
// compound flows are reached only from the dropdown, which offers
// 'commit_push' and 'commit_sync' (there is no 'Commit & Publish' row).
const runCompoundCommitAction = useCallback(
async (remoteKind: 'push' | 'sync'): Promise<void> => {
const ok = await handleCommit()
if (!ok) {
return
}
await runRemoteAction(remoteKind)
},
[handleCommit, runRemoteAction]
)
const hasUnstagedChanges = grouped.unstaged.length > 0 || grouped.untracked.length > 0
const primaryAction: PrimaryAction = useMemo(
() =>
resolvePrimaryAction({
stagedCount: grouped.staged.length,
hasUnstagedChanges,
hasMessage: commitMessage.trim().length > 0,
hasUnresolvedConflicts: unresolvedConflicts.length > 0,
isCommitting,
isRemoteOperationActive,
upstreamStatus: remoteStatus,
inFlightRemoteOpKind
}),
[
commitMessage,
grouped.staged.length,
hasUnstagedChanges,
isCommitting,
isRemoteOperationActive,
inFlightRemoteOpKind,
remoteStatus,
unresolvedConflicts.length
]
)
const dropdownItems: DropdownEntry[] = useMemo(
() =>
resolveDropdownItems({
stagedCount: grouped.staged.length,
hasUnstagedChanges,
hasMessage: commitMessage.trim().length > 0,
hasUnresolvedConflicts: unresolvedConflicts.length > 0,
isCommitting,
isRemoteOperationActive,
upstreamStatus: remoteStatus,
inFlightRemoteOpKind
}),
[
commitMessage,
grouped.staged.length,
hasUnstagedChanges,
isCommitting,
isRemoteOperationActive,
inFlightRemoteOpKind,
remoteStatus,
unresolvedConflicts.length
]
)
// Why: maps both the primary button click and any chevron dropdown item
// click to the right handler. Commit-ish kinds flow through handleCommit
// (which returns a boolean); compound actions use runCompoundCommitAction;
// pure remote actions go through runRemoteAction.
const handleActionInvoke = useCallback(
(kind: DropdownActionKind): void => {
switch (kind) {
case 'commit':
void handleCommit()
return
case 'commit_push':
void runCompoundCommitAction('push')
return
case 'commit_sync':
void runCompoundCommitAction('sync')
return
case 'push':
case 'pull':
case 'sync':
case 'fetch':
case 'publish':
void runRemoteAction(kind)
return
default: {
// Why: exhaustiveness check — if a new DropdownActionKind is added
// to the union, TypeScript will flag this assignment so we can't
// silently drop a case.
const _exhaustive: never = kind
void _exhaustive
}
}
},
[handleCommit, runCompoundCommitAction, runRemoteAction]
)
// Why: PrimaryActionKind is narrowed to the single-action kinds the
// primary can emit ('commit' | 'push' | 'pull' | 'sync' | 'publish') —
// compound commit_* kinds are dropdown-only. An exhaustive switch keeps
// the mapping honest: if a new PrimaryActionKind is added, TypeScript
// lights up the missing case instead of silently falling through.
const handlePrimaryClick = useCallback((): void => {
switch (primaryAction.kind) {
case 'commit':
case 'push':
case 'pull':
case 'sync':
case 'publish':
handleActionInvoke(primaryAction.kind)
return
default: {
const _exhaustive: never = primaryAction.kind
void _exhaustive
}
}
}, [handleActionInvoke, primaryAction.kind])
const handleOpenDiff = useCallback(
(entry: GitStatusEntry) => {
if (!activeWorktreeId || !worktreePath) {
@@ -748,6 +979,18 @@ function SourceControlInner(): React.JSX.Element {
return () => window.clearInterval(intervalId)
}, [activeWorktreeId, effectiveBaseRef, isBranchVisible, isFolder, worktreePath])
useEffect(() => {
// Why: gate on isBranchVisible so we don't spawn git processes while the
// sidebar is closed. Store-slice remote operations refresh upstream-status
// on success anyway, so the user's first sidebar open will show accurate
// state.
if (!activeWorktreeId || !worktreePath || isFolder || !isBranchVisible) {
return
}
const connectionId = getConnectionId(activeWorktreeId) ?? undefined
void fetchUpstreamStatus(activeWorktreeId, worktreePath, connectionId)
}, [activeWorktreeId, fetchUpstreamStatus, isBranchVisible, isFolder, worktreePath])
const toggleSection = useCallback((section: string) => {
setCollapsedSections((prev) => {
const next = new Set(prev)
@@ -1137,13 +1380,23 @@ function SourceControlInner(): React.JSX.Element {
/>
)}
{(scope === 'all' || scope === 'uncommitted') && hasUncommittedEntries && (
{/* Why: keep CommitArea mounted across all source-control states.
The split-button primary rotates through Push / Pull / Sync /
Publish on a clean tree and disables Commit with a "Nothing to
commit" tooltip when nothing is staged — gating on
hasUncommittedEntries (added by #1448 for the older Commit-only
design) would unmount the whole action surface on clean
worktrees and tear it down mid-commit when the staged list
clears. */}
{(scope === 'all' || scope === 'uncommitted') && (
<CommitArea
stagedCount={grouped.staged.length}
hasUnresolvedConflicts={unresolvedConflicts.length > 0}
commitMessage={commitMessage}
commitError={commitError}
isCommitting={isCommitting}
isRemoteOperationActive={isRemoteOperationActive}
inFlightRemoteOpKind={inFlightRemoteOpKind}
primaryAction={primaryAction}
dropdownItems={dropdownItems}
onCommitMessageChange={(value) => {
if (!activeWorktreeId) {
return
@@ -1152,9 +1405,8 @@ function SourceControlInner(): React.JSX.Element {
writeCommitDraftForWorktree(prev, activeWorktreeId, value)
)
}}
onCommitSuccess={() => {
void handleCommit()
}}
onPrimaryAction={handlePrimaryClick}
onDropdownAction={handleActionInvoke}
/>
)}
@@ -1409,47 +1661,62 @@ const SourceControl = React.memo(SourceControlInner)
export default SourceControl
type CommitAreaProps = {
stagedCount: number
hasUnresolvedConflicts: boolean
commitMessage: string
commitError: string | null
isCommitting: boolean
isRemoteOperationActive: boolean
inFlightRemoteOpKind: RemoteOpKind | null
primaryAction: PrimaryAction
dropdownItems: DropdownEntry[]
onCommitMessageChange: (message: string) => void
onCommitSuccess: () => void
onPrimaryAction: () => void
onDropdownAction: (kind: DropdownActionKind) => void
}
export function CommitArea({
stagedCount,
hasUnresolvedConflicts,
commitMessage,
commitError,
isCommitting,
isRemoteOperationActive,
inFlightRemoteOpKind,
primaryAction,
dropdownItems,
onCommitMessageChange,
onCommitSuccess
onPrimaryAction,
onDropdownAction
}: CommitAreaProps): React.JSX.Element {
// Why: cap at 12 rows so a pasted multi-page commit message doesn't push
// the Commit button off-screen. The textarea keeps `resize-none` (matching
// the existing style) — the browser scrolls internally past 12 rows.
const rows = Math.min(12, Math.max(2, commitMessage.split('\n').length))
const hasMessage = commitMessage.trim().length > 0
const isCommitDisabled =
isCommitting || !hasMessage || stagedCount === 0 || hasUnresolvedConflicts
// 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-
// 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
// signal there. Commit still spins on isCommitting because that path
// doesn't go through inFlightRemoteOpKind.
const showSpinner =
primaryAction.kind === 'commit'
? isCommitting
: isRemoteOperationActive && primaryAction.kind === inFlightRemoteOpKind
// Why: when the primary doesn't host the in-flight op (e.g. Fetch, or any
// dropdown action that mismatches the primary's natural label) the click
// would otherwise be silent — the toast only fires on failure and a
// no-op fetch leaves status counts unchanged. Spinning the chevron gives
// the user immediate feedback that the action they picked is running,
// while still leaving the menu reachable to read the disabled-row
// tooltips.
const showChevronSpinner = (isCommitting || isRemoteOperationActive) && !showSpinner
// Why: when the button is disabled, the title surfaces the reason so the
// user doesn't have to guess why Commit is greyed out. Part-2 may extend
// this into a split button (primary action + dropdown for Push / Sync /
// Commit & Push); the label stays as a plain "Commit" here so the shape
// lines up cleanly with the forthcoming "Remote Updates" section beneath it.
let disabledReason: string | undefined
if (isCommitting) {
disabledReason = 'Commit in progress…'
} else if (hasUnresolvedConflicts) {
disabledReason = 'Resolve conflicts before committing'
} else if (stagedCount === 0) {
disabledReason = 'Stage at least one file to commit'
} else if (!hasMessage) {
disabledReason = 'Enter a commit message to commit'
}
// Why: each primary-kind label is anchored by a directional icon so the
// affirmative Commit (✓) reads distinctly from the remote-state labels
// sharing this slot — Push (↑), Pull (↓), Sync (↕), Publish (☁︎↑). The
// icon is decorative; the label and title attribute carry the meaning
// for assistive tech.
const PrimaryIcon = PRIMARY_ICONS[primaryAction.kind]
return (
<div className="px-3 pb-2">
@@ -1462,20 +1729,81 @@ export function CommitArea({
aria-describedby={commitError ? 'commit-area-error' : undefined}
className="mt-0.5 w-full resize-none rounded-md border border-border bg-background px-2 py-1.5 text-xs text-foreground outline-none placeholder:text-muted-foreground/70 focus-visible:ring-1 focus-visible:ring-ring"
/>
{/* Why: match the "Squash and merge" button in PRActions
(size="xs", px-3 text-[11px]) so the sidebar has a consistent
action-button shape across Source Control and Checks. */}
<Button
type="button"
size="xs"
disabled={isCommitDisabled}
onClick={() => onCommitSuccess()}
className="w-full px-3 text-[11px]"
title={disabledReason}
>
{isCommitting && <RefreshCw className="size-3.5 animate-spin" />}
Commit
</Button>
{/* 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. */}
<div className="flex items-stretch">
{/* Why: match the "Squash and merge" button in PRActions
(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. */}
<Button
type="button"
size="xs"
disabled={primaryAction.disabled}
onClick={() => onPrimaryAction()}
className="flex-1 rounded-r-none px-3 text-[11px]"
title={primaryAction.title}
>
{showSpinner ? (
<RefreshCw className="size-3.5 animate-spin" />
) : (
<PrimaryIcon className="size-3.5" aria-hidden="true" />
)}
{primaryAction.label}
</Button>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
type="button"
size="xs"
className={cn(
'rounded-l-none border-l border-primary-foreground/20 px-1.5 shrink-0',
// Why: mirror the primary's disabled dimming so the split
// button reads as one unit when Commit is unavailable. The
// chevron itself stays clickable — its dropdown exposes
// independently-gated remote actions (push / fetch / pull)
// that are still valid when the primary is disabled.
primaryAction.disabled && 'opacity-50'
)}
aria-label="More commit and remote actions"
title="More actions"
>
{showChevronSpinner ? (
<RefreshCw className="size-3.5 animate-spin" />
) : (
<ChevronDown className="size-3.5" />
)}
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="min-w-[14rem]">
{dropdownItems.map((entry, index) =>
entry.kind === 'separator' ? (
<DropdownMenuSeparator key={`sep-${index}`} />
) : (
<DropdownMenuItem
key={entry.kind}
disabled={entry.disabled}
title={entry.title}
onSelect={(event) => {
if (entry.disabled) {
event.preventDefault()
return
}
onDropdownAction(entry.kind)
}}
>
{entry.label}
</DropdownMenuItem>
)
)}
</DropdownMenuContent>
</DropdownMenu>
</div>
{commitError && (
// Why: role="alert" + aria-live="polite" lets screen readers announce
// commit failures; the id ties the message to the textarea via
@@ -0,0 +1,191 @@
import { describe, expect, it } from 'vitest'
import { resolveDropdownItems } from './source-control-dropdown-items'
import 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.
function inputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 0,
hasUnstagedChanges: false,
hasMessage: false,
hasUnresolvedConflicts: false,
isCommitting: false,
isRemoteOperationActive: false,
upstreamStatus: undefined,
...overrides
}
}
describe('resolveDropdownItems', () => {
it('renders every row — Commit through Publish — for a staged, tracked, ahead+behind branch', () => {
const items = resolveDropdownItems(
inputs({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: true, ahead: 2, behind: 3 }
})
)
const kinds = items.map((item) => item.kind)
expect(kinds).toEqual([
'commit',
'commit_push',
'commit_sync',
'separator',
'push',
'pull',
'sync',
'fetch',
'publish'
])
})
it('disables compound commit actions when no staged files', () => {
const items = resolveDropdownItems(
inputs({ upstreamStatus: { hasUpstream: true, ahead: 1, behind: 0 } })
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.commit.disabled).toBe(true)
expect(byKind.commit_push.disabled).toBe(true)
expect(byKind.commit_sync.disabled).toBe(true)
})
it('disables push actions but keeps Fetch enabled when branch has no upstream', () => {
const items = resolveDropdownItems(
inputs({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 }
})
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.disabled).toBe(true)
expect(byKind.commit_push.disabled).toBe(true)
expect(byKind.publish.disabled).toBe(false)
expect(byKind.fetch.disabled).toBe(false)
})
it('disables Publish Branch when branch already has an upstream', () => {
const items = resolveDropdownItems(
inputs({
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }
})
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.publish.disabled).toBe(true)
})
it('renders counts on action labels when > 0', () => {
const items = resolveDropdownItems(
inputs({ upstreamStatus: { hasUpstream: true, ahead: 3, behind: 2 } })
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.label).toBe('Push (3)')
expect(byKind.pull.label).toBe('Pull (2)')
expect(byKind.sync.label).toBe('Sync (↓2 ↑3)')
})
it('omits counts from labels when ahead/behind are 0', () => {
const items = resolveDropdownItems(
inputs({ upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 } })
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.label).toBe('Push')
expect(byKind.pull.label).toBe('Pull')
expect(byKind.sync.label).toBe('Sync')
})
it('locks every item while a remote op is running', () => {
const items = resolveDropdownItems(
inputs({
isRemoteOperationActive: true,
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: true, ahead: 2, behind: 3 }
})
)
for (const entry of items) {
if (entry.kind !== 'separator') {
expect(entry.disabled).toBe(true)
}
}
})
it('disables remote rows with a loading tooltip when upstreamStatus is undefined', () => {
// Why: mirrors the primary-action guard — while fetchUpstreamStatus is in
// flight we must not let the user click Publish on an already-tracked
// branch (which would re-run `git push -u` and clobber the upstream).
const items = resolveDropdownItems(
inputs({ stagedCount: 1, hasMessage: true, upstreamStatus: undefined })
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
const loadingBlocked = [
'commit_push',
'commit_sync',
'push',
'pull',
'sync',
'fetch',
'publish'
] as const
for (const kind of loadingBlocked) {
expect(byKind[kind].disabled).toBe(true)
expect(byKind[kind].title).toBe('Checking branch status…')
}
// Commit itself does not depend on upstream — it remains enabled when
// staged + message are present and no commit is in flight.
expect(byKind.commit.disabled).toBe(false)
})
it('keeps Fetch enabled and surfaces publish-first tooltips when upstream is absent', () => {
// Why: sibling to the upstreamStatus=undefined test above. Once the fetch
// resolves to hasUpstream=false, the dropdown should explain that the
// user needs to publish first (rather than leaving the loading copy).
const items = resolveDropdownItems(
inputs({ upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 } })
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.push.title).toBe('Publish the branch first to push commits')
expect(byKind.pull.title).toBe('Publish the branch first to pull commits')
expect(byKind.sync.title).toBe('Publish the branch first to sync commits')
expect(byKind.fetch.title).toBe('Fetch from remote without merging')
expect(byKind.fetch.disabled).toBe(false)
expect(byKind.publish.title).toBe('Publish this branch to origin')
expect(byKind.publish.disabled).toBe(false)
})
it('omits counts from compound commit labels even when ahead/behind are nonzero', () => {
// Why: the commit itself changes ahead/behind, so pre-commit counts would
// be stale the moment the action fires. Plain Push/Pull/Sync continue to
// carry counts because no commit is interposed.
const items = resolveDropdownItems(
inputs({
stagedCount: 1,
hasMessage: true,
upstreamStatus: { hasUpstream: true, ahead: 2, behind: 3 }
})
)
const byKind = Object.fromEntries(
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
)
expect(byKind.commit_push.label).toBe('Commit & Push')
expect(byKind.commit_sync.label).toBe('Commit & Sync')
// Sanity check: plain counterparts still carry counts.
expect(byKind.push.label).toBe('Push (2)')
expect(byKind.sync.label).toBe('Sync (↓3 ↑2)')
})
})
@@ -0,0 +1,212 @@
// Why: split from source-control-primary-action because the primary and dropdown are independent derivations with different priority ladders; together they exceed the max-lines budget and tangle unrelated concerns.
import type { PrimaryActionInputs } from './source-control-primary-action'
export type DropdownActionKind =
| 'commit'
| 'commit_push'
| 'commit_sync'
| 'push'
| 'pull'
| 'sync'
| 'fetch'
| 'publish'
export type DropdownItem = {
kind: DropdownActionKind
label: string
title: string
disabled: boolean
}
export type DropdownSeparator = { kind: 'separator' }
export type DropdownEntry = DropdownItem | DropdownSeparator
function describePushCount(ahead: number): string {
return `Push ${ahead} commit${ahead === 1 ? '' : 's'}`
}
function describePullCount(behind: number): string {
return `Pull ${behind} commit${behind === 1 ? '' : 's'}`
}
function describeSyncCounts(ahead: number, behind: number): string {
return `Pull ${behind}, push ${ahead}`
}
function formatCountLabel(base: string, count: number): string {
return count > 0 ? `${base} (${count})` : base
}
function formatSyncLabel(base: string, ahead: number, behind: number): string {
if (ahead === 0 && behind === 0) {
return base
}
return `${base} (↓${behind}${ahead})`
}
/**
* Resolve the chevron dropdown items. Every item is always rendered so the
* menu shape stays stable across states; inapplicable rows are disabled
* with a tooltip reason rather than hidden.
*/
export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry[] {
const {
stagedCount,
hasMessage,
hasUnresolvedConflicts,
isCommitting,
isRemoteOperationActive,
upstreamStatus
} = inputs
const hasStaged = stagedCount > 0
// Why: mirror the primary-action guard. When upstreamStatus is undefined,
// fetchUpstreamStatus hasn't resolved for this worktree yet. Collapsing that
// to hasUpstream=false would re-enable Publish Branch on an already-tracked
// branch during the post-worktree-switch transient window, and a click there
// would re-run `git push -u` and clobber the real upstream. Every
// upstream-dependent row disables itself while loading so the primary
// button's stable-frame guarantee extends to the dropdown.
const upstreamLoading = upstreamStatus === undefined
const hasUpstream = upstreamStatus?.hasUpstream ?? false
const ahead = upstreamStatus?.ahead ?? 0
const behind = upstreamStatus?.behind ?? 0
// Why: any in-flight commit or remote operation should lock the whole menu.
// A running push shouldn't let a second pull/sync click queue up behind it
// on a stale status snapshot.
const globalBusy = isCommitting || isRemoteOperationActive
const commitDisabledReason = (() => {
if (hasUnresolvedConflicts) {
return 'Resolve conflicts before committing'
}
if (!hasStaged) {
return 'Stage at least one file to commit'
}
if (!hasMessage) {
return 'Enter a commit message to commit'
}
return null
})()
const canCommit = !globalBusy && commitDisabledReason === null
const commitItem: DropdownItem = {
kind: 'commit',
label: 'Commit',
title: commitDisabledReason ?? 'Commit staged changes',
disabled: !canCommit
}
// Why: compound commit labels omit counts because the commit itself changes
// ahead/behind — surfacing pre-commit numbers would be misleading (e.g.
// "Commit & Push (2)" would still read "2" after the commit lands at 3).
// On an unpublished branch, Commit & Push is unavailable: the user must
// Publish Branch first (offered via the primary action), after which
// Commit & Push becomes enabled. Tooltips mirror pushItem/syncItem copy
// so the "publish first" instruction is consistent across the menu.
const commitPushTitle = upstreamLoading
? 'Checking branch status…'
: !hasUpstream
? 'Publish the branch first to push commits'
: (commitDisabledReason ?? 'Commit staged changes and push')
const commitPushItem: DropdownItem = {
kind: 'commit_push',
label: 'Commit & Push',
title: commitPushTitle,
disabled: globalBusy || upstreamLoading || !hasUpstream || commitDisabledReason !== null
}
const commitSyncTitle = (() => {
if (upstreamLoading) {
return 'Checking branch status…'
}
if (!hasUpstream) {
// Why: mirror pushItem/syncItem — direct the user to Publish Branch
// (the primary action on an unpublished branch) rather than naming a
// nonexistent compound action.
return 'Publish the branch first to sync commits'
}
if (behind === 0) {
return 'Nothing to pull — use Commit & Push instead'
}
return commitDisabledReason ?? 'Commit, then pull and push'
})()
const commitSyncItem: DropdownItem = {
kind: 'commit_sync',
label: 'Commit & Sync',
title: commitSyncTitle,
disabled:
globalBusy || upstreamLoading || !hasUpstream || behind === 0 || commitDisabledReason !== null
}
const pushItem: DropdownItem = {
kind: 'push',
label: formatCountLabel('Push', ahead),
title: upstreamLoading
? 'Checking branch status…'
: !hasUpstream
? 'Publish the branch first to push commits'
: ahead === 0
? 'Nothing to push'
: describePushCount(ahead),
disabled: globalBusy || upstreamLoading || !hasUpstream || ahead === 0
}
const pullItem: DropdownItem = {
kind: 'pull',
label: formatCountLabel('Pull', behind),
title: upstreamLoading
? 'Checking branch status…'
: !hasUpstream
? 'Publish the branch first to pull commits'
: behind === 0
? 'Nothing to pull'
: describePullCount(behind),
disabled: globalBusy || upstreamLoading || !hasUpstream || behind === 0
}
const syncItem: DropdownItem = {
kind: 'sync',
label: formatSyncLabel('Sync', ahead, behind),
title: upstreamLoading
? 'Checking branch status…'
: !hasUpstream
? 'Publish the branch first to sync commits'
: ahead === 0 && behind === 0
? 'Branch is up to date'
: describeSyncCounts(ahead, behind),
disabled: globalBusy || upstreamLoading || !hasUpstream || (ahead === 0 && behind === 0)
}
const fetchItem: DropdownItem = {
kind: 'fetch',
label: 'Fetch',
title: upstreamLoading ? 'Checking branch status…' : 'Fetch from remote without merging',
disabled: globalBusy || upstreamLoading
}
const publishItem: DropdownItem = {
kind: 'publish',
label: 'Publish Branch',
title: upstreamLoading
? 'Checking branch status…'
: hasUpstream
? 'Branch is already published'
: 'Publish this branch to origin',
disabled: globalBusy || upstreamLoading || hasUpstream
}
return [
commitItem,
commitPushItem,
commitSyncItem,
{ kind: 'separator' },
pushItem,
pullItem,
syncItem,
fetchItem,
publishItem
]
}
@@ -0,0 +1,256 @@
import { describe, expect, it } from 'vitest'
import { 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.
function inputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
return {
stagedCount: 0,
hasUnstagedChanges: 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', () => {
it('returns a disabled Commit while a commit is in flight', () => {
const result = resolvePrimaryAction(
inputs({ isCommitting: true, stagedCount: 1, hasMessage: true })
)
expect(result).toEqual({
kind: 'commit',
label: 'Commit',
title: 'Commit in progress…',
disabled: true
})
})
it('keeps the contextual label but disables it while a remote op is in flight', () => {
const result = resolvePrimaryAction(
inputs({
isRemoteOperationActive: true,
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 3 }
})
)
expect(result).toEqual({
kind: 'pull',
label: 'Pull',
title: 'Remote operation in progress…',
disabled: true
})
})
// Why: when the user picks an action from the dropdown that doesn't match
// the primary's natural label, the primary must mirror the user-triggered
// action (label + kind) so the spinner narrates the right thing. Without
// this, picking "Sync" from the dropdown while the primary reads "Push"
// would spin a "Push" button that is not actually pushing.
it('mirrors the in-flight remote op kind on the primary while a remote op runs', () => {
const result = resolvePrimaryAction(
inputs({
isRemoteOperationActive: true,
// Pre-click natural state would resolve to Push (ahead-only).
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
inFlightRemoteOpKind: 'sync'
})
)
expect(result).toEqual({
kind: 'sync',
label: 'Sync',
title: 'Sync in progress…',
disabled: true
})
})
it('mirrors an in-flight Pull on the primary even when natural label is Push', () => {
const result = resolvePrimaryAction(
inputs({
isRemoteOperationActive: true,
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
inFlightRemoteOpKind: 'pull'
})
)
expect(result.kind).toBe('pull')
expect(result.label).toBe('Pull')
expect(result.title).toBe('Pull in progress…')
expect(result.disabled).toBe(true)
})
it('keeps the natural Publish label and tooltip when an in-flight Publish matches', () => {
// Why: when the in-flight kind matches the natural primary kind we
// preserve the candidate's full label (the natural state-machine row
// owns the wording) rather than overriding to a stripped-down version.
const result = resolvePrimaryAction(
inputs({
isRemoteOperationActive: true,
upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 },
inFlightRemoteOpKind: 'publish'
})
)
expect(result.kind).toBe('publish')
expect(result.label).toBe('Publish Branch')
expect(result.title).toBe('Remote operation in progress…')
expect(result.disabled).toBe(true)
})
// Why: Fetch is dropdown-only and never appears as the primary's label.
// When fetch is in flight, the primary must keep its natural label and
// tooltip so the button doesn't claim "Fetch" is a primary action — and
// the CommitArea spinner suppression hangs off the kind mismatch.
it('keeps the natural primary label when an in-flight Fetch is dropdown-only', () => {
const result = resolvePrimaryAction(
inputs({
isRemoteOperationActive: true,
upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 },
inFlightRemoteOpKind: 'fetch'
})
)
expect(result).toEqual({
kind: 'push',
label: 'Push',
title: 'Remote operation in progress…',
disabled: true
})
})
it('blocks commits while unresolved conflicts exist', () => {
const result = resolvePrimaryAction(
inputs({ hasUnresolvedConflicts: true, stagedCount: 2, hasMessage: true })
)
expect(result).toEqual({
kind: 'commit',
label: 'Commit',
title: 'Resolve conflicts before committing',
disabled: true
})
})
// Why: the primary button never compounds ("Commit & Push" etc.) — it
// always reads "Commit" whenever there are staged files with a message,
// regardless of remote state. Compound flows remain available from the
// dropdown; after the commit lands, the primary naturally rotates to
// Push / Sync / Publish Branch.
it('returns plain Commit for staged+message regardless of upstream state', () => {
const upstreams = [
undefined,
{ hasUpstream: false as const, ahead: 0, behind: 0 },
{ hasUpstream: true as const, ahead: 0, behind: 0 },
{ hasUpstream: true as const, ahead: 3, behind: 0 },
{ hasUpstream: true as const, ahead: 2, behind: 1 },
{ hasUpstream: true as const, ahead: 0, behind: 4 }
]
for (const upstreamStatus of upstreams) {
const result = resolvePrimaryAction(
inputs({ stagedCount: 1, hasMessage: true, upstreamStatus })
)
expect(result.kind).toBe('commit')
expect(result.label).toBe('Commit')
expect(result.disabled).toBe(false)
}
})
it('disables Commit with a message-needed hint when staged but no message', () => {
const result = resolvePrimaryAction(inputs({ stagedCount: 1, hasMessage: false }))
expect(result).toEqual({
kind: 'commit',
label: 'Commit',
title: 'Enter a commit message to commit',
disabled: true
})
})
it('returns Publish Branch on a clean tree when no upstream exists', () => {
const result = resolvePrimaryAction(
inputs({ upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 } })
)
expect(result).toEqual({
kind: 'publish',
label: 'Publish Branch',
title: 'Publish this branch to origin',
disabled: false
})
})
it('returns Sync when clean + tracked + diverged both ways', () => {
const result = resolvePrimaryAction(
inputs({ upstreamStatus: { hasUpstream: true, ahead: 2, behind: 3 } })
)
expect(result).toEqual({
kind: 'sync',
label: 'Sync',
title: 'Pull 3, push 2',
disabled: false
})
})
it('returns Pull when clean + behind-only', () => {
const result = resolvePrimaryAction(
inputs({ upstreamStatus: { hasUpstream: true, ahead: 0, behind: 4 } })
)
expect(result.kind).toBe('pull')
expect(result.label).toBe('Pull')
expect(result.title).toBe('Pull 4 commits')
})
it('uses singular copy for a single-commit pull', () => {
const result = resolvePrimaryAction(
inputs({ upstreamStatus: { hasUpstream: true, ahead: 0, behind: 1 } })
)
expect(result.title).toBe('Pull 1 commit')
})
it('returns Push when clean + ahead-only', () => {
const result = resolvePrimaryAction(
inputs({ upstreamStatus: { hasUpstream: true, ahead: 3, behind: 0 } })
)
expect(result).toEqual({
kind: 'push',
label: 'Push',
title: 'Push 3 commits',
disabled: false
})
})
it('returns a disabled up-to-date Commit when tracked branch is clean and in sync', () => {
const result = resolvePrimaryAction(inputs({ upstreamStatus: upstreamInSync }))
expect(result).toEqual({
kind: 'commit',
label: 'Commit',
title: 'Nothing to commit. Branch is up to date.',
disabled: true
})
})
it('asks the user to stage files when unstaged changes exist on an in-sync branch', () => {
const result = resolvePrimaryAction(
inputs({ hasUnstagedChanges: true, upstreamStatus: upstreamInSync })
)
expect(result).toEqual({
kind: 'commit',
label: 'Commit',
title: 'Stage at least one file to commit',
disabled: true
})
})
it('returns a disabled Commit when clean and upstream status not yet resolved', () => {
const result = resolvePrimaryAction(inputs())
expect(result).toEqual({
kind: 'commit',
label: 'Commit',
title: 'Stage at least one file to commit',
disabled: true
})
})
})
@@ -0,0 +1,245 @@
// Why: split from the combined primary+dropdown module because the primary and dropdown are independent derivations with different priority ladders; together they exceed the max-lines budget and tangle unrelated concerns.
import type { GitUpstreamStatus } from '../../../../shared/types'
// Why: this module owns the pure state-machine logic for the Source Control
// primary action (split button). Keeping the logic outside the React component
// makes it straightforward to unit-test each row of the priority table without
// spinning up a renderer.
// Why: the primary button collapses to one-label-per-action. Compound
// kinds ('commit_push', 'commit_sync', 'commit_publish') live in
// DropdownActionKind only — never on the primary — so they are not part
// of this union. Narrowing the type here is load-bearing: it lets
// `handlePrimaryClick` switch exhaustively over only the kinds the
// primary can actually emit, and it kills the compound-commit branch in
// the isRemoteOperationActive tooltip below at compile time.
export type PrimaryActionKind = 'commit' | 'push' | 'pull' | 'sync' | 'publish'
// Why: the in-flight remote op tracker stores which action the user actually
// triggered, so the primary button can mirror that label/spinner instead of
// claiming a stale or unrelated operation is running. 'fetch' is included
// because Fetch participates in the busy flag, but it is intentionally NOT
// in PrimaryActionKind — Fetch is dropdown-only, so when fetch is in flight
// the primary keeps its natural label and CommitArea suppresses the spinner.
export type RemoteOpKind = 'push' | 'pull' | 'sync' | 'fetch' | 'publish'
export type PrimaryAction = {
kind: PrimaryActionKind
label: string
title: string
disabled: boolean
}
export type PrimaryActionInputs = {
stagedCount: number
hasUnstagedChanges: boolean
hasMessage: boolean
hasUnresolvedConflicts: boolean
isCommitting: boolean
isRemoteOperationActive: boolean
upstreamStatus: GitUpstreamStatus | undefined
// Why: which remote op is currently running, when one is. null when no
// remote op is in flight. Used by the in-flight branch below to mirror
// the user-triggered action on the primary button instead of leaving a
// stale label that no longer matches what the slice is doing.
inFlightRemoteOpKind?: RemoteOpKind | null
}
const PRIMARY_LABEL_BY_KIND: Record<Exclude<PrimaryActionKind, 'commit'>, string> = {
push: 'Push',
pull: 'Pull',
sync: 'Sync',
publish: 'Publish Branch'
}
function describePushCount(ahead: number): string {
return `Push ${ahead} commit${ahead === 1 ? '' : 's'}`
}
function describePullCount(behind: number): string {
return `Pull ${behind} commit${behind === 1 ? '' : 's'}`
}
function describeSyncCounts(ahead: number, behind: number): string {
return `Pull ${behind}, push ${ahead}`
}
/**
* Resolve the primary split-button action.
*
* Priority order mirrors the design-doc state machine:
* 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 staged files + message → plain "Commit" (compound flows live in
* the dropdown; after the commit lands, step 6 rotates the primary to
* the appropriate single remote action).
* 5. Has staged files + no message → disabled "Commit" with a reason.
* 6. 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
* stable frame until the real status lands — otherwise it would flash
* through "Publish Branch" on every worktree switch.
*/
export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction {
const {
stagedCount,
hasUnstagedChanges,
hasMessage,
hasUnresolvedConflicts,
isCommitting,
isRemoteOperationActive,
upstreamStatus,
inFlightRemoteOpKind
} = inputs
// 1. Commit in flight — lock the primary no matter what else is true.
if (isCommitting) {
return {
kind: 'commit',
label: 'Commit',
title: 'Commit in progress…',
disabled: true
}
}
// 2. Remote op in flight — disable the primary. When the in-flight op
// is a primary-eligible kind that doesn't match the primary's natural
// label, mirror the in-flight kind so the user sees the action they
// actually triggered (e.g. "Sync" when they picked Sync from the
// dropdown while the primary's natural state was "Push"). When the
// in-flight op matches the primary's natural kind we keep the natural
// label so its richer detail (counts like "Push 3 commits") survives.
// Fetch and unknown in-flight kinds leave the primary's natural label
// intact; CommitArea's spinner suppresses itself via the kind-mismatch
// check so a non-matching in-flight op doesn't visually claim the
// primary as its host.
if (isRemoteOperationActive) {
const candidate = resolvePrimaryAction({ ...inputs, isRemoteOperationActive: false })
const inFlightIsPrimaryKind =
inFlightRemoteOpKind === 'push' ||
inFlightRemoteOpKind === 'pull' ||
inFlightRemoteOpKind === 'sync' ||
inFlightRemoteOpKind === 'publish'
if (inFlightIsPrimaryKind && candidate.kind !== inFlightRemoteOpKind) {
const label = PRIMARY_LABEL_BY_KIND[inFlightRemoteOpKind]
return {
kind: inFlightRemoteOpKind,
label,
title: `${label} in progress…`,
disabled: true
}
}
// Why: when the candidate label is "Commit", the generic "remote
// operation in progress…" tooltip mismatches the visible label. Point
// the user at the fact that the commit will wait, keeping the label and
// the explanation consistent. Conflicts take precedence over the remote
// tooltip because resolving them is the only action the user can start
// while the remote op runs.
const title = hasUnresolvedConflicts
? 'Resolve conflicts before committing'
: candidate.kind === 'commit'
? 'Remote operation in progress — try again once it finishes'
: 'Remote operation in progress…'
return {
...candidate,
title,
disabled: true
}
}
// 3. Unresolved conflicts block any commit path.
if (hasUnresolvedConflicts) {
return {
kind: 'commit',
label: 'Commit',
title: 'Resolve conflicts before committing',
disabled: true
}
}
const hasStaged = stagedCount > 0
// 4. Has staged files + message → plain Commit. The primary button never
// compounds ("Commit & Push" etc.) — after the commit lands, the primary
// naturally rotates to the appropriate remote action (Push / Sync /
// Publish Branch) via step 6 below. Users who want the one-click
// compound flow can still reach it from the dropdown.
if (hasStaged && hasMessage) {
return {
kind: 'commit',
label: 'Commit',
title: 'Commit staged changes',
disabled: false
}
}
// 5. Has staged files but no message — user just needs to type something.
if (hasStaged && !hasMessage) {
return {
kind: 'commit',
label: 'Commit',
title: 'Enter a commit message to commit',
disabled: true
}
}
// 6. Clean tree + no staged files → adaptive remote action.
if (!upstreamStatus) {
return {
kind: 'commit',
label: 'Commit',
title: 'Stage at least one file to commit',
disabled: true
}
}
if (!upstreamStatus.hasUpstream) {
return {
kind: 'publish',
label: 'Publish Branch',
title: 'Publish this branch to origin',
disabled: false
}
}
if (upstreamStatus.ahead > 0 && upstreamStatus.behind > 0) {
return {
kind: 'sync',
label: 'Sync',
title: describeSyncCounts(upstreamStatus.ahead, upstreamStatus.behind),
disabled: false
}
}
if (upstreamStatus.behind > 0) {
return {
kind: 'pull',
label: 'Pull',
title: describePullCount(upstreamStatus.behind),
disabled: false
}
}
if (upstreamStatus.ahead > 0) {
return {
kind: 'push',
label: 'Push',
title: describePushCount(upstreamStatus.ahead),
disabled: false
}
}
// Clean + tracked + in sync — distinguish truly clean from work that still
// needs staging before commit can proceed.
return {
kind: 'commit',
label: 'Commit',
title: hasUnstagedChanges
? 'Stage at least one file to commit'
: 'Nothing to commit. Branch is up to date.',
disabled: true
}
}
@@ -13,6 +13,7 @@ export function useGitStatusPolling(): void {
const activeWorktreeId = useAppStore((s) => s.activeWorktreeId)
const fetchWorktrees = useAppStore((s) => s.fetchWorktrees)
const setGitStatus = useAppStore((s) => s.setGitStatus)
const fetchUpstreamStatus = useAppStore((s) => s.fetchUpstreamStatus)
const setConflictOperation = useAppStore((s) => s.setConflictOperation)
const conflictOperationByWorktree = useAppStore((s) => s.gitConflictOperationByWorktree)
const repoMap = useRepoMap()
@@ -55,10 +56,11 @@ export function useGitStatusPolling(): void {
connectionId
})) as GitStatusResult
setGitStatus(activeWorktreeId, status)
await fetchUpstreamStatus(activeWorktreeId, worktreePath, connectionId)
} catch {
// ignore
}
}, [activeRepoSupportsGit, activeWorktreeId, worktreePath, setGitStatus])
}, [activeRepoSupportsGit, activeWorktreeId, fetchUpstreamStatus, worktreePath, setGitStatus])
useEffect(() => {
void fetchStatus()
@@ -582,6 +582,439 @@ describe('createEditorSlice combined diff exclusions', () => {
})
})
describe('createEditorSlice remote branch actions', () => {
const gitStatusMock = vi.fn()
const gitUpstreamStatusMock = vi.fn()
const gitPushMock = vi.fn()
const gitPullMock = vi.fn()
const gitFetchMock = vi.fn()
beforeEach(() => {
toastErrorMock.mockReset()
gitStatusMock.mockReset()
gitUpstreamStatusMock.mockReset()
gitPushMock.mockReset()
gitPullMock.mockReset()
gitFetchMock.mockReset()
gitStatusMock.mockResolvedValue({ entries: [], conflictOperation: 'unknown' })
gitUpstreamStatusMock.mockResolvedValue({
hasUpstream: true,
upstreamName: 'origin/main',
ahead: 1,
behind: 0
})
// eslint-disable-next-line @typescript-eslint/no-explicit-any
;(globalThis as any).window = (globalThis as any).window ?? {}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
;(globalThis as any).window.api = {
git: {
status: gitStatusMock,
upstreamStatus: gitUpstreamStatusMock,
push: gitPushMock,
pull: gitPullMock,
fetch: gitFetchMock
}
}
})
it('stores upstream status per worktree', () => {
const store = createEditorStore()
store.getState().setUpstreamStatus('wt-1', {
hasUpstream: true,
upstreamName: 'origin/main',
ahead: 2,
behind: 1
})
expect(store.getState().remoteStatusesByWorktree['wt-1']).toEqual({
hasUpstream: true,
upstreamName: 'origin/main',
ahead: 2,
behind: 1
})
})
it('runs pull and refreshes status + upstream on success', async () => {
const store = createEditorStore()
store.getState().setGitStatus('wt-1', {
conflictOperation: 'unknown',
entries: [{ path: 'src/app.ts', status: 'modified', area: 'unstaged' }]
})
await store.getState().pullBranch('wt-1', '/repo')
expect(gitPullMock).toHaveBeenCalledWith({
worktreePath: '/repo',
connectionId: undefined
})
expect(toastErrorMock).not.toHaveBeenCalled()
})
it('surfaces a readable toast when pull reports local changes would be overwritten', async () => {
const store = createEditorStore()
gitPullMock.mockRejectedValueOnce(
new Error(
'error: Your local changes to the following files would be overwritten by merge:\n\tsrc/app.ts\nPlease commit your changes or stash them before you merge.\nAborting'
)
)
await expect(store.getState().pullBranch('wt-1', '/repo')).rejects.toThrow()
expect(toastErrorMock).toHaveBeenCalledWith(
'Pull blocked — commit or stash your local changes first.'
)
})
it('runs publish branch through push with publish=true', async () => {
// Why: pushBranch no longer awaits a post-op git status / upstream
// refresh. The 3s git-status poll and the upstream-status effect in the
// sidebar reconcile state shortly after the IPC returns; keeping the
// mutation tight stops compound flows from stalling between commit and
// push.
const store = createEditorStore()
await store.getState().pushBranch('wt-1', '/repo', true)
expect(gitPushMock).toHaveBeenCalledWith({
worktreePath: '/repo',
publish: true,
connectionId: undefined
})
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('preserves actionable publish errors and avoids refresh on failure', async () => {
const store = createEditorStore()
const publishError = new Error(
'Push rejected: remote has newer commits (non-fast-forward). Please pull or sync first.'
)
gitPushMock.mockRejectedValueOnce(publishError)
await expect(store.getState().pushBranch('wt-1', '/repo', true)).rejects.toThrow(
publishError.message
)
expect(toastErrorMock).toHaveBeenCalledWith(
'Push rejected — remote has changes. Pull first, then try again.'
)
expect(gitStatusMock).not.toHaveBeenCalled()
expect(gitUpstreamStatusMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('maps publish updates-were-rejected into a clean actionable toast', async () => {
const store = createEditorStore()
const publishError = new Error(
'Updates were rejected because the tip of your current branch is behind its remote counterpart.'
)
gitPushMock.mockRejectedValueOnce(publishError)
await expect(store.getState().pushBranch('wt-1', '/repo', true)).rejects.toThrow(
publishError.message
)
expect(toastErrorMock).toHaveBeenCalledWith(
'Push rejected — remote has changes. Pull first, then try again.'
)
expect(gitStatusMock).not.toHaveBeenCalled()
expect(gitUpstreamStatusMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('maps raw publish wrapper errors into a cleaner actionable toast', async () => {
const store = createEditorStore()
const rawPublishError = new Error(
'git push failed: Command failed: git push --set-upstream origin feature-branch\nremote: Repository not found.\nfatal: Authentication failed for https://github.com/acme/private-repo.git'
)
gitPushMock.mockRejectedValueOnce(rawPublishError)
await expect(store.getState().pushBranch('wt-1', '/repo', true)).rejects.toThrow(
rawPublishError.message
)
expect(toastErrorMock).toHaveBeenCalledWith(
'Publish Branch failed. Authentication failed for https://github.com/acme/private-repo.git. Check your remote access and try again.'
)
})
it('uses a fallback message for generic publish errors', async () => {
const store = createEditorStore()
const publishError = new Error('error: RPC failed; curl 56 GnuTLS recv error')
gitPushMock.mockRejectedValueOnce(publishError)
await expect(store.getState().pushBranch('wt-1', '/repo', true)).rejects.toThrow(
publishError.message
)
expect(toastErrorMock).toHaveBeenCalledWith(
'Publish Branch failed. Check your remote access and try again.'
)
expect(gitStatusMock).not.toHaveBeenCalled()
expect(gitUpstreamStatusMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('maps non-fast-forward push errors into a clean actionable toast', async () => {
const store = createEditorStore()
const pushError = new Error(
'Updates were rejected because the tip of your current branch is behind its remote counterpart.'
)
gitPushMock.mockRejectedValueOnce(pushError)
await expect(store.getState().pushBranch('wt-1', '/repo', false)).rejects.toThrow(
pushError.message
)
expect(toastErrorMock).toHaveBeenCalledWith(
'Push rejected — remote has changes. Pull first, then try again.'
)
expect(gitStatusMock).not.toHaveBeenCalled()
expect(gitUpstreamStatusMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('maps non-fast-forward keyword push errors into a clean actionable toast', async () => {
const store = createEditorStore()
const pushError = new Error('Push rejected: remote has newer commits (non-fast-forward).')
gitPushMock.mockRejectedValueOnce(pushError)
await expect(store.getState().pushBranch('wt-1', '/repo', false)).rejects.toThrow(
pushError.message
)
expect(toastErrorMock).toHaveBeenCalledWith(
'Push rejected — remote has changes. Pull first, then try again.'
)
expect(gitStatusMock).not.toHaveBeenCalled()
expect(gitUpstreamStatusMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('uses a fallback message for generic push errors', async () => {
const store = createEditorStore()
const pushError = new Error('network timeout')
gitPushMock.mockRejectedValueOnce(pushError)
await expect(store.getState().pushBranch('wt-1', '/repo', false)).rejects.toThrow(
pushError.message
)
expect(toastErrorMock).toHaveBeenCalledWith('Push failed. Check your connection and try again.')
expect(gitStatusMock).not.toHaveBeenCalled()
expect(gitUpstreamStatusMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('uses a fallback remote failure message when push rejects without Error', async () => {
const store = createEditorStore()
gitPushMock.mockRejectedValueOnce('failure')
await expect(store.getState().pushBranch('wt-1', '/repo', false)).rejects.toBe('failure')
expect(toastErrorMock).toHaveBeenCalledWith('Remote operation failed')
})
it('runs fetchBranch and clears the busy flag on success', async () => {
// Why: fetchBranch no longer awaits a post-op upstream refresh.
// useGitStatusPolling and the sidebar's upstream effect handle the
// reconcile, keeping the mutation focused on the single IPC.
const store = createEditorStore()
await store.getState().fetchBranch('wt-1', '/repo')
expect(gitFetchMock).toHaveBeenCalledWith({
worktreePath: '/repo',
connectionId: undefined
})
expect(store.getState().isRemoteOperationActive).toBe(false)
expect(toastErrorMock).not.toHaveBeenCalled()
})
it('surfaces a toast and clears the busy flag when fetch fails', async () => {
const store = createEditorStore()
gitFetchMock.mockRejectedValueOnce(new Error('network timeout'))
await expect(store.getState().fetchBranch('wt-1', '/repo')).rejects.toThrow('network timeout')
expect(toastErrorMock).toHaveBeenCalledWith('network timeout')
expect(gitUpstreamStatusMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('preserves prior upstream status when fetch fails', async () => {
// Why: a transient upstream fetch failure (network blip, auth prompt
// timeout) must not erase the last-known ahead/behind counts — doing so
// would briefly flip the UI to an unknown/no-upstream state that
// misrepresents the branch's relationship to its remote.
const store = createEditorStore()
store.getState().setUpstreamStatus('wt-1', {
hasUpstream: true,
upstreamName: 'origin/main',
ahead: 2,
behind: 1
})
gitUpstreamStatusMock.mockRejectedValueOnce(new Error('transient failure'))
await store.getState().fetchUpstreamStatus('wt-1', '/repo')
expect(store.getState().remoteStatusesByWorktree['wt-1']).toEqual({
hasUpstream: true,
upstreamName: 'origin/main',
ahead: 2,
behind: 1
})
})
it('keeps isRemoteOperationActive true while any remote op is in flight', async () => {
// Why: a bare boolean races across worktrees — if push A finishes while
// pull B is still running, flipping the flag off would prematurely
// re-enable B's button. The refcount-derived boolean must stay true
// until every in-flight remote op has finished.
const store = createEditorStore()
let resolveA: () => void = () => {}
let resolveB: () => void = () => {}
gitPushMock
.mockImplementationOnce(
() =>
new Promise<void>((resolve) => {
resolveA = resolve
})
)
.mockImplementationOnce(
() =>
new Promise<void>((resolve) => {
resolveB = resolve
})
)
const pushA = store.getState().pushBranch('wt-1', '/a')
// Kick microtasks so pushA has begun and flipped the flag on.
await Promise.resolve()
expect(store.getState().isRemoteOperationActive).toBe(true)
const pushB = store.getState().pushBranch('wt-2', '/b')
await Promise.resolve()
expect(store.getState().isRemoteOperationActive).toBe(true)
resolveA()
await pushA.catch(() => {})
// B is still running, so the busy flag must remain true.
expect(store.getState().isRemoteOperationActive).toBe(true)
resolveB()
await pushB.catch(() => {})
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('runs syncBranch end-to-end (fetch+pull+push) on success', async () => {
// Why: syncBranch no longer awaits a post-op git status / upstream
// refresh. The polling layer reconciles state after the mutation
// returns; the in-mutation upstream-status read remains because it
// gates whether the inner push stage runs.
const store = createEditorStore()
await store.getState().syncBranch('wt-1', '/repo')
expect(gitFetchMock).toHaveBeenCalledWith({
worktreePath: '/repo',
connectionId: undefined
})
expect(gitPullMock).toHaveBeenCalledWith({
worktreePath: '/repo',
connectionId: undefined
})
// ahead=1 in the default mock, so sync pushes.
expect(gitPushMock).toHaveBeenCalledWith({
worktreePath: '/repo',
connectionId: undefined
})
expect(toastErrorMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('skips the inner push when syncBranch sees ahead=0', async () => {
// Why: guards against a no-op push round-trip after a pure fast-forward
// pull. See syncBranch's ahead>0 guard in editor.ts.
const store = createEditorStore()
gitUpstreamStatusMock.mockResolvedValueOnce({
hasUpstream: true,
upstreamName: 'origin/main',
ahead: 0,
behind: 0
})
await store.getState().syncBranch('wt-1', '/repo')
expect(gitFetchMock).toHaveBeenCalled()
expect(gitPullMock).toHaveBeenCalled()
expect(gitPushMock).not.toHaveBeenCalled()
expect(toastErrorMock).not.toHaveBeenCalled()
})
it('surfaces a sync-labeled toast when syncBranch inner push fails with auth error', async () => {
// Why: the user invoked Sync — the toast must read "Sync failed..." even
// though the underlying step is push. Detail extraction still surfaces
// the actionable fatal/remote line so auth/protected-branch reasons stay
// visible.
const store = createEditorStore()
const authError = new Error(
'git push failed: Command failed: git push origin feature\nremote: Repository not found.\nfatal: Authentication failed for https://github.com/acme/private-repo.git'
)
gitPushMock.mockRejectedValueOnce(authError)
await expect(store.getState().syncBranch('wt-1', '/repo')).rejects.toThrow(authError.message)
expect(toastErrorMock).toHaveBeenCalledTimes(1)
expect(toastErrorMock).toHaveBeenCalledWith(
'Sync failed. Authentication failed for https://github.com/acme/private-repo.git. Check your remote access and try again.'
)
expect(store.getState().isRemoteOperationActive).toBe(false)
})
it('surfaces a single sync-labeled toast when syncBranch inner push is non-fast-forward', async () => {
// Why: under sync, NFF means the remote raced ahead between fetch and
// push — sync just pulled, so the bare "Pull first" guidance is wrong.
// Surface a sync-shaped retry hint instead.
const store = createEditorStore()
const pushError = new Error(
'Updates were rejected because the tip of your current branch is behind its remote counterpart.'
)
gitPushMock.mockRejectedValueOnce(pushError)
await expect(store.getState().syncBranch('wt-1', '/repo')).rejects.toThrow(pushError.message)
// No double-toast from the outer catch.
expect(toastErrorMock).toHaveBeenCalledTimes(1)
expect(toastErrorMock).toHaveBeenCalledWith(
'Sync failed — remote moved while syncing. Try again.'
)
})
it('surfaces the pull-blocked toast when syncBranch pull stage fails', async () => {
// Why: failures in sync's fetch/pull/status stages flow through the
// outer catch's generic path; push-specific framing only applies to
// the inner push stage.
const store = createEditorStore()
gitPullMock.mockRejectedValueOnce(
new Error(
'error: Your local changes to the following files would be overwritten by merge:\n\tsrc/app.ts\nPlease commit your changes or stash them before you merge.\nAborting'
)
)
await expect(store.getState().syncBranch('wt-1', '/repo')).rejects.toThrow()
expect(toastErrorMock).toHaveBeenCalledTimes(1)
expect(toastErrorMock).toHaveBeenCalledWith(
'Pull blocked — commit or stash your local changes first.'
)
expect(gitPushMock).not.toHaveBeenCalled()
expect(store.getState().isRemoteOperationActive).toBe(false)
})
})
describe('createEditorSlice activateMarkdownLink', () => {
const openUrlMock = vi.fn()
const openFileUriMock = vi.fn()
+284
View File
@@ -14,10 +14,13 @@ import type {
GitConflictStatusSource,
GitStatusEntry,
GitStatusResult,
GitUpstreamStatus,
SearchResult,
WorkspaceSessionState,
WorkspaceVisibleTabType
} from '../../../../shared/types'
import { stripCredentialsFromMessage } from '../../../../shared/git-remote-error'
import type { RemoteOpKind } from '@/components/right-sidebar/source-control-primary-action'
export type DiffSource =
| 'unstaged'
@@ -265,6 +268,38 @@ export type EditorSlice = {
// Why: lightweight updater for conflict operation only, used to clear stale
// "Rebasing"/"Merging" badges on non-active worktrees without a full git status poll.
setConflictOperation: (worktreeId: string, operation: GitConflictOperation) => void
remoteStatusesByWorktree: Record<string, GitUpstreamStatus>
setUpstreamStatus: (worktreeId: string, status: GitUpstreamStatus) => void
// Why: refcount-backed busy flag. A bare boolean races across worktrees —
// push on A finishing while pull on B is still in flight would flip the
// flag off and prematurely re-enable B's button. beginRemoteOperation /
// endRemoteOperation must be paired (begin at the start of the async
// operation, end in finally) so the derived boolean only flips to false
// once every in-flight remote op has finished.
isRemoteOperationActive: boolean
remoteOperationDepth: number
// Why: surfaces *which* remote op the user actually triggered so the
// primary button can mirror it (label + spinner) rather than leaving a
// stale label from before the dropdown click. Cleared when depth hits 0.
// Last-write-wins on concurrent ops, which is fine — the UI disables
// every entry while busy, so concurrent ops can't be initiated through it.
inFlightRemoteOpKind: RemoteOpKind | null
beginRemoteOperation: (kind?: RemoteOpKind) => void
endRemoteOperation: () => void
fetchUpstreamStatus: (
worktreeId: string,
worktreePath: string,
connectionId?: string
) => Promise<void>
pushBranch: (
worktreeId: string,
worktreePath: string,
publish?: boolean,
connectionId?: string
) => Promise<void>
pullBranch: (worktreeId: string, worktreePath: string, connectionId?: string) => Promise<void>
syncBranch: (worktreeId: string, worktreePath: string, connectionId?: string) => Promise<void>
fetchBranch: (worktreeId: string, worktreePath: string, connectionId?: string) => Promise<void>
gitBranchChangesByWorktree: Record<string, GitBranchChangeEntry[]>
gitBranchCompareSummaryByWorktree: Record<string, GitBranchCompareSummary | null>
gitBranchCompareRequestKeyByWorktree: Record<string, string>
@@ -342,6 +377,109 @@ function openWorkspaceEditorItem(
return created?.id ?? fileId
}
const REMOTE_OPERATION_FAILED_MESSAGE = 'Remote operation failed'
const REMOTE_OPERATION_DETAIL_MAX_LENGTH = 200
// Why: arbitrarily long git stderr lines (for instance, a multi-kilobyte
// server-side pre-receive hook message) should not blow up the toast. Cap the
// detail length so the toast stays readable; the underlying error is still
// rethrown for console/logs if a caller needs the full payload.
function truncateDetail(detail: string): string {
if (detail.length <= REMOTE_OPERATION_DETAIL_MAX_LENGTH) {
return detail
}
return `${detail.slice(0, REMOTE_OPERATION_DETAIL_MAX_LENGTH).trimEnd()}...`
}
function extractPublishFailureDetail(message: string): string | null {
const normalized = message.replace(/\r\n/g, '\n')
const lines = normalized
.split('\n')
.map((line) => line.trim())
.filter(Boolean)
const fatalLine = lines.find((line) => line.startsWith('fatal:'))
if (fatalLine) {
return truncateDetail(stripCredentialsFromMessage(fatalLine.slice('fatal:'.length).trim()))
}
const remoteLine = lines.find((line) => line.startsWith('remote:'))
if (remoteLine) {
return truncateDetail(stripCredentialsFromMessage(remoteLine.slice('remote:'.length).trim()))
}
return null
}
function resolveRemoteOperationErrorMessage(
error: unknown,
options?: { publish?: boolean; isPush?: boolean; isSync?: boolean }
): string {
if (!(error instanceof Error)) {
return REMOTE_OPERATION_FAILED_MESSAGE
}
// Why: under sync, the inner push runs *after* a successful pull, so a
// non-fast-forward at that point means the remote raced ahead between
// fetch and push — not "user forgot to pull". Saying "Pull first" would
// be wrong (sync just did). Branch isSync above the shared NFF path so
// sync gets a sync-shaped message instead of inheriting the push wording.
if (
options?.isSync &&
/non-fast-forward|fetch first|updates were rejected/i.test(error.message)
) {
return 'Sync failed — remote moved while syncing. Try again.'
}
// Why: non-fast-forward/rejected detection is shared across publish and push so
// both paths surface the same actionable toast regardless of operation type.
if (/non-fast-forward|fetch first|updates were rejected/i.test(error.message)) {
return 'Push rejected — remote has changes. Pull first, then try again.'
}
// Why: `git pull` / merge refuses to run when the working tree has changes
// that would be overwritten; surface a single readable line instead of the
// multi-line git stderr (which lists every affected path).
if (
/local changes.*would be overwritten|Please commit your changes or stash them/i.test(
error.message
)
) {
return 'Pull blocked — commit or stash your local changes first.'
}
if (options?.publish) {
// Why: publish failures often bubble up as raw wrapped git/IPC payloads; this
// keeps the toast human-readable while preserving the actionable fatal reason.
const detail = extractPublishFailureDetail(error.message)
if (detail) {
return `Publish Branch failed. ${detail}. Check your remote access and try again.`
}
return 'Publish Branch failed. Check your remote access and try again.'
}
if (options?.isSync) {
// Why: the user invoked Sync — surface "Sync failed" rather than leaking
// the inner-step name ("Push failed"). Detail extraction matches push so
// auth / protected-branch reasons stay actionable.
const detail = extractPublishFailureDetail(error.message)
if (detail) {
return `Sync failed. ${detail}. Check your remote access and try again.`
}
return 'Sync failed. Check your connection and try again.'
}
if (options?.isPush) {
// Why: surfacing fatal/remote lines from git is more actionable than a generic
// connection message for auth errors, protected branches, etc.
const detail = extractPublishFailureDetail(error.message)
if (detail) {
return `Push failed. ${detail}. Check your remote access and try again.`
}
return 'Push failed. Check your connection and try again.'
}
return error.message
}
export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (set, get) => ({
editorDrafts: {},
setEditorDraft: (fileId, content) =>
@@ -1711,6 +1849,152 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
})
}
}),
remoteStatusesByWorktree: {},
setUpstreamStatus: (worktreeId, status) =>
set((s) => ({
remoteStatusesByWorktree: {
...s.remoteStatusesByWorktree,
[worktreeId]: status
}
})),
isRemoteOperationActive: false,
remoteOperationDepth: 0,
inFlightRemoteOpKind: null,
beginRemoteOperation: (kind) =>
set((s) => ({
remoteOperationDepth: s.remoteOperationDepth + 1,
isRemoteOperationActive: true,
// Why: last-write-wins. The UI disables every action entry while busy,
// so a second remote op can't be started from inside Orca. If a
// background caller (future) triggers one, surfacing the most recent
// kind matches "what the user is currently watching".
inFlightRemoteOpKind: kind ?? s.inFlightRemoteOpKind
})),
endRemoteOperation: () =>
set((s) => {
const next = Math.max(0, s.remoteOperationDepth - 1)
return {
remoteOperationDepth: next,
isRemoteOperationActive: next > 0,
// Why: only clear the in-flight kind when no remote op remains. Until
// depth reaches 0 some other op is still running and its label/
// spinner should keep displaying.
inFlightRemoteOpKind: next > 0 ? s.inFlightRemoteOpKind : null
}
}),
fetchUpstreamStatus: async (worktreeId, worktreePath, connectionId) => {
try {
const status = await window.api.git.upstreamStatus({
worktreePath,
connectionId
})
get().setUpstreamStatus(worktreeId, status)
} catch (error) {
// Why: on error we leave the prior status in place rather than writing a
// synthetic {hasUpstream:false} — that would flash 'Publish Branch' on a
// tracked branch after any transient IPC hiccup and a user click would
// re-publish, clobbering the upstream relationship. If the branch is
// genuinely newly unpublished, the polling effect will eventually correct
// the status on success.
console.error('fetchUpstreamStatus failed', error)
}
},
pushBranch: async (worktreeId, worktreePath, publish = false, connectionId) => {
// Why: don't *await* a post-op git status / upstream refresh here.
// Chaining awaited refreshes inside the mutation extends the gap before
// compound flows (runCompoundCommitAction → runRemoteAction) reach the
// next step. But we still need a near-immediate upstream refresh so
// the primary button label rotates from "Push" to "Commit" as soon as
// ahead=0 — the polling layer is on a 3s interval, which is long
// enough to read as a stuck label. Solution: fire the upstream refresh
// as fire-and-forget so it doesn't block the mutation but updates the
// store as soon as the IPC resolves.
get().beginRemoteOperation(publish ? 'publish' : 'push')
try {
await window.api.git.push({ worktreePath, publish, connectionId })
} catch (error) {
toast.error(resolveRemoteOperationErrorMessage(error, { publish, isPush: true }))
throw error
} finally {
get().endRemoteOperation()
}
void get().fetchUpstreamStatus(worktreeId, worktreePath, connectionId)
},
pullBranch: async (worktreeId, worktreePath, connectionId) => {
get().beginRemoteOperation('pull')
try {
await window.api.git.pull({ worktreePath, connectionId })
} catch (error) {
toast.error(resolveRemoteOperationErrorMessage(error))
throw error
} finally {
get().endRemoteOperation()
}
void get().fetchUpstreamStatus(worktreeId, worktreePath, connectionId)
},
syncBranch: async (worktreeId, worktreePath, connectionId) => {
// Why: same shape as pushBranch / pullBranch — fire-and-forget the
// post-op upstream refresh after the busy flag clears so the primary
// button label rotates immediately when the IPC resolves.
get().beginRemoteOperation('sync')
// Why: the inner push stage toasts with { isSync: true } so its failure
// surfaces a "Sync failed..." message instead of "Push failed..." — the
// user invoked Sync; the underlying push is implementation detail. The
// outer catch must then skip toasting to avoid a double-toast.
let pushStageToastShown = false
try {
await window.api.git.fetch({ worktreePath, connectionId })
await window.api.git.pull({ worktreePath, connectionId })
// Why: push only if the pull left local commits that aren't on the
// remote. After a merge pull the ahead count can be >0 (local commits +
// the new merge commit) or 0 (pure fast-forward), and we avoid a
// no-op push round-trip in the fast-forward case.
const upstreamStatus = await window.api.git.upstreamStatus({
worktreePath,
connectionId
})
if (upstreamStatus.ahead > 0) {
try {
await window.api.git.push({ worktreePath, connectionId })
} catch (error) {
// Why: format under the user-facing operation (sync) rather than
// the inner step (push) — the user clicked Sync and shouldn't see
// a "Push failed" toast for a step they didn't directly invoke.
toast.error(resolveRemoteOperationErrorMessage(error, { isSync: true }))
pushStageToastShown = true
throw error
}
}
} catch (error) {
if (!pushStageToastShown) {
// Why: same isSync framing for fetch/pull/upstream-status failures so
// every sync failure path consistently reads as "Sync failed..." (or
// a more specific actionable message like "Pull blocked..." when the
// shared classifiers match first).
toast.error(resolveRemoteOperationErrorMessage(error, { isSync: true }))
}
throw error
} finally {
get().endRemoteOperation()
}
void get().fetchUpstreamStatus(worktreeId, worktreePath, connectionId)
},
fetchBranch: async (worktreeId, worktreePath, connectionId) => {
// Why: same shape as pushBranch / pullBranch — fire-and-forget the
// upstream refresh after the busy flag clears. Fetch updates the
// remote refs only, so the visible signal we want is the new
// ahead/behind counts on the upstream-status payload.
get().beginRemoteOperation('fetch')
try {
await window.api.git.fetch({ worktreePath, connectionId })
} catch (error) {
toast.error(resolveRemoteOperationErrorMessage(error))
throw error
} finally {
get().endRemoteOperation()
}
void get().fetchUpstreamStatus(worktreeId, worktreePath, connectionId)
},
gitBranchChangesByWorktree: {},
gitBranchCompareSummaryByWorktree: {},
gitBranchCompareRequestKeyByWorktree: {},
+22
View File
@@ -0,0 +1,22 @@
import { describe, expect, it } from 'vitest'
import { isNoUpstreamError } from './git-remote-error'
describe('isNoUpstreamError', () => {
it('treats a missing HEAD@{u} tracking ref as no upstream', () => {
const error = new Error(
"fatal: ambiguous argument 'HEAD@{u}': unknown revision or path not in the working tree.\n" +
"Use '--' to separate paths from revisions, like this:\n" +
"'git <command> [<revision>...] -- [<file>...]'"
)
expect(isNoUpstreamError(error)).toBe(true)
})
it('does not treat unrelated ambiguous refs as no upstream', () => {
const error = new Error(
"fatal: ambiguous argument 'feature': unknown revision or path not in the working tree."
)
expect(isNoUpstreamError(error)).toBe(false)
})
})
+98
View File
@@ -0,0 +1,98 @@
// Why: git's stderr often embeds the full remote URL, which can include a
// credential. Redact carefully: classic `user:password@` forms always carry
// a credential on any scheme (HTTPS, ssh://, git://, git+ssh://), but a
// lone `user@` is a credential ONLY for HTTP(S) (e.g. token-only PATs like
// `https://ghp_xxx@host`). For `ssh://git@host/...` the `git` login is
// required by the SSH remote — stripping it would produce a broken URL in
// the surfaced error and hide which remote actually failed. The two
// scheme-scoped patterns below keep SSH user-info intact while still
// scrubbing passwords on any scheme and HTTPS token-only forms.
const USERPASS_URL_PATTERN = /([a-z][a-z0-9+.-]*:\/\/)[^\s/@:]+:[^\s/@]+@/gi
const HTTPS_TOKEN_URL_PATTERN = /(https?:\/\/)[^\s/@:]+@/gi
export function stripCredentialsFromMessage(message: string): string {
return message.replace(USERPASS_URL_PATTERN, '$1').replace(HTTPS_TOKEN_URL_PATTERN, '$1')
}
function extractTailLine(message: string): string {
// Why: execFile rejections prefix the message with "Command failed: git ..."
// followed by the full stderr. The meaningful diagnostic is typically the
// last non-empty line; surfacing the full blob risks leaking local paths or
// environment details to the UI.
const lines = message
.split('\n')
.map((line) => line.trim())
.filter((line) => line.length > 0)
return lines.at(-1) ?? message
}
export type GitRemoteOperation = 'push' | 'pull' | 'fetch' | 'upstream'
export function normalizeGitErrorMessage(error: unknown, operation?: GitRemoteOperation): string {
if (!(error instanceof Error)) {
return 'Git remote operation failed.'
}
// Why: scrub credentials up-front so every downstream branch — including
// any future refactor that returns a substring of `raw` — operates on
// already-redacted text. The fast-path branches below return fixed
// literals today, but this hardens against accidental leakage later.
const raw = stripCredentialsFromMessage(error.message)
// Why: `non-fast-forward` / `fetch first` can appear on fetch (after a
// remote force-push updating a tracking ref) and on pull (with
// `pull.ff=only`), so the "pull or sync first" guidance only makes sense
// when the user was actually pushing. For other operations, fall through
// to the generic tail-line path. `operation === undefined` keeps the
// legacy push-shaped message for any caller that hasn't been updated yet.
if (
(operation === 'push' || operation === undefined) &&
(raw.includes('non-fast-forward') || raw.includes('fetch first'))
) {
return 'Push rejected: remote has newer commits (non-fast-forward). Please pull or sync first.'
}
if (raw.includes('could not read Username') || raw.includes('Authentication failed')) {
return 'Authentication failed. Check your remote credentials.'
}
if (raw.includes('Could not resolve host') || raw.includes('Network is unreachable')) {
return 'Network error. Check your connection.'
}
if (raw.includes('no tracking information') || raw.includes('no upstream')) {
return 'Branch has no upstream. Publish the branch first.'
}
// Fallthrough: extract only the tail stderr line. `raw` was already
// credential-scrubbed at the top of the function, so no further scrub needed.
return extractTailLine(raw)
}
// Why: we only swallow clearly-no-upstream signals — an expected state, not a
// failure. Other errors ('not a git repository', 'corrupt', auth failures,
// sparse-checkout errors, etc.) must fall through to the caller so users can
// act on them. We explicitly avoid matching `HEAD@{u}` alone because execFile
// wraps errors with "Command failed: git rev-parse --abbrev-ref HEAD@{u}…",
// which would cause every non-repo/corrupt failure to spuriously look like
// no-upstream. We also do NOT match 'no such branch' — that phrase is too
// broad and can mask real errors on corrupt refs or sparse-checkout failures.
// Additionally gate the phrase match on a `fatal:` prefix: git always
// prefixes these diagnostics with `fatal:`, so requiring it prevents
// `HEAD does not point` / `Needed a single revision` from matching unrelated
// output (e.g. hook stdout, progress lines) and silently hiding real
// corrupt-repo / unborn-HEAD / ambiguous-ref failures behind a spurious
// "0 ahead / 0 behind, no upstream" UI state. The one ambiguous-ref
// exception is HEAD@{u}: git emits it when branch config points at a
// tracking ref that is missing locally, which is the same expected UX state.
const NO_UPSTREAM_PHRASE_PATTERN =
/no upstream configured|no tracking information|HEAD does not point|Needed a single revision|ambiguous argument 'HEAD@\{u\}'/i
const FATAL_PREFIX_PATTERN = /(^|\n)fatal:/i
export function isNoUpstreamError(error: unknown): boolean {
if (!(error instanceof Error)) {
return false
}
const message = error.message
return FATAL_PREFIX_PATTERN.test(message) && NO_UPSTREAM_PHRASE_PATTERN.test(message)
}
+11
View File
@@ -1588,6 +1588,17 @@ export type GitStatusResult = {
conflictOperation: GitConflictOperation
}
// Why: when hasUpstream is false, ahead/behind are placeholder zeros, not a
// "sync" signal — callers must check hasUpstream before treating 0/0 as in-sync.
// Kept separate from GitStatusResult because upstream lookup can fail for
// reasons unrelated to working-tree status (e.g., no upstream is expected).
export type GitUpstreamStatus = {
hasUpstream: boolean
upstreamName?: string
ahead: number
behind: number
}
export type GitBranchChangeStatus = 'modified' | 'added' | 'deleted' | 'renamed' | 'copied'
export type GitBranchChangeEntry = {