starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-09-22 00:02:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(worktree): stop warning about a stale local base branch that does not exist yet (#15331) (#15871)

Browse Source 
Co-authored-by: vam <a@a.com>
This commit is contained in:
Neil
2026-08-22 23:00:27 -07:00
committed by GitHub
co-authored by vam
parent 8d1557c225
commit af2e825626
5 changed files with 290 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/git/worktree-add-local-base-refresh.test.ts
+115 -1
View File
@@ -347,13 +347,19 @@ describe('addWorktree', () => {
it('skips updating the local branch when it has diverged', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit}
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('not a fast-forward'))
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'refs/heads/main\n' }) // for-each-ref refs/heads/main (exists)
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add
resolveCreationBaseConfigWrite()
gitExecFileAsyncMock.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset)
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote
await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true)
const result = await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true)
expect(result.localBaseRefRefresh).toEqual({
status: 'skipped_not_fast_forward',
baseRef: 'origin/main',
localBranch: 'main'
})
expect(gitExecFileAsyncMock.mock.calls).toEqual([
[
['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'],
@@ -363,6 +369,10 @@ describe('addWorktree', () => {
['rev-list', '--left-right', '--count', 'refs/heads/main...refs/remotes/origin/main'],
expect.objectContaining({ cwd: '/repo' })
],
[
['for-each-ref', '--count=1', '--format=%(refname)', 'refs/heads/main'],
expect.objectContaining({ cwd: '/repo' })
],
[
[
'worktree',
@@ -396,12 +406,112 @@ describe('addWorktree', () => {
])
})
// #15331: evaluation runs before `-b <branch>` exists, so rev-list fails on the missing local ref.
it('does not warn when worktree add itself creates the local base branch', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/feature-x^{commit}
.mockRejectedValueOnce(
new Error(
"fatal: ambiguous argument 'refs/heads/feature-x...refs/remotes/origin/feature-x': unknown revision or path not in the working tree."
)
) // rev-list: refs/heads/feature-x does not exist yet
.mockResolvedValueOnce({ stdout: '' }) // for-each-ref refs/heads/feature-x (missing)
.mockResolvedValueOnce({ stdout: '' }) // worktree add
.mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch.<branch>.base
.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset)
.mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote
const result = await addWorktree(
'/repo',
'/repo-feature-x',
'feature-x',
'origin/feature-x',
true
)
expect(result.localBaseRefRefresh).toBeUndefined()
expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toContainEqual([
'worktree',
'add',
'--no-track',
'-b',
'feature-x',
'/repo-feature-x',
'refs/remotes/origin/feature-x'
])
// Nothing was refreshed, so no ref mutation.
expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0][0])).not.toContain('update-ref')
expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0][0])).not.toContain('reset')
})
// #15331: same missing-local-branch class, but the new branch name differs from the base's.
it('does not warn when the local base branch does not exist in a fetch-only clone', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/main^{commit}
.mockRejectedValueOnce(new Error('unknown revision refs/heads/main')) // rev-list: no local main
.mockResolvedValueOnce({ stdout: '' }) // for-each-ref refs/heads/main (missing)
.mockResolvedValueOnce({ stdout: '' }) // worktree add
.mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch.<branch>.base
.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset)
.mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote
const result = await addWorktree('/repo', '/repo-feature', 'my-feature', 'origin/main', true)
expect(result.localBaseRefRefresh).toBeUndefined()
expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toContainEqual([
'for-each-ref',
'--count=1',
'--format=%(refname)',
'refs/heads/main'
])
})
// A failed probe is not proof of absence, so the warning must survive it.
it('keeps the warning when the local base ref probe itself fails', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/main^{commit}
.mockRejectedValueOnce(new Error('rev-list failed')) // drift probe
.mockRejectedValueOnce(new Error('fatal: not a git repository')) // for-each-ref probe could not run
.mockResolvedValueOnce({ stdout: '' }) // worktree add
.mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch.<branch>.base
.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset)
.mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote
const result = await addWorktree('/repo', '/repo-feature', 'my-feature', 'origin/main', true)
expect(result.localBaseRefRefresh).toEqual({
status: 'skipped_not_fast_forward',
baseRef: 'origin/main',
localBranch: 'main'
})
})
it('still suggests nothing but keeps the warning when the local base ref exists and diverged', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse --verify --quiet refs/remotes/origin/main^{commit}
.mockResolvedValueOnce({ stdout: '2\t3\n' }) // rev-list: 2 local-only commits
.mockResolvedValueOnce({ stdout: '' }) // worktree add
.mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch.<branch>.base
.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset)
.mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote
const result = await addWorktree('/repo', '/repo-feature', 'main', 'origin/main', true)
// Same branch name as the base, but rev-list succeeded: real divergence must still warn.
expect(result.localBaseRefRefresh).toEqual({
status: 'skipped_not_fast_forward',
baseRef: 'origin/main',
localBranch: 'main'
})
})
it('skips local base refresh when captured OIDs are no longer ancestor-safe', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit}
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '0\t2\n' }) // stale rev-list result
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'new-local\n' }) // rev-parse refs/heads/main^{commit}
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'remote-main\n' }) // rev-parse refs/remotes/origin/main^{commit}
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('not an ancestor')) // merge-base captured OIDs
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'refs/heads/main\n' }) // for-each-ref refs/heads/main (exists)
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add
resolveCreationBaseConfigWrite()
gitExecFileAsyncMock.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset)
@@ -435,6 +545,10 @@ describe('addWorktree', () => {
['merge-base', '--is-ancestor', 'new-local', 'remote-main'],
expect.objectContaining({ cwd: '/repo' })
],
[
['for-each-ref', '--count=1', '--format=%(refname)', 'refs/heads/main'],
expect.objectContaining({ cwd: '/repo' })
],
[
[
'worktree',
src/main/git/worktree-base-ref-probe.ts
+29
View File
@@ -37,3 +37,32 @@ export async function hasWorktreeBaseCommitRef(
): Promise<boolean> {
return (await resolveWorktreeBaseCommitOid(repoPath, qualifiedRef, options)) !== null
}
export type WorktreeBaseRefPresence = 'present' | 'absent' | 'unknown'
/**
* Distinguish "the ref does not exist" from "the probe itself failed".
*
* Why for-each-ref: it exits 0 whether or not the pattern matches, so an empty result
* proves absence while a rejection still means the probe never ran (broken repo, dead
* SSH transport). `rev-parse --verify --quiet` exits 1 for both, and reading that as
* "absent" would silently drop warnings the caller must still surface.
*
* Executor-injected so the SSH path can route the same argv through the relay.
*/
export async function probeWorktreeBaseRefPresence(
runGit: (args: string[]) => Promise<{ stdout: string }>,
qualifiedRef: string
): Promise<WorktreeBaseRefPresence> {
try {
const { stdout } = await runGit([
'for-each-ref',
'--count=1',
'--format=%(refname)',
qualifiedRef
])
return stdout.trim() === qualifiedRef ? 'present' : 'absent'
} catch {
return 'unknown'
}
}
src/main/git/worktree.ts
+11 -1
View File
@@ -33,7 +33,7 @@ import {
import { withLocalGitCapabilityCacheForExecution } from './git-capability-state'
import { gitExecFileAsync, translateWslOutputPaths } from './runner'
import { resolveGitDir, runWithGitReadCacheInvalidation } from './status'
import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe'
import { hasWorktreeBaseCommitRef, probeWorktreeBaseRefPresence } from './worktree-base-ref-probe'
export type AddWorktreeResult = {
localBaseRefRefresh?: LocalBaseRefRefreshResult
@@ -265,6 +265,16 @@ async function evaluateLocalBaseRefRefreshability(
)
drift = parsedDrift
} catch {
// Why (#15331): the probes above also fail when refs/heads/<branch> is simply absent; a branch that
// does not exist yet cannot be stale, so report nothing instead of a bogus divergence warning.
// Only a proven absence suppresses: an unusable repo leaves the warning alone.
const presence = await probeWorktreeBaseRefPresence(
(args) => gitExecFileAsync(args, gitExecOptions(repoPath, options)),
parsed.fullRef
)
if (presence === 'absent') {
return undefined
}
return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } }
}
src/main/ipc/worktree-remote.ts
+21 -10
View File
@@ -35,6 +35,7 @@ import {
} from '../git/repo'
import { resolveLocalGitUsername, getSshGitUsername } from '../git/git-username'
import { hasCommitObjectViaGitExec } from '../git/commit-object-ref'
import { probeWorktreeBaseRefPresence } from '../git/worktree-base-ref-probe'
import { resolveWorktreeCreateBase } from '../worktree-create-base'
import { resolveWorktreeAddBaseRef } from '../../shared/worktree/base-ref'
import { getHostedReviewForBranch } from '../source-control/hosted-review'
@@ -181,7 +182,8 @@ type RemoteLocalBaseRefRefreshability =
}
| {
refreshable: false
result: LocalBaseRefRefreshResult
// undefined = nothing to refresh (no local branch yet), so the caller reports no status at all.
result: LocalBaseRefRefreshResult | undefined
}
function appendWorktreeCreateWarning(current: string | undefined, next: string): string {
@@ -698,8 +700,7 @@ async function hasRemoteWorktreeBaseRef(
repoPath: string,
baseRef: string
): Promise<boolean> {
const refExists = (qualifiedRef: string) =>
hasRemoteTrackingRefSsh(provider, repoPath, qualifiedRef)
const refExists = (qualifiedRef: string) => hasCommitRefSsh(provider, repoPath, qualifiedRef)
const resolvedBaseRef = await resolveWorktreeAddBaseRef(baseRef, refExists)
if (resolvedBaseRef !== baseRef) {
return true
@@ -710,8 +711,8 @@ async function hasRemoteWorktreeBaseRef(
return hasRemoteCommitObject(provider, repoPath, baseRef)
}
// Why: hasRemoteCommitObject resolves only SHAs, not symbolic remote-tracking refs; detect those directly for the fetch-failed local fallback.
async function hasRemoteTrackingRefSsh(
// Why: hasRemoteCommitObject resolves only SHAs, not symbolic refs; resolve any qualified ref (remote-tracking or local head) directly.
async function hasCommitRefSsh(
provider: SshGitProvider,
repoPath: string,
ref: string
@@ -1262,7 +1263,7 @@ async function resolveRemoteWorktreeCreateBasePlan(
baseBranchCandidate
)
if (remoteTrackingBase) {
if (await hasRemoteTrackingRefSsh(provider, repo.path, remoteTrackingBase.ref)) {
if (await hasCommitRefSsh(provider, repo.path, remoteTrackingBase.ref)) {
return true
}
return hasRemoteWorktreeBaseRef(provider, repo.path, baseBranchCandidate)
@@ -1313,7 +1314,7 @@ export async function prefetchRemoteWorktreeCreateBase(
}
if (basePlan.remoteTrackingBase) {
if (
(await hasRemoteTrackingRefSsh(provider, repo.path, basePlan.remoteTrackingBase.ref)) ||
(await hasCommitRefSsh(provider, repo.path, basePlan.remoteTrackingBase.ref)) ||
!(await hasRemoteWorktreeBaseRef(provider, repo.path, basePlan.baseBranch))
) {
await refreshRemoteTrackingBaseForWorktreeCreate(provider, repo, basePlan.remoteTrackingBase)
@@ -1333,7 +1334,7 @@ async function refreshLocalBaseRefForRemoteWorktreeCreate(
provider: SshGitProvider,
repoPath: string,
remoteTrackingBase: RemoteTrackingBase
): Promise<LocalBaseRefRefreshResult> {
): Promise<LocalBaseRefRefreshResult | undefined> {
const evaluation = await evaluateRemoteLocalBaseRefRefreshability(
provider,
repoPath,
@@ -1393,6 +1394,16 @@ async function evaluateRemoteLocalBaseRefRefreshability(
}
}
} catch {
// Why (#15331): the probes above also fail when refs/heads/<branch> is simply absent; the relay's
// `worktree add -b` is about to create it, so there is nothing stale to warn about. Only a proven
// absence suppresses: a dropped relay connection is not evidence the branch is missing.
const presence = await probeWorktreeBaseRefPresence(
(args) => provider.exec(args, repoPath),
fullRef
)
if (presence === 'absent') {
return { refreshable: false, result: undefined }
}
return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } }
}
@@ -1550,7 +1561,7 @@ export async function createRemoteWorktree(
let baseFallback: WorktreeCreateBaseFallback | undefined
if (remoteTrackingBase) {
const hasRemoteTrackingBaseRef = await hasRemoteTrackingRefSsh(
const hasRemoteTrackingBaseRef = await hasCommitRefSsh(
provider,
repo.path,
remoteTrackingBase.ref
@@ -1700,7 +1711,7 @@ export async function createRemoteWorktree(
await refreshRemoteTrackingBaseForWorktreeCreate(provider, repo, remoteTrackingBase)
} catch {
// Why: a refresh failure shouldn't block create if a usable (stale) local base ref exists; probe after registerRoot and hard-fail only when none does.
if (!(await hasRemoteTrackingRefSsh(provider, repo.path, remoteTrackingBase.ref))) {
if (!(await hasCommitRefSsh(provider, repo.path, remoteTrackingBase.ref))) {
throw new Error(
`Could not refresh base ref "${baseBranch}" from "${remoteTrackingBase.remote}". Check your network and try again.`
)
src/main/ipc/worktrees-ssh-local-base-refresh.test.ts
+114
View File
@@ -477,4 +477,118 @@ describe('registerWorktreeHandlers', () => {
})
expect(result.localBaseRefUpdateSuggestion).toBeUndefined()
})
// #15331: the pre-create merge-base probe fails when refs/heads/<branch> does not exist yet.
const buildMissingLocalBaseSshCase = (presence: 'absent' | 'present' | 'probe-failed') => {
const repo = {
id: 'repo-ssh',
path: '/remote/repo',
displayName: 'ssh',
badgeColor: '#000',
addedAt: 0,
connectionId: 'conn-1',
worktreeBaseRef: 'origin/main'
}
const provider = {
exec: vi.fn().mockImplementation(async (args: string[]) => {
if (args[0] === 'remote') {
return { stdout: 'origin\n', stderr: '' }
}
if (args[0] === 'for-each-ref' && args.at(-1) === 'refs/heads/main') {
if (presence === 'probe-failed') {
throw new Error('ssh: connection closed by remote host')
}
return { stdout: presence === 'present' ? 'refs/heads/main\n' : '', stderr: '' }
}
if (args[0] === 'rev-parse') {
const ref = args.at(-1) ?? ''
if (ref.startsWith('refs/heads/main')) {
return { stdout: presence === 'present' ? 'local-main\n' : '', stderr: '' }
}
// Other refs/heads probes are the new-branch conflict check; it must stay unresolvable.
return { stdout: ref.startsWith('refs/heads/') ? '' : 'remote-main\n', stderr: '' }
}
if (args[0] === 'merge-base') {
throw new Error('fatal: Not a valid object name refs/heads/main')
}
return { stdout: '', stderr: '' }
}),
fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined),
addWorktree: vi.fn().mockResolvedValue(undefined),
listWorktrees: vi.fn().mockResolvedValue([
{
path: '/remote/repo-improve-dashboard',
head: 'abc123',
branch: 'refs/heads/improve-dashboard',
isBare: false,
isMainWorktree: false
}
]),
worktreeIsClean: vi.fn().mockResolvedValue({ clean: true }),
refreshLocalBaseRefForWorktreeCreate: vi.fn().mockResolvedValue(undefined)
}
store.getSettings.mockReturnValue({
branchPrefix: 'none',
nestWorkspaces: false,
refreshLocalBaseRefOnWorktreeCreate: true,
workspaceDir: '/workspace'
})
store.getRepos.mockReturnValue([repo])
store.getRepo.mockReturnValue(repo)
getSshGitProviderMock.mockReturnValue(provider)
getActiveMultiplexerMock.mockReturnValue({
request: vi.fn().mockResolvedValue(undefined),
notify: vi.fn()
})
store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta)
return provider
}
it('does not report an SSH local base refresh when the local base branch does not exist', async () => {
const provider = buildMissingLocalBaseSshCase('absent')
const result = (await handlers['worktrees:create'](null, {
repoId: 'repo-ssh',
name: 'improve-dashboard'
})) as CreateWorktreeResult
expect(provider.exec).toHaveBeenCalledWith(
['for-each-ref', '--count=1', '--format=%(refname)', 'refs/heads/main'],
'/remote/repo'
)
expect(result.localBaseRefRefresh).toBeUndefined()
expect(provider.refreshLocalBaseRefForWorktreeCreate).not.toHaveBeenCalled()
})
it('keeps the SSH not-fast-forward status when the local base branch exists and diverged', async () => {
const provider = buildMissingLocalBaseSshCase('present')
const result = (await handlers['worktrees:create'](null, {
repoId: 'repo-ssh',
name: 'improve-dashboard'
})) as CreateWorktreeResult
expect(result.localBaseRefRefresh).toEqual({
status: 'skipped_not_fast_forward',
baseRef: 'origin/main',
localBranch: 'main'
})
expect(provider.refreshLocalBaseRefForWorktreeCreate).not.toHaveBeenCalled()
})
// Losing the relay mid-probe is not evidence the branch is missing.
it('keeps the SSH not-fast-forward status when the local base ref probe fails', async () => {
const provider = buildMissingLocalBaseSshCase('probe-failed')
const result = (await handlers['worktrees:create'](null, {
repoId: 'repo-ssh',
name: 'improve-dashboard'
})) as CreateWorktreeResult
expect(result.localBaseRefRefresh).toEqual({
status: 'skipped_not_fast_forward',
baseRef: 'origin/main',
localBranch: 'main'
})
expect(provider.refreshLocalBaseRefForWorktreeCreate).not.toHaveBeenCalled()
})
})