perf(worktrees): remove redundant creation probes (#7161)

Co-authored-by: OrcaWin <alpha-eng@stably.ai>
This commit is contained in:
Brennan Benson 2026-07-11 00:41:28 -07:00 committed by GitHub
parent fa691a0dc8
commit 4572b0a446
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 153 additions and 25 deletions

View File

@ -1254,6 +1254,43 @@ describe('addWorktree', () => {
])
})
it('skips advisory owner probes when the local base is already current', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'abc123\n' }) // resolve creation base
.mockResolvedValueOnce({ stdout: '0\t0\n' }) // local base is current
.mockResolvedValueOnce({ stdout: '' }) // worktree add
.mockResolvedValueOnce({ stdout: '' }) // persist branch base
.mockResolvedValueOnce({ stdout: 'true\n' }) // push.autoSetupRemote already set
await expect(
addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', false, false, {
suggestLocalBaseRefUpdate: true
})
).resolves.toEqual({})
expect(gitExecFileAsyncMock.mock.calls.map(([args]) => args)).toEqual([
['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'],
['rev-list', '--left-right', '--count', 'refs/heads/main...refs/remotes/origin/main'],
[
'worktree',
'add',
'--no-track',
'-b',
'feature/test',
'/repo-feature',
'refs/remotes/origin/main'
],
[
'config',
'--local',
'--replace-all',
'branch.feature/test.base',
'refs/remotes/origin/main'
],
['config', '--get', 'push.autoSetupRemote']
])
})
it('uses normalized branch metadata for slash-containing remotes', async () => {
const worktreeListOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n'
gitExecFileAsyncMock

View File

@ -172,7 +172,8 @@ async function evaluateLocalBaseRefRefreshability(
baseBranch: string,
remoteTrackingRef: string,
remoteTrackingBase?: AddWorktreeOptions['remoteTrackingBase'],
options: GitWorktreeExecOptions = {}
options: GitWorktreeExecOptions = {},
shouldInspectOwner: (behind: number) => boolean = () => true
): Promise<LocalBaseRefRefreshability | undefined> {
const parsed = parseRemoteTrackingLocalBaseRef(baseBranch, remoteTrackingRef, remoteTrackingBase)
if (!parsed) {
@ -196,6 +197,11 @@ async function evaluateLocalBaseRefRefreshability(
if (!parsedDrift || parsedDrift.ahead !== 0) {
return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } }
}
if (!shouldInspectOwner(parsedDrift.behind)) {
// Why: a current local ref cannot produce an update suggestion, so the
// advisory path need not resolve OIDs or inspect its owner worktree.
return undefined
}
const { stdout: localOidOutput } = await gitExecFileAsync(
['rev-parse', '--verify', `${parsed.fullRef}^{commit}`],
gitExecOptions(repoPath, options)
@ -289,7 +295,8 @@ async function getLocalBaseRefUpdateSuggestionForWorktreeCreate(
baseBranch,
remoteTrackingRef,
remoteTrackingBase,
options
options,
(behind) => behind > 0
)
if (!evaluation?.refreshable || evaluation.behind <= 0) {
return undefined

View File

@ -1395,7 +1395,8 @@ async function refreshLocalBaseRefForRemoteWorktreeCreate(
async function evaluateRemoteLocalBaseRefRefreshability(
provider: SshGitProvider,
repoPath: string,
remoteTrackingBase: RemoteTrackingBase
remoteTrackingBase: RemoteTrackingBase,
shouldInspectOwner: (behind: number) => boolean = () => true
): Promise<RemoteLocalBaseRefRefreshability> {
const resultBase = {
baseRef: remoteTrackingBase.base,
@ -1413,6 +1414,17 @@ async function evaluateRemoteLocalBaseRefRefreshability(
repoPath
)
behind = countNonEmptyGitOutputLines(stdout)
if (!shouldInspectOwner(behind)) {
// Why: no behind commits means the advisory cannot offer an update;
// avoid remote worktree/status round trips that cannot change that.
return {
refreshable: true,
...resultBase,
fullRef,
remoteTrackingRef: remoteTrackingBase.ref,
behind
}
}
} catch {
return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } }
}
@ -1467,7 +1479,8 @@ async function getRemoteLocalBaseRefUpdateSuggestionForWorktreeCreate(
const evaluation = await evaluateRemoteLocalBaseRefRefreshability(
provider,
repoPath,
remoteTrackingBase
remoteTrackingBase,
(behind) => behind > 0
)
if (!evaluation.refreshable || evaluation.behind <= 0) {
return undefined
@ -1536,9 +1549,12 @@ export async function createRemoteWorktree(
// Register the repo root first so relays do not report a valid base as stale.
await registerRequiredSshWorktreeCreateRoots(repo.connectionId!, [repo.path])
// Why: SSH targets cannot use the local `gh` account, and git email/name are
// commit author identity rather than hosted-account usernames.
const username = await getSshGitUsername(provider, repo.path)
// Why: explicit branches and non-username prefix modes never consume this
// value; skipping the remote config probes preserves the exact branch name.
const username =
!args.branchNameOverride && settings.branchPrefix === 'git-username'
? await getSshGitUsername(provider, repo.path)
: ''
const branchConflictSubject = args.branchNameOverride ? 'branch name' : 'worktree name'
// Determine base branch
@ -1968,12 +1984,17 @@ export async function createLocalWorktree(
return { ...options, ...localWorktreeGitOptions }
}
const username = await resolveLocalGitUsername(repo.path)
const requestedName = args.name
const sanitizedName = sanitizeWorktreeName(args.name)
const requestedDisplayName = args.displayName
? sanitizeWorktreeDisplayName(args.displayName)
: undefined
// Why: explicit branches and non-username prefix modes never consume this
// value; skipping the probes preserves the exact generated branch name.
const username =
!args.branchNameOverride && settings.branchPrefix === 'git-username'
? await resolveLocalGitUsername(repo.path)
: ''
const baseBranch = await resolveWorktreeCreateBase({
requestedBaseBranch: args.baseBranch,

View File

@ -287,6 +287,7 @@ describe('registerWorktreeHandlers Windows path handling', () => {
'origin/main',
false
)
expect(resolveLocalGitUsernameMock).not.toHaveBeenCalled()
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
'repo-1::C:/workspaces/improve-dashboard',
expect.objectContaining({
@ -302,6 +303,39 @@ describe('registerWorktreeHandlers Windows path handling', () => {
})
})
it('resolves the Git username when the configured prefix consumes it', async () => {
store.getSettings.mockReturnValue({
branchPrefix: 'git-username',
nestWorkspaces: false,
refreshLocalBaseRefOnWorktreeCreate: false,
workspaceDir: 'C:\\workspaces'
})
resolveLocalGitUsernameMock.mockResolvedValue('octocat')
listWorktreesMock.mockResolvedValue([
{
path: 'C:/workspaces/improve-dashboard',
head: 'abc123',
branch: 'refs/heads/octocat/improve-dashboard',
isBare: false,
isMainWorktree: false
}
])
await handlers['worktrees:create'](null, {
repoId: 'repo-1',
name: 'improve-dashboard'
})
expect(resolveLocalGitUsernameMock).toHaveBeenCalledWith('C:\\repo')
expect(addWorktreeMock).toHaveBeenCalledWith(
'C:\\repo',
'C:\\workspaces\\improve-dashboard',
'octocat/improve-dashboard',
'origin/main',
false
)
})
it('preserves create-time metadata on the next list when Windows path formatting differs', async () => {
const worktreeEntry = {
path: 'C:/workspaces/improve-dashboard',

View File

@ -908,6 +908,13 @@ describe('registerWorktreeHandlers', () => {
})
it('uses branchNameOverride for the git branch while keeping the sanitized worktree path', async () => {
store.getSettings.mockReturnValue({
branchPrefix: 'git-username',
nestWorkspaces: false,
refreshLocalBaseRefOnWorktreeCreate: false,
workspaceDir: '/workspace'
})
resolveLocalGitUsernameMock.mockResolvedValue('unused-user')
listWorktreesMock.mockResolvedValue([
{
path: '/workspace/feature-something',
@ -935,6 +942,7 @@ describe('registerWorktreeHandlers', () => {
'origin/main',
false
)
expect(resolveLocalGitUsernameMock).not.toHaveBeenCalled()
expect(result).toMatchObject({
worktree: expect.objectContaining({
path: '/workspace/feature-something',
@ -2809,6 +2817,13 @@ describe('registerWorktreeHandlers', () => {
fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined),
addWorktree: vi.fn().mockResolvedValue(undefined),
listWorktrees: vi.fn().mockResolvedValue([
{
path: '/remote/repo',
head: 'base123',
branch: 'refs/heads/main',
isBare: false,
isMainWorktree: true
},
{
path: '/remote/improve-dashboard',
head: 'abc123',
@ -2816,7 +2831,8 @@ describe('registerWorktreeHandlers', () => {
isBare: false,
isMainWorktree: false
}
])
]),
worktreeIsClean: vi.fn().mockResolvedValue({ clean: true })
}
const mux = {
request: vi.fn().mockResolvedValue(undefined),
@ -2838,6 +2854,16 @@ describe('registerWorktreeHandlers', () => {
manualOrder: 123_456
})
expect(provider.exec).not.toHaveBeenCalledWith(
['config', '--get', 'github.user'],
'/remote/repo'
)
expect(provider.exec).not.toHaveBeenCalledWith(
['config', '--get', 'user.username'],
'/remote/repo'
)
expect(provider.listWorktrees).toHaveBeenCalledTimes(1)
expect(provider.worktreeIsClean).not.toHaveBeenCalled()
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
'repo-ssh::/remote/improve-dashboard',
expect.objectContaining({
@ -3798,18 +3824,15 @@ describe('registerWorktreeHandlers', () => {
}),
fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined),
addWorktree: vi.fn().mockResolvedValue(undefined),
listWorktrees: vi
.fn()
.mockResolvedValueOnce([])
.mockResolvedValueOnce([
{
path: '/remote/improve-dashboard',
head: 'abc123',
branch: 'refs/heads/improve-dashboard',
isBare: false,
isMainWorktree: false
}
])
listWorktrees: vi.fn().mockResolvedValueOnce([
{
path: '/remote/improve-dashboard',
head: 'abc123',
branch: 'refs/heads/improve-dashboard',
isBare: false,
isMainWorktree: false
}
])
}
const mux = {
request: vi.fn().mockResolvedValue(undefined),
@ -4019,7 +4042,6 @@ describe('registerWorktreeHandlers', () => {
addWorktree: vi.fn().mockResolvedValue(undefined),
listWorktrees: vi
.fn()
.mockResolvedValueOnce([])
.mockResolvedValueOnce([
{
path: '/remote/first-worktree',
@ -4029,7 +4051,6 @@ describe('registerWorktreeHandlers', () => {
isMainWorktree: false
}
])
.mockResolvedValueOnce([])
.mockResolvedValueOnce([
{
path: '/remote/second-worktree',

View File

@ -91,6 +91,7 @@ import { TERMINAL_METHODS } from './rpc/methods/terminal'
const ORIGINAL_PLATFORM = process.platform
const ORIGINAL_PLATFORM_DESCRIPTOR = Object.getOwnPropertyDescriptor(process, 'platform')
const removeWorktreeLinkedPathsMock = vi.hoisted(() => vi.fn())
const resolveLocalGitUsernameMock = vi.hoisted(() => vi.fn(async () => ''))
vi.mock('../ipc/worktree-symlinks', () => ({
createWorktreeLinkedPaths: vi.fn(),
@ -532,7 +533,7 @@ vi.mock('../git/repo', async (importOriginal) => {
vi.mock('../git/git-username', async () => {
const actual = await vi.importActual<typeof GitUsernameModule>('../git/git-username')
return { ...actual, resolveLocalGitUsername: vi.fn(async () => '') }
return { ...actual, resolveLocalGitUsername: resolveLocalGitUsernameMock }
})
function resetRuntimeTestMocks(): void {
@ -552,6 +553,7 @@ function resetRuntimeTestMocks(): void {
vi.mocked(assertWorktreeCleanForRemoval).mockResolvedValue(undefined)
vi.mocked(removeWorktree).mockReset()
removeWorktreeLinkedPathsMock.mockReset()
resolveLocalGitUsernameMock.mockReset().mockResolvedValue('')
vi.mocked(forceDeleteLocalBranchMock).mockReset()
vi.mocked(forceDeleteLocalBranchMock).mockResolvedValue(undefined)
sshGitProviders.clear()
@ -3048,6 +3050,7 @@ describe('OrcaRuntimeService', () => {
'origin/feature/something',
false
)
expect(resolveLocalGitUsernameMock).not.toHaveBeenCalled()
expect(result.worktree).toMatchObject({
path: '/tmp/workspaces/feature-something',
branch: 'feature/something'

View File

@ -14249,7 +14249,12 @@ export class OrcaRuntimeService {
const requestedDisplayName = args.displayName?.trim() || undefined
const sanitizedName = sanitizeWorktreeName(args.name)
let effectiveSanitizedName = sanitizedName
const username = await resolveLocalGitUsername(repo.path)
// Why: explicit branches and non-username prefix modes never consume this
// value; skipping the probes preserves the exact generated branch name.
const username =
!args.branchNameOverride && settings.branchPrefix === 'git-username'
? await resolveLocalGitUsername(repo.path)
: ''
const baseBranch = await resolveWorktreeCreateBase({
requestedBaseBranch: args.baseBranch,