mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
perf(worktree): defer fork-PR remote creation from create-time to first use (#17922)
* perf(worktree): defer fork-PR remote creation from create-time to first use Fork-PR review worktrees eagerly ran `git remote add` + `git fetch` for the contributor's fork (and pinned branch.<x>.remote) at create time, even for a read-only review. That grows remote count unboundedly with review volume and pays a network fetch nobody asked for yet. Defer prepareWorktreePushTarget(Ssh) and the --set-upstream-to configure step at create time (local + SSH, IPC + runtime create paths); persist the pushTarget metadata untouched. Materialize the remote on demand the first time push/pull/fetch/fast-forward actually needs it, via two shared functions (materializeWorktreePushTargetRemote(Ssh)) reused across the legacy IPC handlers and the RPC runtime sync commands. A cheap `remote get-url <name>` probe keeps steady-state calls down to one extra subprocess once materialized, instead of repeating the O(remotes) scan. Add repo-local `remote.<name>.orca-created` config provenance, written when the remote is added, so cleanup can recognize ownership of a remote that was lazily materialized (and therefore never round-tripped through the store's `remoteCreated` flag). Refs #17828 * perf(worktree): materialize a deferred fork-PR remote on terminal spawn An agent running raw git in a freshly opened fork-PR review terminal has no usable upstream until an Orca-driven sync happens -- "sync through Orca first" isn't available mid-task, and git pull/log @{u}.. hard-fail without one (verified against real git). Fire the same on-demand materialization used by push/pull/fetch/fast-forward from the single terminal-spawn resolver (resolveTerminalWorkspaceLaunchTarget), fire-and-forget, so a newly opened terminal gets a working upstream without blocking spawn. * fix(worktree): retest deferred fork-remote CI failures, fix SSH provenance-marker RPC Rewrites the 5 CI failures on the deferred fork-remote change (#17828) as evidence, not fixtures: the SSH relay-upgrade/rollback/sibling-ownership tests move to materializeWorktreePushTargetRemoteSsh, where that unchanged logic now actually runs (create defers it to first sync). While writing a stricter test that routes its mock exec through the relay's real validateGitExecArgs, found that the SSH provenance-marker write (`git config remote.<name>.orca-created true`) was unconditionally rejected by the relay's generic git.exec (it blocks all non-read-only config writes) -- a real bug that would break every SSH fork-remote materialization against a live relay. Fixes it with a narrow git.markRemoteOrcaCreated RPC, mirroring renameCurrentBranch, with a graceful no-op fallback for relays that predate it. * fix(worktree): scope post-#17887 test assertions past narrow-refspec config calls Rebasing onto #17887's narrow-refspec `remote add` broke two broad `['config']` call-filters into false positives/negatives, and the local materialize test still asserted the pre-#17887 wide `remote add`/fetch-refspec forms. * fix(worktree): restructure upstream restore, persist provenance, widen short-circuit refspec (#17828 review) - Move upstream restoration to the materializer level so it runs on both the remoteAlreadyMatchesUrl short-circuit and the full-prepare path, not just buried inside prepare*. - Persist {remoteCreated, remoteName} to the store on materialize so #17842's orphan sweep can see a lazily-created remote, including via desktop IPC, terminal-spawn, and the RPC host-callback paths. - Widen the refspec on the local short-circuit path too (SSH's bare `remote add` refspec gap remains a documented, pre-existing limitation). - Fetch the branch's tracking ref before restoring upstream when the short-circuit widens onto a *new* branch on an already-existing remote -- a bare refspec-config widen never itself imports anything, so `branch --set-upstream-to` was hard-failing for a sibling worktree's first materialize (found via a real-git fixture, not just mocked unit tests). Skipped when the ref already exists so the common repeat-call case stays a local-only probe with no network round-trip. * fix(worktree): merge duplicate shared/worktree/types import oxlint --deny-warnings flags the split import as no-duplicates; full pnpm lint was failing on it after the #17828 review restructuring. * fix(worktree): scope the deferred fetch timeout to fetch calls, retarget stale create-time assertions CI on the previous push failed 3 shards, all argument-shape mismatches: - worktrees-wsl-runtime-routing.test.ts: the "restructure upstream restore" commit wrapped every call `prepareWorktreePushTarget` makes (remote, remote add, config, fetch) with DEFERRED_PUSH_TARGET_FETCH_TIMEOUT_MS, not just the network fetch. Local git subprocesses never need a timeout; scope it to `args[0] === 'fetch'` only, matching the short-circuit path's existing pattern. Updated the test to expect the timeout on the fetch call specifically (point 5 legitimately adds it there), while every other call stays untimed. - worktrees-create-metadata-persistence.test.ts (2 tests): stale from before this session -- create no longer mints a fork remote at all (#17828 deferred that to first sync), so asserting `remote add`/`fetch`/`remoteCreated: true` at create time no longer matches reality. Retargeted both tests to assert the deferred contract (no remote add at create, pushTarget persisted unmaterialized); minting itself stays covered by worktree-remote-push-target-materialization.test.ts and worktree-push-target-setup.test.ts. Re-verified all 5 fixture points (mint upstream, store persistence, single-flight, short-circuit refspec widen + fetch-missing-ref for local and SSH, finite timeout) against a real git fixture after this fix -- all still pass. * fix(worktree): hook pty:spawn into deferred push-target materialization (#17828) triggerTerminalSpawnPushTargetMaterialization only fired for agent/background/ mobile terminals; the desktop GUI's own pty:spawn path (new tab, split, reattach) never materialized a deferred fork-PR remote before raw git commands could run there. Add a small wrapper that resolves the worktree's push target and owning repo from args.worktreeId via the store, and fire-and-forget delegates to the existing materializer, wired as the first statement of runPtyIpcSpawn. Degrades silently (optional chaining + catch) so a partial/fake Store in existing spawn tests can't turn this into a spawn-blocking throw. * test(worktree): retarget stale editor-remote-branch assertions for worktreeId threading runtime-git-sync-client's local-path fetch/pull/fastForward/push calls now forward context.worktreeId (needed by the main-process handlers to key deferred push-target materialization). Update the 17 call-site mocks across 15 tests in editor-remote-branch-actions.test.ts to expect worktreeId: 'wt-1', matching the already-correct source behavior -- no assertion was loosened. * fix(worktree): give a materialize joiner its own branch wiring The materialize single flight is keyed on the remote, but everything after the remote add is per-branch. A sibling worktree joining an in-flight mint for a different branch received the minter's target and skipped its own refspec widen, tracking-ref fetch, and upstream link, so its branch ended with no upstream at all. Wait for the remote, then run the per-branch work against the joiner's own target -- the same path the already-exists short-circuit takes, now shared rather than duplicated. Adopting a remote a sibling minted also stamps ownership, so removing the minter cannot strand the survivor's metadata outside the orphan sweep's reach. * fix(worktree): stop a failed mint from leaving a config-only fork remote Review of the joiner fix found it made things worse in three ways. Swallowing the mint's rejection let a joiner adopt a remote the rollback had already removed, writing remote.<name>.fetch with no URL. Verified on real git: that ghost section breaks `git fetch --all`, forces every later mint to a `-2` name, and cannot be removed by `git remote remove`. Propagate instead; the in-flight map is already cleared, so a retry re-mints. The SSH twin still returned the minter's target to a joiner, so the original per-branch bug survived there. It now adopts against its own target through a twin helper. The ownership stamp was unreachable: it required both a store and a repo id, and no caller passes both. Derive the repo id from the worktree id. Adopters also write remote config, and concurrent `git config --add` has no lock retry -- 135 of 160 writes failed at 8-way concurrency, and equal values duplicate the refspec. Chain adoptions per remote.
This commit is contained in:
Neil
@@ -55,6 +55,30 @@ function refspecSource(refspec: string): string {
|
||||
return refspec.replace(/^\+/, '').split(':')[0]!
|
||||
}
|
||||
|
||||
/**
|
||||
* True if `branchName`'s remote-tracking ref already exists locally under `remoteName`.
|
||||
* Used to skip a redundant fetch on the common repeat-materialize case (the ref was
|
||||
* already pulled in by an earlier mint/fetch) while still fetching it on demand the
|
||||
* first time a sibling worktree widens an existing remote onto a new branch -- a bare
|
||||
* refspec-config widen never itself imports anything (see `ensureRemoteTracksBranchNarrowly`).
|
||||
*/
|
||||
export async function forkRemoteTrackingRefExists(
|
||||
execGit: GitExecFn,
|
||||
repoPath: string,
|
||||
remoteName: string,
|
||||
branchName: string
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
await execGit(
|
||||
['rev-parse', '--verify', '--quiet', `refs/remotes/${remoteName}/${branchName}`],
|
||||
repoPath
|
||||
)
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True only if `remote.<name>.url` is actually set. Deliberately plumbing (`config --get`),
|
||||
* not porcelain `git remote get-url` -- the latter falls back to echoing the remote *name*
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import type { GitPushTarget } from '../../shared/worktree/types'
|
||||
import { getUpstreamStatus } from './upstream'
|
||||
|
||||
// Why: on-demand remote materialization (#17828) defers `git remote add` for a
|
||||
// fork PR to first push/pull/fetch/fast-forward, so an unpublished review's
|
||||
// status must be read against a pushTarget whose remote was never created.
|
||||
// This exercises the real `rev-parse --verify --quiet` failure path -- a
|
||||
// fake/mocked git can't reproduce its exact exit-code/stderr shape, which is
|
||||
// exactly what `getPublishTargetStatus`'s missing-ref fallback depends on.
|
||||
describe('getUpstreamStatus with a deferred (not-yet-materialized) fork remote', () => {
|
||||
const tempPaths: string[] = []
|
||||
|
||||
afterEach(() => {
|
||||
for (const path of tempPaths.splice(0)) {
|
||||
rmSync(path, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('reports the graceful "publish" state instead of 0 ahead/0 behind', async () => {
|
||||
const repoPath = mkdtempSync(join(tmpdir(), 'orca-deferred-fork-remote-'))
|
||||
tempPaths.push(repoPath)
|
||||
const git = (...args: string[]): string =>
|
||||
execFileSync('git', args, { cwd: repoPath, encoding: 'utf8' })
|
||||
|
||||
git('init', '--quiet')
|
||||
git('config', 'user.name', 'Orca Test')
|
||||
git('config', 'user.email', 'orca@example.test')
|
||||
git('config', 'commit.gpgSign', 'false')
|
||||
git('config', 'core.hooksPath', '.git/no-hooks')
|
||||
writeFileSync(join(repoPath, 'fixture.txt'), 'base\n')
|
||||
git('add', 'fixture.txt')
|
||||
git('commit', '-m', 'base')
|
||||
git('branch', '-M', 'contributor/fix')
|
||||
|
||||
// Simulates a fork-PR review worktree right after create: pushTarget
|
||||
// metadata is persisted, but `pr-contributor-orca` was never added as a
|
||||
// remote because materialization is deferred to first use.
|
||||
const pushTarget: GitPushTarget = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
}
|
||||
|
||||
const status = await getUpstreamStatus(repoPath, pushTarget)
|
||||
|
||||
expect(status).toEqual({
|
||||
hasUpstream: false,
|
||||
upstreamName: 'pr-contributor-orca/contributor/fix',
|
||||
ahead: 0,
|
||||
behind: 0,
|
||||
hasConfiguredPushTarget: true
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -9,6 +9,10 @@ import {
|
||||
import { resolveRegisteredWorktreePath } from '../../registered-worktree-roots-cache'
|
||||
import { getLocalGitOptionsForRegisteredWorktree } from '../../local-worktree-runtime-options'
|
||||
import { assertGitPushTargetShape } from '../../../../shared/git-push-target-validation'
|
||||
import {
|
||||
materializeWorktreePushTargetRemote,
|
||||
materializeWorktreePushTargetRemoteSsh
|
||||
} from '../../worktree-remote'
|
||||
import type { FilesystemHandlerContext } from '../filesystem-handler-context'
|
||||
|
||||
export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandlerContext): void {
|
||||
@@ -20,6 +24,7 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
_event,
|
||||
args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
publish?: boolean
|
||||
forceWithLease?: boolean
|
||||
connectionId?: string
|
||||
@@ -36,7 +41,18 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
return provider.pushBranch(args.worktreePath, publish, args.pushTarget, {
|
||||
// Why: a fork remote deferred at create time (#17828) must exist before push.
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(
|
||||
provider,
|
||||
args.worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
return provider.pushBranch(args.worktreePath, publish, materializedPushTarget, {
|
||||
forceWithLease: args.forceWithLease === true
|
||||
})
|
||||
}
|
||||
@@ -46,13 +62,23 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
args.worktreePath,
|
||||
worktreePath
|
||||
)
|
||||
if (args.pushTarget) {
|
||||
await validateGitPushTarget(worktreePath, args.pushTarget, {
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
gitOptions,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
if (materializedPushTarget) {
|
||||
await validateGitPushTarget(worktreePath, materializedPushTarget, {
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
}
|
||||
await gitPush(worktreePath, publish, args.pushTarget, {
|
||||
await gitPush(worktreePath, publish, materializedPushTarget, {
|
||||
forceWithLease: args.forceWithLease === true,
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
@@ -64,7 +90,12 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
'git:pull',
|
||||
async (
|
||||
_event,
|
||||
args: { worktreePath: string; connectionId?: string; pushTarget?: GitPushTarget }
|
||||
args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}
|
||||
): Promise<void> => {
|
||||
if (args.connectionId) {
|
||||
if (args.pushTarget) {
|
||||
@@ -74,7 +105,17 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
return provider.pullBranch(args.worktreePath, args.pushTarget)
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(
|
||||
provider,
|
||||
args.worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
return provider.pullBranch(args.worktreePath, materializedPushTarget)
|
||||
}
|
||||
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
|
||||
const gitOptions = getLocalGitOptionsForRegisteredWorktree(
|
||||
@@ -82,13 +123,23 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
args.worktreePath,
|
||||
worktreePath
|
||||
)
|
||||
if (args.pushTarget) {
|
||||
await validateGitPushTarget(worktreePath, args.pushTarget, {
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
gitOptions,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
if (materializedPushTarget) {
|
||||
await validateGitPushTarget(worktreePath, materializedPushTarget, {
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
}
|
||||
await gitPull(worktreePath, args.pushTarget, {
|
||||
await gitPull(worktreePath, materializedPushTarget, {
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
@@ -99,7 +150,12 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
'git:fastForward',
|
||||
async (
|
||||
_event,
|
||||
args: { worktreePath: string; connectionId?: string; pushTarget?: GitPushTarget }
|
||||
args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}
|
||||
): Promise<void> => {
|
||||
if (args.connectionId) {
|
||||
if (args.pushTarget) {
|
||||
@@ -109,7 +165,17 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
return provider.fastForwardBranch(args.worktreePath, args.pushTarget)
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(
|
||||
provider,
|
||||
args.worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
return provider.fastForwardBranch(args.worktreePath, materializedPushTarget)
|
||||
}
|
||||
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
|
||||
const gitOptions = getLocalGitOptionsForRegisteredWorktree(
|
||||
@@ -117,13 +183,23 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl
|
||||
args.worktreePath,
|
||||
worktreePath
|
||||
)
|
||||
if (args.pushTarget) {
|
||||
await validateGitPushTarget(worktreePath, args.pushTarget, {
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
gitOptions,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
if (materializedPushTarget) {
|
||||
await validateGitPushTarget(worktreePath, materializedPushTarget, {
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
}
|
||||
await gitFastForward(worktreePath, args.pushTarget, {
|
||||
await gitFastForward(worktreePath, materializedPushTarget, {
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
|
||||
@@ -17,6 +17,10 @@ import { resolveRegisteredWorktreePath } from '../../registered-worktree-roots-c
|
||||
import { getLocalGitOptionsForRegisteredWorktree } from '../../local-worktree-runtime-options'
|
||||
import { assertGitPushTargetShape } from '../../../../shared/git-push-target-validation'
|
||||
import { validateGitForkSyncExpectedUpstream } from '../../../../shared/git-fork-sync'
|
||||
import {
|
||||
materializeWorktreePushTargetRemote,
|
||||
materializeWorktreePushTargetRemoteSsh
|
||||
} from '../../worktree-remote'
|
||||
import type { FilesystemHandlerContext } from '../filesystem-handler-context'
|
||||
|
||||
export function registerGitRemoteSyncHandlers(context: FilesystemHandlerContext): void {
|
||||
@@ -52,7 +56,12 @@ export function registerGitRemoteSyncHandlers(context: FilesystemHandlerContext)
|
||||
'git:fetch',
|
||||
async (
|
||||
_event,
|
||||
args: { worktreePath: string; connectionId?: string; pushTarget?: GitPushTarget }
|
||||
args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}
|
||||
): Promise<void> => {
|
||||
if (args.connectionId) {
|
||||
if (args.pushTarget) {
|
||||
@@ -62,7 +71,17 @@ export function registerGitRemoteSyncHandlers(context: FilesystemHandlerContext)
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
return provider.fetchRemote(args.worktreePath, args.pushTarget)
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(
|
||||
provider,
|
||||
args.worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
return provider.fetchRemote(args.worktreePath, materializedPushTarget)
|
||||
}
|
||||
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
|
||||
const gitOptions = getLocalGitOptionsForRegisteredWorktree(
|
||||
@@ -70,13 +89,23 @@ export function registerGitRemoteSyncHandlers(context: FilesystemHandlerContext)
|
||||
args.worktreePath,
|
||||
worktreePath
|
||||
)
|
||||
if (args.pushTarget) {
|
||||
await validateGitPushTarget(worktreePath, args.pushTarget, {
|
||||
const materializedPushTarget = args.pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
worktreePath,
|
||||
args.pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
gitOptions,
|
||||
args.worktreeId
|
||||
)
|
||||
: undefined
|
||||
if (materializedPushTarget) {
|
||||
await validateGitPushTarget(worktreePath, materializedPushTarget, {
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
}
|
||||
await gitFetch(worktreePath, args.pushTarget, {
|
||||
await gitFetch(worktreePath, materializedPushTarget, {
|
||||
...gitOptions,
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
// Real-binary coverage for #17828's remaining gap: the mocked-underlying-trigger suite in
|
||||
// `spawn-push-target-materialization.test.ts` proves the wiring/delegation logic, but not
|
||||
// that a `pty:spawn`-originated terminal -- the desktop GUI's own terminal path, previously
|
||||
// uncovered -- actually ends up with a configured upstream against real git. No mocks here:
|
||||
// this exercises the real `triggerTerminalSpawnPushTargetMaterialization` and real
|
||||
// `materializeWorktreePushTargetRemote`, driven only through `runPtyIpcSpawn`'s hook.
|
||||
import { execFile } from 'node:child_process'
|
||||
import { mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { promisify } from 'node:util'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitPushTarget } from '../../../../shared/worktree/types'
|
||||
import type { Repo } from '../../../../shared/repo-types'
|
||||
import type { WorktreeMeta } from '../../../../shared/worktree/meta-types'
|
||||
import type { Store } from '../../../persistence'
|
||||
import type { PtySpawnIpcDeps } from './spawn-types'
|
||||
import { triggerPtySpawnPushTargetMaterialization } from './spawn-push-target-materialization'
|
||||
|
||||
const execFileAsync = promisify(execFile)
|
||||
|
||||
const REPO_ID = 'repo-1'
|
||||
const FORK_REMOTE = 'pr-contributor-orca'
|
||||
const TRACKED_BRANCH = 'contributor/fix'
|
||||
|
||||
let scratchDir = ''
|
||||
let repoPath = ''
|
||||
let forkPath = ''
|
||||
let worktreeId = ''
|
||||
let mainBranch = ''
|
||||
|
||||
async function git(args: string[], cwd: string): Promise<string> {
|
||||
const { stdout } = await execFileAsync('git', args, { cwd })
|
||||
return stdout
|
||||
}
|
||||
|
||||
async function setIdentity(cwd: string): Promise<void> {
|
||||
await git(['config', 'user.name', 'Orca Test'], cwd)
|
||||
await git(['config', 'user.email', 'orca@example.test'], cwd)
|
||||
await git(['config', 'commit.gpgSign', 'false'], cwd)
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
// realpath: macOS hands out /var/... temp paths while Git reports /private/var/...
|
||||
scratchDir = await realpath(await mkdtemp(join(tmpdir(), 'orca-pty-spawn-push-target-')))
|
||||
repoPath = join(scratchDir, 'repo')
|
||||
forkPath = join(scratchDir, 'fork')
|
||||
worktreeId = `${REPO_ID}::${repoPath}`
|
||||
|
||||
await mkdir(repoPath, { recursive: true })
|
||||
await git(['init', '-q'], repoPath)
|
||||
await setIdentity(repoPath)
|
||||
await writeFile(join(repoPath, 'seed.txt'), 'seed\n')
|
||||
await git(['add', '-A'], repoPath)
|
||||
await git(['commit', '-qm', 'seed'], repoPath)
|
||||
mainBranch = (await git(['rev-parse', '--abbrev-ref', 'HEAD'], repoPath)).trim()
|
||||
|
||||
await git(['clone', '-q', repoPath, forkPath], scratchDir)
|
||||
await setIdentity(forkPath)
|
||||
await git(['checkout', '-qb', TRACKED_BRANCH], forkPath)
|
||||
await writeFile(join(forkPath, 'fix.txt'), 'fix\n')
|
||||
await git(['add', '-A'], forkPath)
|
||||
await git(['commit', '-qm', 'fix'], forkPath)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(scratchDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
function forkTarget(): GitPushTarget {
|
||||
return { remoteName: FORK_REMOTE, branchName: TRACKED_BRANCH, remoteUrl: forkPath }
|
||||
}
|
||||
|
||||
function depsFor(
|
||||
pushTarget: GitPushTarget,
|
||||
setWorktreeMeta?: Store['setWorktreeMeta']
|
||||
): {
|
||||
deps: PtySpawnIpcDeps
|
||||
meta: Record<string, WorktreeMeta>
|
||||
} {
|
||||
const meta: Record<string, WorktreeMeta> = { [worktreeId]: { pushTarget } as WorktreeMeta }
|
||||
const store = {
|
||||
getWorktreeMeta: (id: string) => meta[id],
|
||||
getRepo: (id: string) => ({ id, path: repoPath, connectionId: null }) as unknown as Repo,
|
||||
getAllWorktreeMeta: () => meta,
|
||||
...(setWorktreeMeta ? { setWorktreeMeta } : {})
|
||||
} as unknown as Store
|
||||
return { deps: { store } as unknown as PtySpawnIpcDeps, meta }
|
||||
}
|
||||
|
||||
describe('triggerPtySpawnPushTargetMaterialization (real git fixture)', () => {
|
||||
it('materializes the fork remote and configures the upstream for a pty:spawn-originated terminal', async () => {
|
||||
// Why: not just that materialization was *called* -- the coordinator's bar for closing
|
||||
// the gap is a real, git-verified configured upstream reachable from a pty:spawn arg set.
|
||||
// Pre-seeds the remote so materialize takes the short-circuit branch (worktree-remote.ts):
|
||||
// real `remote add`/`fetch` against a fabricated fork is already covered against real git by
|
||||
// worktree-push-target-refspec-real-git.test.ts; the top-level entry point this hook calls
|
||||
// additionally validates `remoteUrl` against a GitHub URL shape, which a local fixture path
|
||||
// can never satisfy. The short-circuit is also the common case in practice -- every pty:spawn
|
||||
// after the worktree's first (new tab, split, reattach) -- and still drives real
|
||||
// `ensureRemoteTracksBranchNarrowly` / narrow `fetch` / `--set-upstream-to` git calls.
|
||||
await git(['remote', 'add', FORK_REMOTE, forkPath], repoPath)
|
||||
const { deps } = depsFor(forkTarget())
|
||||
|
||||
triggerPtySpawnPushTargetMaterialization(deps, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
worktreeId
|
||||
})
|
||||
|
||||
await vi.waitFor(
|
||||
async () => {
|
||||
const upstream = await git(
|
||||
['rev-parse', '--abbrev-ref', `${mainBranch}@{u}`],
|
||||
repoPath
|
||||
).catch(() => '')
|
||||
expect(upstream.trim()).toBe(`${FORK_REMOTE}/${TRACKED_BRANCH}`)
|
||||
},
|
||||
{ timeout: 5000, interval: 25 }
|
||||
)
|
||||
|
||||
const remoteUrl = (await git(['remote', 'get-url', FORK_REMOTE], repoPath)).trim()
|
||||
expect(remoteUrl).toBe(forkPath)
|
||||
|
||||
// The tracked branch's commit must actually be present -- confirms the narrow fetch ran,
|
||||
// not just that the remote config was written.
|
||||
const forkHead = (await git(['rev-parse', TRACKED_BRANCH], forkPath)).trim()
|
||||
const fetchedHead = (
|
||||
await git(['rev-parse', `${FORK_REMOTE}/${TRACKED_BRANCH}`], repoPath)
|
||||
).trim()
|
||||
expect(fetchedHead).toBe(forkHead)
|
||||
})
|
||||
|
||||
it('is a no-op once the remote was already created (repeat pty:spawn, e.g. reattach)', async () => {
|
||||
const target = { ...forkTarget(), remoteCreated: true }
|
||||
const { deps } = depsFor(target)
|
||||
await git(['remote', 'add', FORK_REMOTE, forkPath], repoPath)
|
||||
|
||||
triggerPtySpawnPushTargetMaterialization(deps, { cols: 80, rows: 24, worktreeId })
|
||||
|
||||
// Give the fire-and-forget chain a tick; there is nothing to wait for since a
|
||||
// remoteCreated target must short-circuit before any git call.
|
||||
await new Promise((resolve) => setImmediate(resolve))
|
||||
const upstream = await git(['rev-parse', '--abbrev-ref', `${mainBranch}@{u}`], repoPath).catch(
|
||||
() => ''
|
||||
)
|
||||
expect(upstream.trim()).toBe('')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,145 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitPushTarget } from '../../../../shared/worktree/types'
|
||||
import type { Repo } from '../../../../shared/repo-types'
|
||||
import type { WorktreeMeta } from '../../../../shared/worktree/meta-types'
|
||||
import type { Store } from '../../../persistence'
|
||||
import type { PtySpawnIpcArgs, PtySpawnIpcDeps } from './spawn-types'
|
||||
|
||||
const { triggerMock } = vi.hoisted(() => ({ triggerMock: vi.fn() }))
|
||||
vi.mock('../../../runtime/runtime-terminal-spawn-push-target-materialization', () => ({
|
||||
triggerTerminalSpawnPushTargetMaterialization: triggerMock
|
||||
}))
|
||||
|
||||
import { triggerPtySpawnPushTargetMaterialization } from './spawn-push-target-materialization'
|
||||
|
||||
const REPO_ID = 'repo-1'
|
||||
const WORKTREE_PATH = '/repo/worktree'
|
||||
const WORKTREE_ID = `${REPO_ID}::${WORKTREE_PATH}`
|
||||
const FORK_TARGET: GitPushTarget = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
}
|
||||
const REPO = { id: REPO_ID, path: '/repo', connectionId: null } as unknown as Repo
|
||||
|
||||
function depsWithStore(overrides: Partial<Store> = {}): PtySpawnIpcDeps {
|
||||
return {
|
||||
store: {
|
||||
getWorktreeMeta: vi.fn().mockReturnValue({ pushTarget: FORK_TARGET } as WorktreeMeta),
|
||||
getRepo: vi.fn().mockReturnValue(REPO),
|
||||
...overrides
|
||||
} as unknown as Store
|
||||
} as unknown as PtySpawnIpcDeps
|
||||
}
|
||||
|
||||
function baseArgs(overrides: Partial<PtySpawnIpcArgs> = {}): PtySpawnIpcArgs {
|
||||
return { cols: 80, rows: 24, worktreeId: WORKTREE_ID, ...overrides }
|
||||
}
|
||||
|
||||
describe('triggerPtySpawnPushTargetMaterialization', () => {
|
||||
let warnSpy: ReturnType<typeof vi.spyOn>
|
||||
|
||||
beforeEach(() => {
|
||||
triggerMock.mockReset()
|
||||
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
})
|
||||
|
||||
it('is a no-op when args has no worktreeId', () => {
|
||||
triggerPtySpawnPushTargetMaterialization(depsWithStore(), baseArgs({ worktreeId: undefined }))
|
||||
expect(triggerMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('is a no-op when deps has no store', () => {
|
||||
triggerPtySpawnPushTargetMaterialization({} as unknown as PtySpawnIpcDeps, baseArgs())
|
||||
expect(triggerMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('is a no-op for a malformed worktreeId (no separator)', () => {
|
||||
triggerPtySpawnPushTargetMaterialization(
|
||||
depsWithStore(),
|
||||
baseArgs({ worktreeId: 'not-a-valid-id' })
|
||||
)
|
||||
expect(triggerMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('parses the worktreeId, looks up the push target and repo, and delegates', () => {
|
||||
const deps = depsWithStore()
|
||||
triggerPtySpawnPushTargetMaterialization(deps, baseArgs())
|
||||
|
||||
expect(deps.store!.getWorktreeMeta).toHaveBeenCalledWith(WORKTREE_ID)
|
||||
expect(deps.store!.getRepo).toHaveBeenCalledWith(REPO_ID)
|
||||
expect(triggerMock).toHaveBeenCalledWith(
|
||||
WORKTREE_PATH,
|
||||
FORK_TARGET,
|
||||
REPO,
|
||||
deps.store,
|
||||
REPO_ID,
|
||||
WORKTREE_ID
|
||||
)
|
||||
})
|
||||
|
||||
it('passes null when the repo lookup misses', () => {
|
||||
const deps = depsWithStore({ getRepo: vi.fn().mockReturnValue(undefined) })
|
||||
triggerPtySpawnPushTargetMaterialization(deps, baseArgs())
|
||||
|
||||
expect(triggerMock).toHaveBeenCalledWith(
|
||||
WORKTREE_PATH,
|
||||
FORK_TARGET,
|
||||
null,
|
||||
deps.store,
|
||||
REPO_ID,
|
||||
WORKTREE_ID
|
||||
)
|
||||
})
|
||||
|
||||
// Why: many pty:spawn unit tests supply a narrow fake Store missing these methods --
|
||||
// this is the actual bug the hook must guard against (#17828), not a hypothetical.
|
||||
// Optional chaining degrades the lookups to undefined/null; the underlying trigger
|
||||
// itself no-ops on an undefined push target, so this never blocks or throws on spawn.
|
||||
it('does not throw when the store lacks getWorktreeMeta/getRepo, delegating with undefined/null', () => {
|
||||
const partialStore = {} as Store
|
||||
expect(() =>
|
||||
triggerPtySpawnPushTargetMaterialization(
|
||||
{ store: partialStore } as unknown as PtySpawnIpcDeps,
|
||||
baseArgs()
|
||||
)
|
||||
).not.toThrow()
|
||||
expect(triggerMock).toHaveBeenCalledWith(
|
||||
WORKTREE_PATH,
|
||||
undefined,
|
||||
null,
|
||||
partialStore,
|
||||
REPO_ID,
|
||||
WORKTREE_ID
|
||||
)
|
||||
})
|
||||
|
||||
it('warns and swallows an error thrown by the underlying trigger', () => {
|
||||
triggerMock.mockImplementation(() => {
|
||||
throw new Error('boom')
|
||||
})
|
||||
expect(() =>
|
||||
triggerPtySpawnPushTargetMaterialization(depsWithStore(), baseArgs())
|
||||
).not.toThrow()
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('failed to trigger push target materialization'),
|
||||
expect.any(Error)
|
||||
)
|
||||
})
|
||||
|
||||
it('strips a folder-workspace instance suffix from the worktree path before delegating', () => {
|
||||
const instanceId = 'a1b2c3d4-e5f6-4789-a012-b3c4d5e6f789'
|
||||
const deps = depsWithStore()
|
||||
const suffixedId = `${WORKTREE_ID}::workspace:${instanceId}`
|
||||
triggerPtySpawnPushTargetMaterialization(deps, baseArgs({ worktreeId: suffixedId }))
|
||||
|
||||
expect(triggerMock).toHaveBeenCalledWith(
|
||||
WORKTREE_PATH,
|
||||
FORK_TARGET,
|
||||
REPO,
|
||||
deps.store,
|
||||
REPO_ID,
|
||||
suffixedId
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,40 @@
|
||||
import { splitWorktreeIdForFilesystem } from '../../../../shared/worktree/id'
|
||||
import { triggerTerminalSpawnPushTargetMaterialization } from '../../../runtime/runtime-terminal-spawn-push-target-materialization'
|
||||
import type { PtySpawnIpcArgs, PtySpawnIpcDeps } from './spawn-types'
|
||||
|
||||
// Why (#17828): pty:spawn is the desktop GUI's own terminal path (new tab, split, reattach) --
|
||||
// raw git commands can run here before any Orca-driven sync, so a deferred fork-PR remote must
|
||||
// exist first. Mirrors the agent/background-terminal hook in
|
||||
// runtime-terminal-spawn-push-target-materialization.ts, which this delegates to; fire-and-forget
|
||||
// and a no-op once the remote already exists, so it is safe on every spawn including reattaches.
|
||||
export function triggerPtySpawnPushTargetMaterialization(
|
||||
deps: PtySpawnIpcDeps,
|
||||
args: PtySpawnIpcArgs
|
||||
): void {
|
||||
if (!args.worktreeId || !deps.store) {
|
||||
return
|
||||
}
|
||||
const parsed = splitWorktreeIdForFilesystem(args.worktreeId)
|
||||
if (!parsed) {
|
||||
return
|
||||
}
|
||||
// Why: never let a partial/fake Store (many pty:spawn unit tests supply a narrow one) or an
|
||||
// unexpected lookup failure turn this best-effort hook into a spawn-blocking exception.
|
||||
try {
|
||||
const pushTarget = deps.store.getWorktreeMeta?.(args.worktreeId)?.pushTarget
|
||||
const repo = deps.store.getRepo?.(parsed.repoId) ?? null
|
||||
triggerTerminalSpawnPushTargetMaterialization(
|
||||
parsed.worktreePath,
|
||||
pushTarget,
|
||||
repo,
|
||||
deps.store,
|
||||
parsed.repoId,
|
||||
args.worktreeId
|
||||
)
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
`[pty-spawn] failed to trigger push target materialization for ${args.worktreeId}:`,
|
||||
error
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import { buildPtyIpcSpawnOptions } from './spawn-options'
|
||||
import { executePtyIpcSpawn } from './spawn-execute'
|
||||
import { commitPtyIpcSpawn } from './spawn-commit'
|
||||
import { createPtyIpcSpawnState, type PtyIpcSpawnState } from './spawn-state'
|
||||
import { triggerPtySpawnPushTargetMaterialization } from './spawn-push-target-materialization'
|
||||
import type { PtySpawnIpcArgs, PtySpawnIpcDeps } from './spawn-types'
|
||||
|
||||
function releaseAbandonedAgentTeamsLeader(ctx: PtyIpcSpawnState): void {
|
||||
@@ -30,6 +31,7 @@ function restoreProvisionalPtySize(ctx: PtyIpcSpawnState): void {
|
||||
}
|
||||
|
||||
export async function runPtyIpcSpawn(deps: PtySpawnIpcDeps, args: PtySpawnIpcArgs) {
|
||||
triggerPtySpawnPushTargetMaterialization(deps, args)
|
||||
const ctx = createPtyIpcSpawnState(deps, args)
|
||||
const early = await beginPtyIpcSpawn(ctx)
|
||||
if (early) {
|
||||
|
||||
@@ -108,8 +108,13 @@ describe('cleanupUnusedWorktreePushTargetRemoteWithExec', () => {
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
// No probing at all when we won't act.
|
||||
expect(exec).not.toHaveBeenCalled()
|
||||
// Why: the store flag alone can't rule out ownership -- on-demand
|
||||
// materialization (#17828) never sets it, so cleanup also probes the
|
||||
// repo-local `orca-created` config provenance before bailing.
|
||||
expect(exec).toHaveBeenCalledWith(
|
||||
['config', '--get', `remote.${FORK_REMOTE}.orca-created`],
|
||||
REPO_PATH
|
||||
)
|
||||
})
|
||||
|
||||
it('never touches origin or upstream', async () => {
|
||||
@@ -242,6 +247,20 @@ describe('cleanupUnusedWorktreePushTargetRemoteWithExec', () => {
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
})
|
||||
|
||||
it('removes a remote owned only via git-config provenance (lazily materialized, #17828)', async () => {
|
||||
// Why: on-demand materialization never sets the store's `remoteCreated`
|
||||
// flag, so ownership must also be provable from `remote.<name>.orca-created`.
|
||||
const exec = makeExec({ branchConfig: 'true' })
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget({ remoteCreated: false }),
|
||||
storeOf({ 'repo-1::/wt/a': forkTarget({ remoteCreated: false }) }),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([['remote', 'remove', FORK_REMOTE]])
|
||||
})
|
||||
|
||||
it('does nothing when the remote is already gone (get-url throws)', async () => {
|
||||
const exec = makeExec({ getUrlThrows: true })
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
|
||||
@@ -16,7 +16,11 @@ export type GitRemoteExec = (
|
||||
args: string[],
|
||||
cwd: string
|
||||
) => Promise<{ stdout: string; stderr?: string }>
|
||||
export type WorktreePushTargetStore = Pick<Store, 'getAllWorktreeMeta'>
|
||||
// Why: `setWorktreeMeta` is optional so existing narrow test stubs (only
|
||||
// `getAllWorktreeMeta`) keep compiling; callers that want materialize-time
|
||||
// provenance persistence (worktree-remote.ts) pass a store that has it.
|
||||
export type WorktreePushTargetStore = Pick<Store, 'getAllWorktreeMeta'> &
|
||||
Partial<Pick<Store, 'setWorktreeMeta'>>
|
||||
|
||||
export function sameGitHubRemoteUrl(left: string, right: string): boolean {
|
||||
if (left === right) {
|
||||
@@ -181,6 +185,26 @@ function isBranchConfigSeparator(code: number): boolean {
|
||||
return code === 32 || (code >= 9 && code <= 13)
|
||||
}
|
||||
|
||||
// Why: on-demand materialization (push/pull/fetch/fast-forward, #17828) never
|
||||
// updates the store's `pushTarget.remoteCreated` flag, so ownership must also be
|
||||
// readable from the repo-local `remote.<name>.orca-created` config Orca writes
|
||||
// when it creates the remote (see `worktree-push-target-setup.ts`).
|
||||
async function remoteHasOrcaProvenance(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
remoteName: string
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await execGit(
|
||||
['config', '--get', `remote.${remoteName}.orca-created`],
|
||||
repoPath
|
||||
)
|
||||
return stdout.trim() === 'true'
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Exported for unit tests: the `execGit` seam lets tests drive the multi-fork
|
||||
// cleanup matrix without touching a real repo.
|
||||
export async function cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
@@ -190,11 +214,12 @@ export async function cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
store: WorktreePushTargetStore,
|
||||
execGit: GitRemoteExec
|
||||
): Promise<void> {
|
||||
if (!target?.remoteUrl || target.remoteName === 'origin' || target.remoteName === 'upstream') {
|
||||
return
|
||||
}
|
||||
if (
|
||||
!target?.remoteCreated ||
|
||||
!target.remoteUrl ||
|
||||
target.remoteName === 'origin' ||
|
||||
target.remoteName === 'upstream'
|
||||
!target.remoteCreated &&
|
||||
!(await remoteHasOrcaProvenance(execGit, repoPath, target.remoteName))
|
||||
) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -5,7 +5,9 @@ import {
|
||||
configureCreatedWorktreePushTargetWithExec,
|
||||
ensureUniqueRemoteName,
|
||||
findRemoteForUrl,
|
||||
prepareWorktreePushTargetWithExec
|
||||
prepareWorktreePushTargetWithExec,
|
||||
remoteAlreadyMatchesUrl,
|
||||
restoreUpstreamAfterMaterialize
|
||||
} from './worktree-push-target-setup'
|
||||
|
||||
type ExecMock = Mock<GitRemoteExec>
|
||||
@@ -15,9 +17,17 @@ const FORK_SSH = 'git@github.com:contributor/orca.git'
|
||||
const FORK_HTTPS = 'https://github.com/contributor/orca.git'
|
||||
|
||||
// A stateful fake git: `remotes` maps name -> url. `remote add` mutates it so
|
||||
// later lookups see the new remote, matching real git behavior.
|
||||
function makeRepoExec(remotes: Record<string, string>): ExecMock {
|
||||
// later lookups see the new remote, matching real git behavior. Defaults
|
||||
// `symbolic-ref --short HEAD` to a real branch name, since a worktree's HEAD
|
||||
// always resolves to one (mirrors real git, unlike an empty-stdout stub).
|
||||
function makeRepoExec(
|
||||
remotes: Record<string, string>,
|
||||
checkedOutBranch = 'local-branch'
|
||||
): ExecMock {
|
||||
return vi.fn<GitRemoteExec>(async (args: string[]) => {
|
||||
if (args[0] === 'symbolic-ref' && args[1] === '--short' && args[2] === 'HEAD') {
|
||||
return { stdout: `${checkedOutBranch}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args.length === 1) {
|
||||
return { stdout: Object.keys(remotes).join('\n'), stderr: '' }
|
||||
}
|
||||
@@ -80,6 +90,29 @@ describe('prepareWorktreePushTargetWithExec', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('records repo-local provenance on the remote it adds (#17828)', async () => {
|
||||
const exec = makeRepoExec({ origin: 'git@github.com:stablyai/orca.git' })
|
||||
|
||||
await prepareWorktreePushTargetWithExec(exec, REPO, forkTarget(), () => false)
|
||||
|
||||
// Why: cleanup's ownership check must survive a store purge (worktree-push-target-cleanup.ts).
|
||||
// Narrowing the refspec (#17887) also writes `config` calls, so scope to the marker itself.
|
||||
expect(callsMatching(exec, ['config', 'remote.pr-contributor-orca.orca-created'])).toEqual([
|
||||
['config', 'remote.pr-contributor-orca.orca-created', 'true']
|
||||
])
|
||||
})
|
||||
|
||||
it('does not record provenance when reusing an existing remote', async () => {
|
||||
const exec = makeRepoExec({
|
||||
origin: 'git@github.com:stablyai/orca.git',
|
||||
'pr-contributor-orca': FORK_HTTPS
|
||||
})
|
||||
|
||||
await prepareWorktreePushTargetWithExec(exec, REPO, forkTarget(), () => false)
|
||||
|
||||
expect(callsMatching(exec, ['config', 'remote.pr-contributor-orca.orca-created'])).toEqual([])
|
||||
})
|
||||
|
||||
it('reuses an existing remote pointing at the same fork (SSH vs HTTPS) without adding', async () => {
|
||||
const exec = makeRepoExec({
|
||||
origin: 'git@github.com:stablyai/orca.git',
|
||||
@@ -158,6 +191,38 @@ describe('findRemoteForUrl', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('remoteAlreadyMatchesUrl', () => {
|
||||
it('matches an exact URL', async () => {
|
||||
const exec = makeRepoExec({ 'pr-contributor-orca': FORK_SSH })
|
||||
await expect(
|
||||
remoteAlreadyMatchesUrl(exec, REPO, 'pr-contributor-orca', FORK_SSH)
|
||||
).resolves.toBe(true)
|
||||
})
|
||||
|
||||
it('matches by GitHub owner/repo across URL protocols', async () => {
|
||||
const exec = makeRepoExec({ 'pr-contributor-orca': FORK_HTTPS })
|
||||
await expect(
|
||||
remoteAlreadyMatchesUrl(exec, REPO, 'pr-contributor-orca', FORK_SSH)
|
||||
).resolves.toBe(true)
|
||||
})
|
||||
|
||||
it('returns false when the named remote points elsewhere', async () => {
|
||||
const exec = makeRepoExec({
|
||||
'pr-contributor-orca': 'git@github.com:someone-else/orca.git'
|
||||
})
|
||||
await expect(
|
||||
remoteAlreadyMatchesUrl(exec, REPO, 'pr-contributor-orca', FORK_SSH)
|
||||
).resolves.toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when the named remote does not exist', async () => {
|
||||
const exec = makeRepoExec({ origin: 'git@github.com:stablyai/orca.git' })
|
||||
await expect(
|
||||
remoteAlreadyMatchesUrl(exec, REPO, 'pr-contributor-orca', FORK_SSH)
|
||||
).resolves.toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensureUniqueRemoteName', () => {
|
||||
it('returns the preferred name when it is free', async () => {
|
||||
const exec = makeRepoExec({ origin: 'x' })
|
||||
@@ -190,6 +255,46 @@ describe('configureCreatedWorktreePushTargetWithExec', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('restoreUpstreamAfterMaterialize', () => {
|
||||
it('points the checked-out branch upstream at the fork remote', async () => {
|
||||
const exec = makeRepoExec({}, 'local-branch')
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await restoreUpstreamAfterMaterialize(exec, '/wt/path', target)
|
||||
|
||||
expect(exec).toHaveBeenCalledWith(
|
||||
['branch', '--set-upstream-to', 'pr-contributor-orca/contributor/fix', 'local-branch'],
|
||||
'/wt/path'
|
||||
)
|
||||
expect(result).toBe(target)
|
||||
})
|
||||
|
||||
it('is a no-op when the target has no remoteUrl', async () => {
|
||||
const exec = makeRepoExec({}, 'local-branch')
|
||||
const target: GitPushTarget = { remoteName: 'origin', branchName: 'feature' }
|
||||
|
||||
const result = await restoreUpstreamAfterMaterialize(exec, '/wt/path', target)
|
||||
|
||||
expect(callsMatching(exec, ['branch', '--set-upstream-to'])).toEqual([])
|
||||
expect(result).toBe(target)
|
||||
})
|
||||
|
||||
it('is a no-op when HEAD is detached (no checked-out branch)', async () => {
|
||||
const exec = vi.fn<GitRemoteExec>(async (args: string[]) => {
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
throw new Error('fatal: ref HEAD is not a symbolic ref')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await restoreUpstreamAfterMaterialize(exec, '/wt/path', target)
|
||||
|
||||
expect(callsMatching(exec, ['branch', '--set-upstream-to'])).toEqual([])
|
||||
expect(result).toBe(target)
|
||||
})
|
||||
})
|
||||
|
||||
describe('prepareWorktreePushTargetWithExec rollback', () => {
|
||||
it('removes the remote it just added when the fetch fails', async () => {
|
||||
const remotes: Record<string, string> = { origin: 'git@github.com:stablyai/orca.git' }
|
||||
|
||||
@@ -49,6 +49,54 @@ export async function findRemoteForUrl(
|
||||
return null
|
||||
}
|
||||
|
||||
// O(1) probe used before materializing on demand (push/pull/fetch/fast-forward):
|
||||
// a single `remote get-url <name>` avoids the O(remotes) `findRemoteForUrl` scan
|
||||
// once a fork remote already exists under its expected name (#17828).
|
||||
export async function remoteAlreadyMatchesUrl(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
remoteName: string,
|
||||
remoteUrl: string
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await execGit(['remote', 'get-url', remoteName], repoPath)
|
||||
const candidateUrl = stdout.trim()
|
||||
if (candidateUrl === remoteUrl) {
|
||||
return true
|
||||
}
|
||||
const target = parseGitHubOwnerRepo(remoteUrl)
|
||||
const candidate = parseGitHubOwnerRepo(candidateUrl)
|
||||
return Boolean(
|
||||
target &&
|
||||
candidate &&
|
||||
target.owner.toLowerCase() === candidate.owner.toLowerCase() &&
|
||||
target.repo.toLowerCase() === candidate.repo.toLowerCase()
|
||||
)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Why (#17828 CodeRabbit follow-up): a deferred remote materialized after create
|
||||
// (terminal spawn, push/pull/fetch) must restore the upstream link create used to
|
||||
// configure, or raw `git pull`/`git log @{u}..` keep failing even once the remote
|
||||
// exists. The checked-out branch is resolved fresh rather than threaded through
|
||||
// every materialize call site, since `target.branchName` is the fork's PR head ref
|
||||
// and can differ from the worktree's local branch name (rename-on-collision).
|
||||
export async function resolveCheckedOutBranchName(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await execGit(['symbolic-ref', '--short', 'HEAD'], repoPath)
|
||||
const branch = stdout.trim()
|
||||
return branch.length > 0 ? branch : null
|
||||
} catch {
|
||||
// Detached HEAD or an unreadable ref -- nothing to point upstream.
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
export async function ensureUniqueRemoteName(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
@@ -103,16 +151,26 @@ export async function prepareWorktreePushTargetWithExec(
|
||||
remoteName = await ensureUniqueRemoteName(execGit, repoPath, target.remoteName)
|
||||
// Why: `-t <branch> --no-tags` means this remote is never, even transiently,
|
||||
// written with the wide default `refs/heads/*` refspec + tag auto-follow (#17828).
|
||||
// `-t` itself writes a literal (non-wildcard-suffixed) refspec, so immediately
|
||||
// rewrite it to the trailing-`*` form via `ensureRemoteTracksBranchNarrowly`
|
||||
// (see that function's comment for why the suffix matters).
|
||||
await execGit(
|
||||
['remote', 'add', '-t', target.branchName, '--no-tags', remoteName, target.remoteUrl],
|
||||
repoPath
|
||||
)
|
||||
await ensureRemoteTracksBranchNarrowly(execGit, repoPath, remoteName, target.branchName)
|
||||
remoteCreated = true
|
||||
remoteAddedHere = true
|
||||
try {
|
||||
// `-t` itself writes a literal (non-wildcard-suffixed) refspec, so immediately
|
||||
// rewrite it to the trailing-`*` form via `ensureRemoteTracksBranchNarrowly`
|
||||
// (see that function's comment for why the suffix matters).
|
||||
await ensureRemoteTracksBranchNarrowly(execGit, repoPath, remoteName, target.branchName)
|
||||
// Why: repo-local provenance that survives a store purge and is removed
|
||||
// atomically with the remote itself, unlike the store's `remoteCreated` flag.
|
||||
await execGit(['config', `remote.${remoteName}.orca-created`, 'true'], repoPath)
|
||||
} catch (error) {
|
||||
// Why: a half-configured remote with no provenance marker is unreclaimable --
|
||||
// cleanup only runs off that marker, so a failure here must undo the add.
|
||||
await execGit(['remote', 'remove', remoteName], repoPath).catch(() => {})
|
||||
throw error
|
||||
}
|
||||
remoteCreated = true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -138,6 +196,31 @@ export async function prepareWorktreePushTargetWithExec(
|
||||
}
|
||||
}
|
||||
|
||||
// Why (#17828 CodeRabbit follow-up, restructured per review): materializing the remote
|
||||
// alone isn't enough -- raw `git pull`/`git push`/`git log @{u}..` still fail without the
|
||||
// upstream link create-time configuration used to set up. This must run at the *materializer*
|
||||
// level (called by both the short-circuit and full-prepare paths in worktree-remote.ts), not
|
||||
// buried inside `prepare*`, or every call after the first materialize -- and any sibling
|
||||
// worktree that reuses the same fork remote -- never reaches it. Unconditional (not just
|
||||
// "newly added") because a reused remote's upstream for *this* worktree's branch isn't
|
||||
// guaranteed set. The checked-out branch is resolved fresh rather than threaded through
|
||||
// every materialize call site, since `target.branchName` is the fork's PR head ref and can
|
||||
// differ from the worktree's local branch name (rename-on-collision).
|
||||
export async function restoreUpstreamAfterMaterialize(
|
||||
execGit: GitRemoteExec,
|
||||
worktreePath: string,
|
||||
target: GitPushTarget
|
||||
): Promise<GitPushTarget> {
|
||||
if (!target.remoteUrl) {
|
||||
return target
|
||||
}
|
||||
const checkedOutBranch = await resolveCheckedOutBranchName(execGit, worktreePath)
|
||||
if (!checkedOutBranch) {
|
||||
return target
|
||||
}
|
||||
return configureCreatedWorktreePushTargetWithExec(execGit, worktreePath, checkedOutBranch, target)
|
||||
}
|
||||
|
||||
export async function configureCreatedWorktreePushTargetWithExec(
|
||||
execGit: GitRemoteExec,
|
||||
worktreePath: string,
|
||||
|
||||
@@ -0,0 +1,593 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { SshGitProvider } from '../providers/ssh-git-provider'
|
||||
import type { GitPushTarget } from '../../shared/worktree/types'
|
||||
import type { WorktreePushTargetStore } from './worktree-push-target-cleanup'
|
||||
|
||||
const { gitExecFileAsyncMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn() }))
|
||||
vi.mock('../git/runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock }))
|
||||
|
||||
import {
|
||||
materializeWorktreePushTargetRemote,
|
||||
materializeWorktreePushTargetRemoteSsh
|
||||
} from './worktree-remote'
|
||||
|
||||
const REPO_PATH = '/repo-root'
|
||||
const FORK_URL = 'git@github.com:contributor/orca.git'
|
||||
const FORK_REMOTE = 'pr-contributor-orca'
|
||||
|
||||
function forkTarget(overrides: Partial<GitPushTarget> = {}): GitPushTarget {
|
||||
return {
|
||||
remoteName: FORK_REMOTE,
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: FORK_URL,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('materializeWorktreePushTargetRemote', () => {
|
||||
beforeEach(() => {
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
})
|
||||
|
||||
it('is a no-op when the target already reports remoteCreated', async () => {
|
||||
const target = forkTarget({ remoteCreated: true })
|
||||
|
||||
const result = await materializeWorktreePushTargetRemote(REPO_PATH, target)
|
||||
|
||||
expect(result).toBe(target)
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('is a no-op for a same-repo target with no remoteUrl', async () => {
|
||||
const target = forkTarget({ remoteUrl: undefined })
|
||||
|
||||
const result = await materializeWorktreePushTargetRemote(REPO_PATH, target)
|
||||
|
||||
expect(result).toBe(target)
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('short-circuits the remote probe but still restores upstream and widens the refspec', async () => {
|
||||
// Why (#17828 review follow-up): the short-circuit is the common case for every call
|
||||
// after the first, and for a sibling worktree reusing the same fork remote under a
|
||||
// different branch -- it must still restore the upstream link and widen the refspec.
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: `${FORK_URL}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'config' && args[1] === '--get-all') {
|
||||
throw new Error('no such section')
|
||||
}
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
return { stdout: 'contributor/fix\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await materializeWorktreePushTargetRemote(REPO_PATH, target)
|
||||
|
||||
expect(result).toBe(target)
|
||||
const calls = gitExecFileAsyncMock.mock.calls.map((call) => call[0] as string[])
|
||||
expect(calls).toContainEqual(['remote', 'get-url', FORK_REMOTE])
|
||||
expect(calls).toContainEqual([
|
||||
'config',
|
||||
'--add',
|
||||
`remote.${FORK_REMOTE}.fetch`,
|
||||
`+refs/heads/${target.branchName}*:refs/remotes/${FORK_REMOTE}/${target.branchName}*`
|
||||
])
|
||||
expect(calls).toContainEqual(['config', `remote.${FORK_REMOTE}.tagOpt`, '--no-tags'])
|
||||
expect(calls).toContainEqual(['symbolic-ref', '--short', 'HEAD'])
|
||||
expect(calls).toContainEqual([
|
||||
'branch',
|
||||
'--set-upstream-to',
|
||||
`${FORK_REMOTE}/${target.branchName}`,
|
||||
'contributor/fix'
|
||||
])
|
||||
})
|
||||
|
||||
it('materializes the remote (add + provenance + fetch) when the probe misses', async () => {
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await materializeWorktreePushTargetRemote(REPO_PATH, target)
|
||||
|
||||
expect(result).toEqual({ ...target, remoteCreated: true })
|
||||
const calls = gitExecFileAsyncMock.mock.calls.map((call) => call[0] as string[])
|
||||
// Mint uses the narrow `-t <branch> --no-tags` add form (#17887), not a bare `remote add`.
|
||||
expect(calls).toContainEqual([
|
||||
'remote',
|
||||
'add',
|
||||
'-t',
|
||||
target.branchName,
|
||||
'--no-tags',
|
||||
FORK_REMOTE,
|
||||
FORK_URL
|
||||
])
|
||||
expect(calls).toContainEqual(['config', `remote.${FORK_REMOTE}.orca-created`, 'true'])
|
||||
expect(calls).toContainEqual([
|
||||
'fetch',
|
||||
FORK_REMOTE,
|
||||
`+refs/heads/${target.branchName}*:refs/remotes/${FORK_REMOTE}/${target.branchName}*`
|
||||
])
|
||||
})
|
||||
|
||||
it('fetches the missing tracking ref before restoring upstream on the short-circuit path (#17828 sibling worktree)', async () => {
|
||||
// Why: a sibling worktree short-circuiting onto an already-existing remote under a
|
||||
// *new* branch has a widened refspec but no tracking ref yet -- against real git,
|
||||
// `branch --set-upstream-to` hard-fails with "the requested upstream branch does not
|
||||
// exist" unless something fetches that branch first. Verified against a real git
|
||||
// fixture, not just this mock (see PR discussion).
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: `${FORK_URL}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'config' && args[1] === '--get-all') {
|
||||
throw new Error('no such section')
|
||||
}
|
||||
if (args[0] === 'rev-parse') {
|
||||
throw new Error('unknown revision')
|
||||
}
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
return { stdout: 'contributor/fix\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await materializeWorktreePushTargetRemote(REPO_PATH, target)
|
||||
|
||||
expect(result).toBe(target)
|
||||
const fetchCalls = gitExecFileAsyncMock.mock.calls.filter(
|
||||
(call) => (call[0] as string[])[0] === 'fetch'
|
||||
)
|
||||
expect(fetchCalls).toEqual([
|
||||
[
|
||||
[
|
||||
'fetch',
|
||||
FORK_REMOTE,
|
||||
`+refs/heads/${target.branchName}*:refs/remotes/${FORK_REMOTE}/${target.branchName}*`
|
||||
],
|
||||
expect.objectContaining({ timeout: expect.any(Number) })
|
||||
]
|
||||
])
|
||||
const calls = gitExecFileAsyncMock.mock.calls.map((call) => call[0] as string[])
|
||||
expect(calls).toContainEqual([
|
||||
'rev-parse',
|
||||
'--verify',
|
||||
'--quiet',
|
||||
`refs/remotes/${FORK_REMOTE}/${target.branchName}`
|
||||
])
|
||||
expect(calls).toContainEqual([
|
||||
'branch',
|
||||
'--set-upstream-to',
|
||||
`${FORK_REMOTE}/${target.branchName}`,
|
||||
'contributor/fix'
|
||||
])
|
||||
})
|
||||
|
||||
it('skips the fetch when the tracking ref already exists on the short-circuit path', async () => {
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: `${FORK_URL}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'config' && args[1] === '--get-all') {
|
||||
throw new Error('no such section')
|
||||
}
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
return { stdout: 'contributor/fix\n', stderr: '' }
|
||||
}
|
||||
// rev-parse succeeds by default (ref already exists) -- no fetch should follow.
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const target = forkTarget()
|
||||
|
||||
await materializeWorktreePushTargetRemote(REPO_PATH, target)
|
||||
|
||||
const fetchCalls = gitExecFileAsyncMock.mock.calls.filter(
|
||||
(call) => (call[0] as string[])[0] === 'fetch'
|
||||
)
|
||||
expect(fetchCalls).toEqual([])
|
||||
})
|
||||
|
||||
it("gives a joiner its own branch wiring instead of the minting sibling's target", async () => {
|
||||
// Why (#17828 review): the single flight is keyed on the remote, but the refspec widen,
|
||||
// tracking-ref fetch and upstream link are all per-branch. A sibling worktree joining an
|
||||
// in-flight mint for a *different* branch previously received the minter's target and
|
||||
// skipped all three, leaving its own branch with no upstream.
|
||||
let remoteExists = false
|
||||
let releaseAdd!: () => void
|
||||
const addGate = new Promise<void>((resolve) => {
|
||||
releaseAdd = resolve
|
||||
})
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
if (!remoteExists) {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: `${FORK_URL}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'add') {
|
||||
await addGate
|
||||
remoteExists = true
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'config' && args[1] === '--get-all') {
|
||||
throw new Error('no such section')
|
||||
}
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
return { stdout: 'joiner/branch\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
const minter = materializeWorktreePushTargetRemote(REPO_PATH, forkTarget())
|
||||
await Promise.resolve()
|
||||
const joiner = materializeWorktreePushTargetRemote(
|
||||
REPO_PATH,
|
||||
forkTarget({ branchName: 'joiner/branch' })
|
||||
)
|
||||
releaseAdd()
|
||||
const [, joined] = await Promise.all([minter, joiner])
|
||||
|
||||
// The joiner keeps its own branch rather than inheriting the minter's.
|
||||
expect(joined.branchName).toBe('joiner/branch')
|
||||
const calls = gitExecFileAsyncMock.mock.calls.map((call) => call[0] as string[])
|
||||
expect(calls).toContainEqual([
|
||||
'branch',
|
||||
'--set-upstream-to',
|
||||
`${FORK_REMOTE}/joiner/branch`,
|
||||
'joiner/branch'
|
||||
])
|
||||
// Exactly one mint: the joiner must not have raced a second `remote add`.
|
||||
expect(calls.filter((call) => call[0] === 'remote' && call[1] === 'add')).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('propagates a failed mint instead of adopting a remote the rollback removed', async () => {
|
||||
// Why (#17828 review): both mint rollbacks `remote remove`, so adopting after a failed mint
|
||||
// writes `remote.<name>.fetch` with no URL -- a config-only ghost that breaks
|
||||
// `git fetch --all`, forces later mints to a `-2` name, and survives `git remote remove`.
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'add') {
|
||||
throw new Error('mint failed')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
const minter = materializeWorktreePushTargetRemote(REPO_PATH, forkTarget())
|
||||
await Promise.resolve()
|
||||
const joiner = materializeWorktreePushTargetRemote(
|
||||
REPO_PATH,
|
||||
forkTarget({ branchName: 'joiner/branch' })
|
||||
)
|
||||
|
||||
await expect(minter).rejects.toThrow()
|
||||
await expect(joiner).rejects.toThrow()
|
||||
const calls = gitExecFileAsyncMock.mock.calls.map((call) => call[0] as string[])
|
||||
// No ghost: nothing wrote refspec or tagOpt config for a remote that does not exist.
|
||||
expect(
|
||||
calls.filter((call) => call[0] === 'config' && String(call[2] ?? '').includes(FORK_REMOTE))
|
||||
).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('persists remoteCreated to the store when a worktreeId is provided and the mint succeeds', async () => {
|
||||
// Why (#17828 review follow-up): on-demand materialization never went through the
|
||||
// create-time setWorktreeMeta write, so a lazily-minted remote stayed invisible to
|
||||
// #17842's orphan sweep (which gates solely on the stored remoteCreated flag).
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const target = forkTarget()
|
||||
const setWorktreeMeta = vi.fn()
|
||||
const store: WorktreePushTargetStore = {
|
||||
getAllWorktreeMeta: () => ({}),
|
||||
setWorktreeMeta
|
||||
} as unknown as WorktreePushTargetStore
|
||||
|
||||
const result = await materializeWorktreePushTargetRemote(
|
||||
REPO_PATH,
|
||||
target,
|
||||
store,
|
||||
undefined,
|
||||
{},
|
||||
'worktree-1'
|
||||
)
|
||||
|
||||
expect(result).toEqual({ ...target, remoteCreated: true })
|
||||
expect(setWorktreeMeta).toHaveBeenCalledWith('worktree-1', {
|
||||
pushTarget: { ...target, remoteCreated: true }
|
||||
})
|
||||
})
|
||||
|
||||
it('does not touch the store when no worktreeId is provided', async () => {
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const target = forkTarget()
|
||||
const setWorktreeMeta = vi.fn()
|
||||
const store: WorktreePushTargetStore = {
|
||||
getAllWorktreeMeta: () => ({}),
|
||||
setWorktreeMeta
|
||||
} as unknown as WorktreePushTargetStore
|
||||
|
||||
await materializeWorktreePushTargetRemote(REPO_PATH, target, store)
|
||||
|
||||
expect(setWorktreeMeta).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('materializeWorktreePushTargetRemoteSsh', () => {
|
||||
it('is a no-op when the target already reports remoteCreated', async () => {
|
||||
const exec = vi.fn()
|
||||
const target = forkTarget({ remoteCreated: true })
|
||||
|
||||
const result = await materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target
|
||||
)
|
||||
|
||||
expect(result).toBe(target)
|
||||
expect(exec).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('short-circuits the remote probe but still restores upstream (refspec widening is a local-only gap)', async () => {
|
||||
// Why: mirrors the local short-circuit's upstream restore. Refspec widening is
|
||||
// intentionally NOT mirrored here -- SSH's bare `remote add` is a pre-existing,
|
||||
// documented gap this fix does not touch.
|
||||
const exec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: `${FORK_URL}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
return { stdout: 'contributor/fix\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const fetchRemoteTrackingRef = vi.fn()
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target
|
||||
)
|
||||
|
||||
expect(result).toBe(target)
|
||||
const calls = exec.mock.calls.map((call) => call[0] as string[])
|
||||
expect(calls).toContainEqual(['remote', 'get-url', FORK_REMOTE])
|
||||
expect(calls).toContainEqual(['symbolic-ref', '--short', 'HEAD'])
|
||||
expect(calls).toContainEqual([
|
||||
'branch',
|
||||
'--set-upstream-to',
|
||||
`${FORK_REMOTE}/${target.branchName}`,
|
||||
'contributor/fix'
|
||||
])
|
||||
expect(calls.some((call) => call[0] === 'config' && String(call[2]).includes('.fetch'))).toBe(
|
||||
false
|
||||
)
|
||||
expect(fetchRemoteTrackingRef).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fetches the missing tracking ref (one-off, no config write) before restoring upstream on the short-circuit path', async () => {
|
||||
// SSH mirror of the local sibling-worktree fix: refspec widening stays out of scope
|
||||
// here, but the branch must still be fetched once before `--set-upstream-to` can
|
||||
// succeed for a branch this remote has never pulled in.
|
||||
const exec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: `${FORK_URL}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'rev-parse') {
|
||||
throw new Error('unknown revision')
|
||||
}
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
return { stdout: 'contributor/fix\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const fetchRemoteTrackingRef = vi.fn(async () => {})
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target
|
||||
)
|
||||
|
||||
expect(result).toBe(target)
|
||||
expect(fetchRemoteTrackingRef).toHaveBeenCalledWith(
|
||||
REPO_PATH,
|
||||
FORK_REMOTE,
|
||||
target.branchName,
|
||||
`refs/remotes/${FORK_REMOTE}/${target.branchName}`
|
||||
)
|
||||
const calls = exec.mock.calls.map((call) => call[0] as string[])
|
||||
expect(calls).toContainEqual([
|
||||
'branch',
|
||||
'--set-upstream-to',
|
||||
`${FORK_REMOTE}/${target.branchName}`,
|
||||
'contributor/fix'
|
||||
])
|
||||
// Still no config write -- the fetch is a one-off refspec argument, not a widen.
|
||||
expect(calls.some((call) => call[0] === 'config' && String(call[2]).includes('.fetch'))).toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('materializes the remote (add + provenance + fetch) when the probe misses', async () => {
|
||||
const exec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const fetchRemoteTrackingRef = vi.fn(async () => {})
|
||||
const markRemoteOrcaCreated = vi.fn(async () => {})
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef, markRemoteOrcaCreated } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target
|
||||
)
|
||||
|
||||
expect(result).toEqual({ ...target, remoteCreated: true })
|
||||
const calls = exec.mock.calls.map((call) => call[0] as string[])
|
||||
expect(calls).toContainEqual(['check-ref-format', '--branch', target.branchName])
|
||||
expect(calls).toContainEqual(['remote', 'add', FORK_REMOTE, FORK_URL])
|
||||
// Provenance is a narrow RPC, not exec: the relay's generic git.exec blocks config writes.
|
||||
expect(markRemoteOrcaCreated).toHaveBeenCalledWith(REPO_PATH, FORK_REMOTE)
|
||||
expect(fetchRemoteTrackingRef).toHaveBeenCalledWith(
|
||||
REPO_PATH,
|
||||
FORK_REMOTE,
|
||||
target.branchName,
|
||||
`refs/remotes/${FORK_REMOTE}/${target.branchName}`
|
||||
)
|
||||
})
|
||||
|
||||
it('persists remoteCreated to the store when a worktreeId is provided and the mint succeeds', async () => {
|
||||
const exec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const fetchRemoteTrackingRef = vi.fn(async () => {})
|
||||
const markRemoteOrcaCreated = vi.fn(async () => {})
|
||||
const target = forkTarget()
|
||||
const setWorktreeMeta = vi.fn()
|
||||
const store: WorktreePushTargetStore = {
|
||||
getAllWorktreeMeta: () => ({}),
|
||||
setWorktreeMeta
|
||||
} as unknown as WorktreePushTargetStore
|
||||
|
||||
const result = await materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef, markRemoteOrcaCreated } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target,
|
||||
store,
|
||||
undefined,
|
||||
'worktree-1'
|
||||
)
|
||||
|
||||
expect(result).toEqual({ ...target, remoteCreated: true })
|
||||
expect(setWorktreeMeta).toHaveBeenCalledWith('worktree-1', {
|
||||
pushTarget: { ...target, remoteCreated: true }
|
||||
})
|
||||
})
|
||||
|
||||
// Moved from worktrees-ssh-fork-push-target-remote.test.ts: this behavior lives in
|
||||
// prepareWorktreePushTargetSsh (invoked here through the materialize wrapper, once
|
||||
// the fast probe misses) and is unchanged -- it just no longer runs at create time.
|
||||
it('names the relay upgrade when an older host still rejects the fork remote', async () => {
|
||||
const exec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'add') {
|
||||
throw new Error('Destructive git remote operations are not allowed via exec')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const fetchRemoteTrackingRef = vi.fn()
|
||||
const target = forkTarget()
|
||||
|
||||
await expect(
|
||||
materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target
|
||||
)
|
||||
).rejects.toThrow('Reconnect to deploy the latest relay')
|
||||
expect(fetchRemoteTrackingRef).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('drops the fork remote it just added when the SSH head fetch fails', async () => {
|
||||
const exec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const fetchRemoteTrackingRef = vi.fn(async () => {
|
||||
throw new Error('network unreachable')
|
||||
})
|
||||
const markRemoteOrcaCreated = vi.fn(async () => {})
|
||||
const target = forkTarget()
|
||||
|
||||
await expect(
|
||||
materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef, markRemoteOrcaCreated } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target
|
||||
)
|
||||
).rejects.toThrow('network unreachable')
|
||||
|
||||
expect(exec).toHaveBeenCalledWith(['remote', 'remove', FORK_REMOTE], REPO_PATH)
|
||||
})
|
||||
|
||||
// Regression: the rollback must not fire on ownership inherited from a sibling
|
||||
// worktree, deleting the remote that worktree is still pushing through. The probe
|
||||
// misses under the *requested* remote name so this reaches prepareWorktreePushTargetSsh's
|
||||
// own by-URL reuse scan, which finds the sibling's differently-named remote.
|
||||
it('keeps a reused fork remote a sibling worktree owns when the SSH head fetch fails', async () => {
|
||||
const SIBLING_REMOTE = 'pr-contributor-orca-existing'
|
||||
const exec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
if (args[2] === SIBLING_REMOTE) {
|
||||
return { stdout: `${FORK_URL}\n`, stderr: '' }
|
||||
}
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
if (args[0] === 'remote' && args.length === 1) {
|
||||
return { stdout: `origin\n${SIBLING_REMOTE}\n`, stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const fetchRemoteTrackingRef = vi.fn(async () => {
|
||||
throw new Error('network unreachable')
|
||||
})
|
||||
const target = forkTarget()
|
||||
const store: WorktreePushTargetStore = {
|
||||
getAllWorktreeMeta: () => ({
|
||||
'repo::/repo-root-sibling': {
|
||||
pushTarget: {
|
||||
remoteName: SIBLING_REMOTE,
|
||||
branchName: 'contributor/other',
|
||||
remoteUrl: FORK_URL,
|
||||
remoteCreated: true
|
||||
}
|
||||
}
|
||||
})
|
||||
} as unknown as WorktreePushTargetStore
|
||||
|
||||
await expect(
|
||||
materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef } as unknown as SshGitProvider,
|
||||
REPO_PATH,
|
||||
target,
|
||||
store
|
||||
)
|
||||
).rejects.toThrow('network unreachable')
|
||||
|
||||
expect(exec).not.toHaveBeenCalledWith(['remote', 'remove', SIBLING_REMOTE], REPO_PATH)
|
||||
expect(exec).not.toHaveBeenCalledWith(
|
||||
['remote', 'add', expect.anything(), expect.anything()],
|
||||
REPO_PATH
|
||||
)
|
||||
})
|
||||
})
|
||||
+325
-34
@@ -112,8 +112,15 @@ import {
|
||||
configureCreatedWorktreePushTargetWithExec,
|
||||
ensureUniqueRemoteName,
|
||||
findRemoteForUrl,
|
||||
prepareWorktreePushTargetWithExec
|
||||
prepareWorktreePushTargetWithExec,
|
||||
remoteAlreadyMatchesUrl,
|
||||
restoreUpstreamAfterMaterialize
|
||||
} from './worktree-push-target-setup'
|
||||
import {
|
||||
buildNarrowForkFetchRefspec,
|
||||
ensureRemoteTracksBranchNarrowly,
|
||||
forkRemoteTrackingRefExists
|
||||
} from '../git/fork-remote-refspec'
|
||||
import { migrateForkRemoteRefspecs } from './worktree-push-target-refspec-migration'
|
||||
import { isENOENT } from './filesystem-path-containment'
|
||||
import {
|
||||
@@ -166,9 +173,36 @@ const SSH_WORKTREE_CREATE_FETCH_FRESHNESS_MS = 30_000
|
||||
const SSH_WORKTREE_CREATE_FETCH_CACHE_MAX = 512
|
||||
// Why: bound the fallback `git fetch origin` so a Windows credential-manager GUI hang (STA-1292) can't wedge worktree creation forever.
|
||||
const CREATE_BASE_FALLBACK_FETCH_TIMEOUT_MS = 60_000
|
||||
// Why (#17828 CodeRabbit follow-up): the deferred materialize fetch runs off the main
|
||||
// create path (terminal spawn, mid-session sync) with nothing else bounding it -- same
|
||||
// STA-1292 hang risk as the create-time fallback above, so mirror its timeout.
|
||||
const DEFERRED_PUSH_TARGET_FETCH_TIMEOUT_MS = 60_000
|
||||
const sshWorktreeCreateFetchInflight = new Map<string, Promise<void>>()
|
||||
const sshWorktreeCreateFetchCompletedAt = new Map<string, number>()
|
||||
const sshWorktreeCreateFetchQueueTail = new Map<string, Promise<void>>()
|
||||
// Why (#17828 CodeRabbit follow-up): a terminal spawn and an explicit sync action can
|
||||
// both call materialize for the same worktree remote at once; without single-flighting,
|
||||
// the loser's `remote add` races the winner's fetch and can strand a duplicate remote.
|
||||
const worktreePushTargetMaterializeInflight = new Map<string, Promise<GitPushTarget>>()
|
||||
const sshWorktreePushTargetMaterializeInflight = new WeakMap<
|
||||
SshGitProvider,
|
||||
Map<string, Promise<GitPushTarget>>
|
||||
>()
|
||||
|
||||
function worktreePushTargetMaterializeKey(repoPath: string, remoteName: string): string {
|
||||
return `${repoPath}::${remoteName}`
|
||||
}
|
||||
|
||||
function getSshWorktreePushTargetMaterializeInflight(
|
||||
provider: SshGitProvider
|
||||
): Map<string, Promise<GitPushTarget>> {
|
||||
let inflight = sshWorktreePushTargetMaterializeInflight.get(provider)
|
||||
if (!inflight) {
|
||||
inflight = new Map()
|
||||
sshWorktreePushTargetMaterializeInflight.set(provider, inflight)
|
||||
}
|
||||
return inflight
|
||||
}
|
||||
const sshWorktreeCreateBasePlanInflight = new Map<
|
||||
string,
|
||||
Promise<RemoteWorktreeCreateBasePlan | null>
|
||||
@@ -972,7 +1006,16 @@ export async function prepareWorktreePushTarget(
|
||||
): Promise<GitPushTarget> {
|
||||
await validateGitPushTarget(repoPath, target, gitOptions)
|
||||
const prepared = await prepareWorktreePushTargetWithExec(
|
||||
(args, cwd) => gitExecFileAsync(args, { cwd, ...gitOptions }),
|
||||
// Why: this is only ever reached via the deferred materialize path (#17828) -- bound
|
||||
// just the network fetch so it can't hang indefinitely (see the timeout constant's
|
||||
// comment). The other calls this makes (`remote`, `remote add`, `config`) are local-only
|
||||
// and must stay untimed, matching every other local git call in this file.
|
||||
(args, cwd) =>
|
||||
gitExecFileAsync(args, {
|
||||
cwd,
|
||||
...gitOptions,
|
||||
...(args[0] === 'fetch' ? { timeout: DEFERRED_PUSH_TARGET_FETCH_TIMEOUT_MS } : {})
|
||||
}),
|
||||
repoPath,
|
||||
target,
|
||||
(existingRemote) =>
|
||||
@@ -993,6 +1036,170 @@ export async function prepareWorktreePushTarget(
|
||||
return prepared
|
||||
}
|
||||
|
||||
// Why: on-demand twin of `prepareWorktreePushTarget` for push/pull/fetch/
|
||||
// fast-forward (#17828) -- a deferred fork remote is materialized the first
|
||||
// time it's needed. The cheap named-remote probe keeps every push after the
|
||||
// first one down to a handful of extra subprocesses (probe, refspec-widen,
|
||||
// upstream-restore) instead of repeating the O(remotes) scan
|
||||
// `prepareWorktreePushTargetWithExec` does when it must add.
|
||||
export async function materializeWorktreePushTargetRemote(
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
store?: WorktreePushTargetStore,
|
||||
repoId?: string,
|
||||
gitOptions: { wslDistro?: string } = {},
|
||||
worktreeId?: string
|
||||
): Promise<GitPushTarget> {
|
||||
if (!target.remoteUrl || target.remoteCreated) {
|
||||
return target
|
||||
}
|
||||
const execGit: GitRemoteExec = (args, cwd) => gitExecFileAsync(args, { cwd, ...gitOptions })
|
||||
if (await remoteAlreadyMatchesUrl(execGit, repoPath, target.remoteName, target.remoteUrl)) {
|
||||
return runForkRemoteAdoption(repoPath, target, () =>
|
||||
adoptExistingForkRemoteForBranch(
|
||||
execGit,
|
||||
repoPath,
|
||||
target,
|
||||
gitOptions,
|
||||
store,
|
||||
repoId,
|
||||
worktreeId
|
||||
)
|
||||
)
|
||||
}
|
||||
const key = worktreePushTargetMaterializeKey(repoPath, target.remoteName)
|
||||
const existing = worktreePushTargetMaterializeInflight.get(key)
|
||||
if (existing) {
|
||||
// Why: the single flight is keyed on the *remote*, but everything after the remote add is
|
||||
// per-branch. A joiner waiting on a sibling worktree's mint must not take that sibling's
|
||||
// target -- it would inherit the sibling's branch and silently skip its own refspec widen,
|
||||
// tracking-ref fetch, and upstream link. Wait for the remote, then do its own.
|
||||
//
|
||||
// Why not swallow the rejection: both mint rollbacks remove the remote, so adopting after a
|
||||
// failed mint would write `remote.<name>.fetch` with no URL -- a config-only ghost that
|
||||
// breaks `git fetch --all`, forces every later mint to a `-2` name, and survives
|
||||
// `git remote remove`. Propagate instead; the map is already cleared, so a retry re-mints.
|
||||
await existing
|
||||
return runForkRemoteAdoption(repoPath, target, () =>
|
||||
adoptExistingForkRemoteForBranch(
|
||||
execGit,
|
||||
repoPath,
|
||||
target,
|
||||
gitOptions,
|
||||
store,
|
||||
repoId,
|
||||
worktreeId
|
||||
)
|
||||
)
|
||||
}
|
||||
const promise = prepareWorktreePushTarget(repoPath, target, store, repoId, gitOptions)
|
||||
.then((prepared) => restoreUpstreamAfterMaterialize(execGit, repoPath, prepared))
|
||||
.then((prepared) => {
|
||||
persistMaterializedPushTargetIfCreated(store, worktreeId, prepared)
|
||||
return prepared
|
||||
})
|
||||
.finally(() => {
|
||||
if (worktreePushTargetMaterializeInflight.get(key) === promise) {
|
||||
worktreePushTargetMaterializeInflight.delete(key)
|
||||
}
|
||||
})
|
||||
worktreePushTargetMaterializeInflight.set(key, promise)
|
||||
return promise
|
||||
}
|
||||
|
||||
// Why: the remote already exists -- minted by an earlier call, by create, or by a sibling
|
||||
// worktree. Everything left is per-branch, and it must run for *this* target: the refspec
|
||||
// widen, the tracking-ref fetch, and the upstream link. Previously these only ran inside
|
||||
// prepareWorktreePushTarget, unreachable once the remote was there.
|
||||
async function adoptExistingForkRemoteForBranch(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
gitOptions: { wslDistro?: string },
|
||||
store: WorktreePushTargetStore | undefined,
|
||||
repoId: string | undefined,
|
||||
worktreeId: string | undefined
|
||||
): Promise<GitPushTarget> {
|
||||
await ensureRemoteTracksBranchNarrowly(execGit, repoPath, target.remoteName, target.branchName)
|
||||
// Why: widening only rewrites config -- it never imports anything. For a sibling worktree's
|
||||
// first materialize of a *new* branch on an already-existing remote, the branch's tracking
|
||||
// ref doesn't exist yet, and `--set-upstream-to` below hard-fails with "the requested
|
||||
// upstream branch does not exist" (verified against real git). Skip the fetch when the ref
|
||||
// is already there so a repeat push/pull materialize stays a local-only probe.
|
||||
if (
|
||||
!(await forkRemoteTrackingRefExists(execGit, repoPath, target.remoteName, target.branchName))
|
||||
) {
|
||||
// Why: a network fetch, unlike the local-only probes above -- bound it the same as the
|
||||
// full-mint path's fetch so it can't hang indefinitely.
|
||||
await gitExecFileAsync(
|
||||
[
|
||||
'fetch',
|
||||
target.remoteName,
|
||||
buildNarrowForkFetchRefspec(target.remoteName, target.branchName)
|
||||
],
|
||||
{ cwd: repoPath, ...gitOptions, timeout: DEFERRED_PUSH_TARGET_FETCH_TIMEOUT_MS }
|
||||
)
|
||||
}
|
||||
const restored = await restoreUpstreamAfterMaterialize(execGit, repoPath, target)
|
||||
// Why: a remote another worktree minted is still Orca-owned. Without stamping ownership on
|
||||
// the adopting worktree too, removing the minter leaves the survivor's metadata unowned and
|
||||
// #17842's sweep -- which gates solely on `remoteCreated` -- can never reclaim the remote.
|
||||
// Why derive: no caller supplies both -- IPC handlers pass a store with no repo id, runtime
|
||||
// commands pass a repo id with no store -- so requiring both made this branch unreachable.
|
||||
const ownerRepoId = repoId ?? (worktreeId ? getRepoIdFromWorktreeId(worktreeId) : undefined)
|
||||
const owned =
|
||||
store !== undefined &&
|
||||
ownerRepoId !== undefined &&
|
||||
isPushTargetRemoteCreatedByKnownWorktree(store, restored, ownerRepoId)
|
||||
const adopted = owned ? { ...restored, remoteCreated: true } : restored
|
||||
persistMaterializedPushTargetIfCreated(store, worktreeId, adopted)
|
||||
return adopted
|
||||
}
|
||||
|
||||
// Why: the mint single flight only covers `remote add`. Every adopter afterwards writes
|
||||
// `remote.<name>.fetch` and `.tagOpt`, and concurrent `git config --add` has no lock retry --
|
||||
// measured 135/160 failures at 8-way concurrency, plus duplicate refspecs when two adopts add
|
||||
// the same value. Chain adopts per remote so they serialize instead of fanning out.
|
||||
const forkRemoteAdoptionQueue = new Map<string, Promise<unknown>>()
|
||||
|
||||
function runForkRemoteAdoption<T>(
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
run: () => Promise<T>
|
||||
): Promise<T> {
|
||||
const key = worktreePushTargetMaterializeKey(repoPath, target.remoteName)
|
||||
const previous = forkRemoteAdoptionQueue.get(key)
|
||||
const next = previous ? previous.then(run, run) : run()
|
||||
const settled = next.then(
|
||||
() => undefined,
|
||||
() => undefined
|
||||
)
|
||||
forkRemoteAdoptionQueue.set(key, settled)
|
||||
void settled.finally(() => {
|
||||
if (forkRemoteAdoptionQueue.get(key) === settled) {
|
||||
forkRemoteAdoptionQueue.delete(key)
|
||||
}
|
||||
})
|
||||
return next
|
||||
}
|
||||
|
||||
// Why (review follow-up): on-demand materialization never went through the create-time
|
||||
// `setWorktreeMeta` write, so the store's `pushTarget.remoteCreated` flag stayed stale
|
||||
// forever for a lazily-minted remote -- invisible to #17842's orphan sweep
|
||||
// (`shouldReclaimPrRemote` gates solely on that flag) and to any SSH host whose relay
|
||||
// predates `markRemoteOrcaCreated` (no git-config marker either). `setWorktreeMeta` is
|
||||
// optional on `WorktreePushTargetStore` so narrow test/reconciliation stores keep compiling.
|
||||
function persistMaterializedPushTargetIfCreated(
|
||||
store: WorktreePushTargetStore | undefined,
|
||||
worktreeId: string | undefined,
|
||||
target: GitPushTarget
|
||||
): void {
|
||||
if (!target.remoteCreated || !worktreeId || !store?.setWorktreeMeta) {
|
||||
return
|
||||
}
|
||||
store.setWorktreeMeta(worktreeId, { pushTarget: target })
|
||||
}
|
||||
|
||||
function isPushTargetRemoteCreatedByKnownWorktree(
|
||||
store: WorktreePushTargetStore,
|
||||
target: GitPushTarget,
|
||||
@@ -1062,7 +1269,7 @@ export async function configureCreatedWorktreePushTarget(
|
||||
)
|
||||
}
|
||||
|
||||
async function prepareWorktreePushTargetSsh(
|
||||
export async function prepareWorktreePushTargetSsh(
|
||||
provider: SshGitProvider,
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
@@ -1106,8 +1313,18 @@ async function prepareWorktreePushTargetSsh(
|
||||
}
|
||||
throw error
|
||||
}
|
||||
remoteCreated = true
|
||||
remoteAddedHere = true
|
||||
try {
|
||||
// Why: repo-local provenance mirroring the local path (worktree-push-target-setup.ts).
|
||||
// A narrow RPC, not provider.exec: the relay's generic git.exec blocks all config writes.
|
||||
await provider.markRemoteOrcaCreated(repoPath, remoteName)
|
||||
} catch (error) {
|
||||
// Why: a remote with no provenance marker is unreclaimable -- cleanup only
|
||||
// runs off that marker, so a failure here must undo the add.
|
||||
await provider.exec(['remote', 'remove', remoteName], repoPath).catch(() => {})
|
||||
throw error
|
||||
}
|
||||
remoteCreated = true
|
||||
}
|
||||
}
|
||||
try {
|
||||
@@ -1129,6 +1346,96 @@ async function prepareWorktreePushTargetSsh(
|
||||
return { ...sanitizedTarget, remoteName, ...(remoteCreated ? { remoteCreated: true } : {}) }
|
||||
}
|
||||
|
||||
// SSH twin of `adoptExistingForkRemoteForBranch`. Refspec widening is intentionally absent --
|
||||
// SSH's bare `remote add` (no `-t`/`--no-tags`) is a pre-existing, documented gap -- but the
|
||||
// tracking ref must still exist before `--set-upstream-to` can succeed, and the upstream link
|
||||
// must be made against *this* target's branch rather than a minting sibling's.
|
||||
async function adoptExistingSshForkRemoteForBranch(
|
||||
provider: SshGitProvider,
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
store: WorktreePushTargetStore | undefined,
|
||||
worktreeId: string | undefined
|
||||
): Promise<GitPushTarget> {
|
||||
if (
|
||||
!(await forkRemoteTrackingRefExists(execGit, repoPath, target.remoteName, target.branchName))
|
||||
) {
|
||||
await provider.fetchRemoteTrackingRef(
|
||||
repoPath,
|
||||
target.remoteName,
|
||||
target.branchName,
|
||||
`refs/remotes/${target.remoteName}/${target.branchName}`
|
||||
)
|
||||
}
|
||||
const restored = await restoreUpstreamAfterMaterialize(execGit, repoPath, target)
|
||||
const ownerRepoId = worktreeId ? getRepoIdFromWorktreeId(worktreeId) : undefined
|
||||
const owned =
|
||||
store !== undefined &&
|
||||
ownerRepoId !== undefined &&
|
||||
isPushTargetRemoteCreatedByKnownWorktree(store, restored, ownerRepoId)
|
||||
const adopted = owned ? { ...restored, remoteCreated: true } : restored
|
||||
persistMaterializedPushTargetIfCreated(store, worktreeId, adopted)
|
||||
return adopted
|
||||
}
|
||||
|
||||
// SSH twin of `materializeWorktreePushTargetRemote` -- the relay has no store
|
||||
// access and trusts `pushTarget.remoteName` already exists, so a deferred fork
|
||||
// remote must be materialized client-side before dispatching push/pull/fetch/
|
||||
// fast-forward over the mux (#17828).
|
||||
export async function materializeWorktreePushTargetRemoteSsh(
|
||||
provider: SshGitProvider,
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
store?: WorktreePushTargetStore,
|
||||
repoId?: string,
|
||||
worktreeId?: string
|
||||
): Promise<GitPushTarget> {
|
||||
if (!target.remoteUrl || target.remoteCreated) {
|
||||
return target
|
||||
}
|
||||
const execGit: GitRemoteExec = (args, cwd) => provider.exec(args, cwd)
|
||||
if (await remoteAlreadyMatchesUrl(execGit, repoPath, target.remoteName, target.remoteUrl)) {
|
||||
// Why (review follow-up): mirrors the local short-circuit's upstream restore. Refspec
|
||||
// widening is intentionally NOT mirrored here -- SSH's bare `remote add` (no `-t`/
|
||||
// `--no-tags`, see prepareWorktreePushTargetSsh) is a pre-existing, documented gap this
|
||||
// fix does not touch.
|
||||
//
|
||||
// The tracking ref itself, though, must still exist before `--set-upstream-to` below
|
||||
// can succeed -- a reused remote's wide default refspec covers a future bare fetch,
|
||||
// but imports nothing on its own. Fetch just this branch (a one-off refspec argument,
|
||||
// not a config write) when it isn't already there; skip it otherwise so a repeat
|
||||
// push/pull materialize stays a local-only probe with no relay round-trip.
|
||||
return runForkRemoteAdoption(repoPath, target, () =>
|
||||
adoptExistingSshForkRemoteForBranch(provider, execGit, repoPath, target, store, worktreeId)
|
||||
)
|
||||
}
|
||||
const inflight = getSshWorktreePushTargetMaterializeInflight(provider)
|
||||
const key = worktreePushTargetMaterializeKey(repoPath, target.remoteName)
|
||||
const existing = inflight.get(key)
|
||||
if (existing) {
|
||||
// Why: same per-branch reasoning as the local twin -- a joiner must not inherit the
|
||||
// minter's branch. Rejection propagates rather than adopting a remote the rollback removed.
|
||||
await existing
|
||||
return runForkRemoteAdoption(repoPath, target, () =>
|
||||
adoptExistingSshForkRemoteForBranch(provider, execGit, repoPath, target, store, worktreeId)
|
||||
)
|
||||
}
|
||||
const promise = prepareWorktreePushTargetSsh(provider, repoPath, target, store, repoId)
|
||||
.then((prepared) => restoreUpstreamAfterMaterialize(execGit, repoPath, prepared))
|
||||
.then((prepared) => {
|
||||
persistMaterializedPushTargetIfCreated(store, worktreeId, prepared)
|
||||
return prepared
|
||||
})
|
||||
.finally(() => {
|
||||
if (inflight.get(key) === promise) {
|
||||
inflight.delete(key)
|
||||
}
|
||||
})
|
||||
inflight.set(key, promise)
|
||||
return promise
|
||||
}
|
||||
|
||||
export async function cleanupUnusedWorktreePushTargetRemoteSsh(
|
||||
provider: SshGitProvider,
|
||||
repoPath: string,
|
||||
@@ -1763,17 +2070,9 @@ export async function createRemoteWorktree(
|
||||
}
|
||||
}
|
||||
|
||||
let preparedPushTarget: GitPushTarget | undefined
|
||||
if (args.pushTarget) {
|
||||
// Why: fork-PR SSH worktrees need contributor-remote setup before create, else Push/Sync target origin.
|
||||
preparedPushTarget = await prepareWorktreePushTargetSsh(
|
||||
provider,
|
||||
repo.path,
|
||||
args.pushTarget,
|
||||
store,
|
||||
repo.id
|
||||
)
|
||||
}
|
||||
// Why: defer the remote add + fetch to first push/pull/fetch/fast-forward
|
||||
// (#17828) instead of paying it at create time for a read-only review.
|
||||
const preparedPushTarget: GitPushTarget | undefined = args.pushTarget
|
||||
|
||||
try {
|
||||
await timing.time('git_worktree_add', async () =>
|
||||
@@ -1854,8 +2153,10 @@ export async function createRemoteWorktree(
|
||||
const now = Date.now()
|
||||
// Why: PR/MR worktrees start from a head ref/SHA but Source Control must compare against the review target branch.
|
||||
const metadataBaseRef = args.compareBaseRef ?? remoteTrackingBase?.ref ?? baseBranch
|
||||
let configuredPushTarget: GitPushTarget | undefined
|
||||
if (preparedPushTarget) {
|
||||
// Why: `--set-upstream-to` needs the remote to exist -- true for a same-repo
|
||||
// target but not for a fork remote, which materializes lazily (#17828).
|
||||
let configuredPushTarget: GitPushTarget | undefined = preparedPushTarget
|
||||
if (preparedPushTarget && !preparedPushTarget.remoteUrl) {
|
||||
configuredPushTarget = await configureCreatedWorktreePushTargetWithExec(
|
||||
(args, cwd) => provider.exec(args, cwd),
|
||||
created.path,
|
||||
@@ -2369,20 +2670,9 @@ export async function createLocalWorktree(
|
||||
}
|
||||
emitCreateWorktreeProgress(mainWindow, 'creating', args.creationId)
|
||||
|
||||
let preparedPushTarget: GitPushTarget | undefined
|
||||
const requestedPushTarget = args.pushTarget
|
||||
if (requestedPushTarget) {
|
||||
// Why: validate/fetch the contributor remote before create so a failure doesn't leave a half-created worktree with conflicts on retry.
|
||||
preparedPushTarget = await timing.time('prepare_push_target', () =>
|
||||
prepareWorktreePushTarget(
|
||||
repo.path,
|
||||
requestedPushTarget,
|
||||
store,
|
||||
repo.id,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
)
|
||||
}
|
||||
// Why: defer the remote add + fetch to first push/pull/fetch/fast-forward
|
||||
// (#17828) instead of paying it at create time for a read-only review.
|
||||
const preparedPushTarget: GitPushTarget | undefined = args.pushTarget
|
||||
|
||||
const suggestLocalBaseRefUpdate =
|
||||
!settings.refreshLocalBaseRefOnWorktreeCreate &&
|
||||
@@ -2522,9 +2812,10 @@ export async function createLocalWorktree(
|
||||
await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName)
|
||||
}
|
||||
|
||||
let configuredPushTarget: GitPushTarget | undefined
|
||||
if (preparedPushTarget) {
|
||||
// Why: fork-PR review worktrees publish back to the PR author's branch; set upstream so Push/Sync use the contributor remote, not origin.
|
||||
// Why: `--set-upstream-to` needs the remote to exist -- true for a same-repo
|
||||
// target but not for a fork remote, which materializes lazily (#17828).
|
||||
let configuredPushTarget: GitPushTarget | undefined = preparedPushTarget
|
||||
if (preparedPushTarget && !preparedPushTarget.remoteUrl) {
|
||||
configuredPushTarget = await configureCreatedWorktreePushTarget(
|
||||
worktreePath,
|
||||
branchName,
|
||||
|
||||
@@ -427,7 +427,10 @@ describe('registerWorktreeHandlers', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('configures a PR push target during local create', async () => {
|
||||
// Was "configures a PR push target during local create": create used to mint the
|
||||
// fork remote up front. It now defers to first sync (#17828); the minting itself is
|
||||
// covered by worktree-remote-push-target-materialization.test.ts.
|
||||
it('defers the fork-PR remote during local create and persists the target unmaterialized', async () => {
|
||||
listWorktreesMock.mockResolvedValue([
|
||||
{
|
||||
path: '/workspace/improve-dashboard',
|
||||
@@ -449,7 +452,7 @@ describe('registerWorktreeHandlers', () => {
|
||||
}
|
||||
})
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith(
|
||||
[
|
||||
'remote',
|
||||
'add',
|
||||
@@ -461,7 +464,7 @@ describe('registerWorktreeHandlers', () => {
|
||||
],
|
||||
{ cwd: '/workspace/repo' }
|
||||
)
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith(
|
||||
[
|
||||
'fetch',
|
||||
'pr-prateek-orca',
|
||||
@@ -469,7 +472,8 @@ describe('registerWorktreeHandlers', () => {
|
||||
],
|
||||
{ cwd: '/workspace/repo' }
|
||||
)
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
// Upstream can only be set once the remote exists, so it defers with the remote.
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith(
|
||||
[
|
||||
'branch',
|
||||
'--set-upstream-to',
|
||||
@@ -478,20 +482,25 @@ describe('registerWorktreeHandlers', () => {
|
||||
],
|
||||
{ cwd: '/workspace/improve-dashboard' }
|
||||
)
|
||||
// Exact object, not objectContaining: `remoteCreated` must stay absent until
|
||||
// something actually mints the remote.
|
||||
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
|
||||
'repo-1::/workspace/improve-dashboard',
|
||||
expect.objectContaining({
|
||||
pushTarget: expect.objectContaining({
|
||||
pushTarget: {
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
remoteUrl: 'git@github.com:prateek/orca.git',
|
||||
remoteCreated: true
|
||||
})
|
||||
remoteUrl: 'git@github.com:prateek/orca.git'
|
||||
}
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps the Orca-created marker when a new worktree reuses an Orca-created fork remote', async () => {
|
||||
// Was "keeps the Orca-created marker ...": create used to inherit the marker while
|
||||
// minting. With minting deferred (#17828) create must not claim ownership it has not
|
||||
// earned; marker inheritance now happens at materialization and is covered by
|
||||
// worktree-push-target-setup.test.ts.
|
||||
it('does not claim the Orca-created marker at create when a sibling worktree minted the fork remote', async () => {
|
||||
listWorktreesMock.mockResolvedValue([
|
||||
{
|
||||
path: '/workspace/improve-dashboard',
|
||||
@@ -538,12 +547,11 @@ describe('registerWorktreeHandlers', () => {
|
||||
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
|
||||
'repo-1::/workspace/improve-dashboard',
|
||||
expect.objectContaining({
|
||||
pushTarget: expect.objectContaining({
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/new-fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
})
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
}
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
@@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { validateGitExecArgs } from '../../relay/git-exec-validator'
|
||||
import { getSshGitProviderMock, getActiveMultiplexerMock } from './worktrees-test-module-mocks'
|
||||
import { handlers, setupWorktreeHandlers, store } from './worktrees-test-harness'
|
||||
import { materializeWorktreePushTargetRemoteSsh } from './worktree-remote'
|
||||
import type { SshGitProvider } from '../providers/ssh-git-provider'
|
||||
|
||||
vi.mock('electron', async () =>
|
||||
(await import('./worktrees-test-module-mocks')).electronModuleMock()
|
||||
@@ -90,7 +92,10 @@ describe('registerWorktreeHandlers', () => {
|
||||
setupWorktreeHandlers()
|
||||
})
|
||||
|
||||
it('adds the fork remote for an SSH fork-PR worktree through git.exec', async () => {
|
||||
// Was "adds the fork remote ... through git.exec": create used to mint the fork
|
||||
// remote unconditionally. It now defers to first sync (#17828) -- split in two so
|
||||
// each half stays true to a single claim: create stays a no-op, sync still mints.
|
||||
it('defers minting the fork remote for an SSH fork-PR worktree until first sync', async () => {
|
||||
const repo = {
|
||||
id: 'repo-ssh',
|
||||
path: '/remote/repo',
|
||||
@@ -147,11 +152,13 @@ describe('registerWorktreeHandlers', () => {
|
||||
}
|
||||
})
|
||||
|
||||
expect(exec).toHaveBeenCalledWith(
|
||||
expect(exec).not.toHaveBeenCalledWith(
|
||||
['remote', 'add', 'pr-contributor-orca', 'https://github.com/contributor/orca.git'],
|
||||
'/remote/repo'
|
||||
)
|
||||
expect(provider.fetchRemoteTrackingRef).toHaveBeenCalledWith(
|
||||
// fetchRemoteTrackingRef IS called once here, but for create's unrelated
|
||||
// base-ref refresh (origin/main) -- not for the fork remote, which defers.
|
||||
expect(provider.fetchRemoteTrackingRef).not.toHaveBeenCalledWith(
|
||||
'/remote/repo',
|
||||
'pr-contributor-orca',
|
||||
'contributor/fix',
|
||||
@@ -163,201 +170,58 @@ describe('registerWorktreeHandlers', () => {
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
}
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('names the relay upgrade when an older host still rejects the fork remote', async () => {
|
||||
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' && args[1] === 'add') {
|
||||
throw new Error('Destructive git remote operations are not allowed via exec')
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: 'git@github.com:stablyai/orca.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args.length === 1) {
|
||||
return { stdout: 'origin\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'show-ref') {
|
||||
throw Object.assign(new Error('missing exact ref'), { code: 1 })
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
}),
|
||||
fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined),
|
||||
addWorktree: vi.fn().mockResolvedValue(undefined),
|
||||
listWorktrees: vi.fn().mockResolvedValue([])
|
||||
}
|
||||
const mux = { request: vi.fn().mockResolvedValue(undefined), notify: vi.fn() }
|
||||
store.getRepos.mockReturnValue([repo])
|
||||
store.getRepo.mockReturnValue(repo)
|
||||
getSshGitProviderMock.mockReturnValue(provider)
|
||||
getActiveMultiplexerMock.mockReturnValue(mux)
|
||||
|
||||
await expect(
|
||||
handlers['worktrees:create'](null, {
|
||||
repoId: 'repo-ssh',
|
||||
name: 'contributor-fix',
|
||||
branchNameOverride: 'contributor/fix',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
}
|
||||
})
|
||||
).rejects.toThrow('Reconnect to deploy the latest relay')
|
||||
expect(provider.addWorktree).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('drops the fork remote it just added when the SSH head fetch fails', async () => {
|
||||
const repo = {
|
||||
id: 'repo-ssh',
|
||||
path: '/remote/repo',
|
||||
displayName: 'ssh',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
connectionId: 'conn-1',
|
||||
worktreeBaseRef: 'origin/main'
|
||||
}
|
||||
// Companion to the deferral test above: `materializeWorktreePushTargetRemoteSsh` is
|
||||
// exactly what `git:push`/`git:pull`'s SSH dispatch calls before syncing, so this is
|
||||
// "first sync" without needing the sync IPC handlers registered in this harness.
|
||||
it('mints the fork remote for an SSH fork-PR worktree on first sync', async () => {
|
||||
const exec = vi.fn().mockImplementation(async (args: string[]) => {
|
||||
validateGitExecArgs(args)
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: 'git@github.com:stablyai/orca.git\n', stderr: '' }
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
if (args[0] === 'remote' && args.length === 1) {
|
||||
return { stdout: 'origin\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'show-ref') {
|
||||
throw Object.assign(new Error('missing exact ref'), { code: 1 })
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const provider = {
|
||||
exec,
|
||||
fetchRemoteTrackingRef: vi
|
||||
.fn()
|
||||
.mockImplementation(async (_repoPath: string, remote: string) => {
|
||||
if (remote === 'pr-contributor-orca') {
|
||||
throw new Error('network unreachable')
|
||||
}
|
||||
}),
|
||||
addWorktree: vi.fn().mockResolvedValue(undefined),
|
||||
listWorktrees: vi.fn().mockResolvedValue([])
|
||||
const fetchRemoteTrackingRef = vi.fn().mockResolvedValue(undefined)
|
||||
const markRemoteOrcaCreated = vi.fn().mockResolvedValue(undefined)
|
||||
const target = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
}
|
||||
const mux = { request: vi.fn().mockResolvedValue(undefined), notify: vi.fn() }
|
||||
store.getRepos.mockReturnValue([repo])
|
||||
store.getRepo.mockReturnValue(repo)
|
||||
getSshGitProviderMock.mockReturnValue(provider)
|
||||
getActiveMultiplexerMock.mockReturnValue(mux)
|
||||
|
||||
await expect(
|
||||
handlers['worktrees:create'](null, {
|
||||
repoId: 'repo-ssh',
|
||||
name: 'contributor-fix',
|
||||
branchNameOverride: 'contributor/fix',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
}
|
||||
})
|
||||
).rejects.toThrow('network unreachable')
|
||||
|
||||
expect(exec).toHaveBeenCalledWith(['remote', 'remove', 'pr-contributor-orca'], '/remote/repo')
|
||||
expect(provider.addWorktree).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Regression: the rollback used to fire on ownership inherited from a sibling
|
||||
// worktree, deleting the remote that worktree was still pushing through.
|
||||
it('keeps a reused fork remote a sibling worktree owns when the SSH head fetch fails', async () => {
|
||||
const repo = {
|
||||
id: 'repo-ssh',
|
||||
path: '/remote/repo',
|
||||
displayName: 'ssh',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
connectionId: 'conn-1',
|
||||
worktreeBaseRef: 'origin/main'
|
||||
}
|
||||
const exec = vi.fn().mockImplementation(async (args: string[]) => {
|
||||
validateGitExecArgs(args)
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return {
|
||||
stdout:
|
||||
args[2] === 'pr-contributor-orca'
|
||||
? 'git@github.com:contributor/orca.git\n'
|
||||
: 'git@github.com:stablyai/orca.git\n',
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'remote' && args.length === 1) {
|
||||
return { stdout: 'origin\npr-contributor-orca\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'show-ref') {
|
||||
throw Object.assign(new Error('missing exact ref'), { code: 1 })
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
const provider = {
|
||||
exec,
|
||||
fetchRemoteTrackingRef: vi
|
||||
.fn()
|
||||
.mockImplementation(async (_repoPath: string, remote: string) => {
|
||||
if (remote === 'pr-contributor-orca') {
|
||||
throw new Error('network unreachable')
|
||||
}
|
||||
}),
|
||||
addWorktree: vi.fn().mockResolvedValue(undefined),
|
||||
listWorktrees: vi.fn().mockResolvedValue([])
|
||||
}
|
||||
const mux = { request: vi.fn().mockResolvedValue(undefined), notify: vi.fn() }
|
||||
store.getRepos.mockReturnValue([repo])
|
||||
store.getRepo.mockReturnValue(repo)
|
||||
store.getAllWorktreeMeta.mockReturnValue({
|
||||
'repo-ssh::/remote/repo-sibling': {
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/other',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
}
|
||||
}
|
||||
})
|
||||
getSshGitProviderMock.mockReturnValue(provider)
|
||||
getActiveMultiplexerMock.mockReturnValue(mux)
|
||||
|
||||
await expect(
|
||||
handlers['worktrees:create'](null, {
|
||||
repoId: 'repo-ssh',
|
||||
name: 'contributor-fix',
|
||||
branchNameOverride: 'contributor/fix',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
}
|
||||
})
|
||||
).rejects.toThrow('network unreachable')
|
||||
|
||||
expect(exec).not.toHaveBeenCalledWith(
|
||||
['remote', 'remove', 'pr-contributor-orca'],
|
||||
'/remote/repo'
|
||||
const result = await materializeWorktreePushTargetRemoteSsh(
|
||||
{ exec, fetchRemoteTrackingRef, markRemoteOrcaCreated } as unknown as SshGitProvider,
|
||||
'/remote/repo',
|
||||
target
|
||||
)
|
||||
expect(exec).not.toHaveBeenCalledWith(
|
||||
|
||||
expect(result).toEqual({ ...target, remoteCreated: true })
|
||||
expect(exec).toHaveBeenCalledWith(
|
||||
['remote', 'add', 'pr-contributor-orca', 'https://github.com/contributor/orca.git'],
|
||||
'/remote/repo'
|
||||
)
|
||||
expect(fetchRemoteTrackingRef).toHaveBeenCalledWith(
|
||||
'/remote/repo',
|
||||
'pr-contributor-orca',
|
||||
'contributor/fix',
|
||||
'refs/remotes/pr-contributor-orca/contributor/fix'
|
||||
)
|
||||
expect(markRemoteOrcaCreated).toHaveBeenCalledWith('/remote/repo', 'pr-contributor-orca')
|
||||
})
|
||||
|
||||
// The relay-upgrade-messaging, fetch-failure rollback, and sibling-remote-preserved
|
||||
// cases used to be exercised here because create minted the remote unconditionally.
|
||||
// That code (prepareWorktreePushTargetSsh) is unchanged -- it just no longer runs at
|
||||
// create time for a fork remote, only from materializeWorktreePushTargetRemoteSsh on
|
||||
// first sync. Coverage for all three moved with it to
|
||||
// worktree-remote-push-target-materialization.test.ts, which calls that function directly.
|
||||
})
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
gitExecFileAsyncMock
|
||||
} from './worktrees-test-module-mocks'
|
||||
import { handlers, harnessRepo, setupWorktreeHandlers, store } from './worktrees-test-harness'
|
||||
import { materializeWorktreePushTargetRemote } from './worktree-remote'
|
||||
import type { WorktreeRuntimeStub } from './worktrees-test-runtime-stub'
|
||||
import {
|
||||
createdWorktreeList,
|
||||
@@ -243,7 +244,11 @@ describe('registerWorktreeHandlers', () => {
|
||||
expectEveryGitCallRoutedTo('Ubuntu')
|
||||
})
|
||||
|
||||
it('routes fork push target setup through the selected WSL project runtime', async () => {
|
||||
// Was "routes fork push target setup ... through create": create used to mint the
|
||||
// fork remote (and route it to the selected WSL distro) unconditionally. It now
|
||||
// defers to first sync (#17828) -- split in two so each half stays true to a single
|
||||
// claim: create stays a no-op even for a WSL-routed repo, sync still routes to the distro.
|
||||
it('does not mint a fork remote at create time for a WSL-routed worktree', async () => {
|
||||
mockSelectedWslProjectRuntime()
|
||||
listWorktreesMock.mockResolvedValue([
|
||||
{
|
||||
@@ -266,6 +271,43 @@ describe('registerWorktreeHandlers', () => {
|
||||
}
|
||||
})
|
||||
|
||||
const calls = gitExecFileAsyncMock.mock.calls.map((call) => call[0] as string[])
|
||||
expect(calls).not.toContainEqual(['check-ref-format', '--branch', 'contributor/wsl-fork'])
|
||||
expect(calls.some((args) => args[0] === 'remote' && args[1] === 'add')).toBe(false)
|
||||
expect(calls.some((args) => args[0] === 'fetch')).toBe(false)
|
||||
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
expect.objectContaining({
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/wsl-fork',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
}
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
// Companion to the deferral test above: `materializeWorktreePushTargetRemote` is
|
||||
// exactly what `git:push`/`git:pull`'s local dispatch calls (with the repo's resolved
|
||||
// wslDistro) before syncing, so this is "first sync" without needing that IPC handler
|
||||
// registered in this harness.
|
||||
it('routes fork push target materialization through the selected WSL project runtime', async () => {
|
||||
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
|
||||
const target = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/wsl-fork',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
}
|
||||
|
||||
const result = await materializeWorktreePushTargetRemote(
|
||||
'/workspace/repo',
|
||||
target,
|
||||
undefined,
|
||||
undefined,
|
||||
{ wslDistro: 'Ubuntu' }
|
||||
)
|
||||
|
||||
expect(result).toEqual({ ...target, remoteCreated: true })
|
||||
const wslRoutingOptions = { cwd: '/workspace/repo', wslDistro: 'Ubuntu' }
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
['check-ref-format', '--branch', 'contributor/wsl-fork'],
|
||||
@@ -303,18 +345,29 @@ describe('registerWorktreeHandlers', () => {
|
||||
['config', 'remote.pr-contributor-orca.tagOpt', '--no-tags'],
|
||||
wslRoutingOptions
|
||||
)
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
['config', 'remote.pr-contributor-orca.orca-created', 'true'],
|
||||
wslRoutingOptions
|
||||
)
|
||||
// Why: the mint's fetch is the one call in this sequence that talks to the network --
|
||||
// bounded the same as the deferred short-circuit's fetch (see DEFERRED_PUSH_TARGET_FETCH_TIMEOUT_MS)
|
||||
// so a hung credential prompt can't wedge it forever. Every other call here is local-only
|
||||
// and stays untimed, per `wslRoutingOptions` above.
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
[
|
||||
'fetch',
|
||||
'pr-contributor-orca',
|
||||
'+refs/heads/contributor/wsl-fork*:refs/remotes/pr-contributor-orca/contributor/wsl-fork*'
|
||||
],
|
||||
wslRoutingOptions
|
||||
{ ...wslRoutingOptions, timeout: expect.any(Number) }
|
||||
)
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
['branch', '--set-upstream-to', 'pr-contributor-orca/contributor/wsl-fork', 'wsl-fork'],
|
||||
{ cwd: '/workspace/wsl-fork', wslDistro: 'Ubuntu' }
|
||||
// wslDistro threaded through every subprocess this materialize made, not just the adds.
|
||||
const distros = new Set(
|
||||
gitExecFileAsyncMock.mock.calls.map(
|
||||
([, options]) => (options as { wslDistro?: string } | undefined)?.wslDistro
|
||||
)
|
||||
)
|
||||
expect(distros).toEqual(new Set(['Ubuntu']))
|
||||
})
|
||||
|
||||
it('routes selected PR branch conflict lookup through the selected WSL project runtime', async () => {
|
||||
|
||||
@@ -54,6 +54,7 @@ describe('SshGitProvider public API parity', () => {
|
||||
'worktreeIsClean',
|
||||
'refreshLocalBaseRefForWorktreeCreate',
|
||||
'renameCurrentBranch',
|
||||
'markRemoteOrcaCreated',
|
||||
'forceDeletePreservedBranch',
|
||||
'exec',
|
||||
'clone',
|
||||
@@ -63,7 +64,7 @@ describe('SshGitProvider public API parity', () => {
|
||||
'getRemoteCommitUrl'
|
||||
] as const
|
||||
|
||||
expect(methods).toHaveLength(51)
|
||||
expect(methods).toHaveLength(52)
|
||||
for (const method of methods) {
|
||||
expect(provider[method], method).toBeTypeOf('function')
|
||||
}
|
||||
|
||||
@@ -395,4 +395,38 @@ describe('SshGitProvider', () => {
|
||||
provider.forceDeletePreservedBranch('/home/user/repo', 'you/fix-auth', 'abc123')
|
||||
).rejects.toBe(error)
|
||||
})
|
||||
|
||||
it('markRemoteOrcaCreated sends the narrow provenance-marker request', async () => {
|
||||
await provider.markRemoteOrcaCreated('/home/user/repo', 'pr-contributor-orca')
|
||||
expect(mux.request).toHaveBeenCalledWith('git.markRemoteOrcaCreated', {
|
||||
repoPath: '/home/user/repo',
|
||||
remoteName: 'pr-contributor-orca'
|
||||
})
|
||||
})
|
||||
|
||||
it('markRemoteOrcaCreated degrades to a one-time warning for an older relay', async () => {
|
||||
mux.request.mockRejectedValue(methodNotFound('git.markRemoteOrcaCreated'))
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
|
||||
try {
|
||||
await expect(
|
||||
provider.markRemoteOrcaCreated('/home/user/repo', 'pr-contributor-orca')
|
||||
).resolves.toBeUndefined()
|
||||
await expect(
|
||||
provider.markRemoteOrcaCreated('/home/user/repo', 'pr-contributor-orca')
|
||||
).resolves.toBeUndefined()
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1)
|
||||
} finally {
|
||||
warnSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('markRemoteOrcaCreated rethrows non-method-not-found errors', async () => {
|
||||
const error = new Error('remote config write failed')
|
||||
mux.request.mockRejectedValueOnce(error)
|
||||
|
||||
await expect(
|
||||
provider.markRemoteOrcaCreated('/home/user/repo', 'pr-contributor-orca')
|
||||
).rejects.toBe(error)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -24,6 +24,7 @@ function filterUntrackedPorcelainStatus(stdout: string | undefined): string | un
|
||||
|
||||
export class SshGitWorktreeProvider extends SshGitReviewHeadProvider {
|
||||
private loggedWorktreeIsCleanFallback = false
|
||||
private loggedMarkRemoteOrcaCreatedFallback = false
|
||||
// Why: reconnect replaces this provider, so an upgraded relay is naturally re-probed.
|
||||
private readonly worktreeIsCleanCapabilityCache = new CapabilityProbeCache<
|
||||
typeof WORKTREE_IS_CLEAN_CAPABILITY
|
||||
@@ -131,6 +132,25 @@ export class SshGitWorktreeProvider extends SshGitReviewHeadProvider {
|
||||
})
|
||||
}
|
||||
|
||||
// Why: git.exec blocks config writes outright, so the deferred fork-remote provenance
|
||||
// marker (#17828) needs its own RPC. Non-essential to push/pull, so an older relay
|
||||
// that hasn't shipped it yet degrades to no marker rather than failing materialization.
|
||||
async markRemoteOrcaCreated(repoPath: string, remoteName: string): Promise<void> {
|
||||
try {
|
||||
await this.mux.request('git.markRemoteOrcaCreated', { repoPath, remoteName })
|
||||
} catch (error) {
|
||||
if (!isJsonRpcMethodNotFoundError(error)) {
|
||||
throw error
|
||||
}
|
||||
if (!this.loggedMarkRemoteOrcaCreatedFallback) {
|
||||
this.loggedMarkRemoteOrcaCreatedFallback = true
|
||||
console.warn(
|
||||
"[ssh-git] Relay does not implement git.markRemoteOrcaCreated; this remote will lack a git-config provenance marker permanently (reconnecting does not retroactively add it -- only a newer relay deployment does, for remotes added after that). The store's remoteCreated flag remains the fallback ownership signal for cleanup."
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async forceDeletePreservedBranch(
|
||||
repoPath: string,
|
||||
branchName: string,
|
||||
|
||||
@@ -97,6 +97,16 @@ export class OrcaRuntimeWithFileCommands extends OrcaRuntimeWithPreservedBranchC
|
||||
linkedWorkItem: meta.linkedWorkItem
|
||||
}
|
||||
: null
|
||||
},
|
||||
// Why (#17828 review follow-up): RuntimeGitSyncCommands materializes with no store to
|
||||
// avoid unrelated side effects; this is its only way back into the persisted
|
||||
// `pushTarget.remoteCreated` flag that #17842's orphan sweep relies on.
|
||||
persistMaterializedPushTarget: (worktreeId, pushTarget) => {
|
||||
const store = this.store
|
||||
if (!store?.setWorktreeMeta) {
|
||||
return
|
||||
}
|
||||
store.setWorktreeMeta(worktreeId, { pushTarget })
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -23,6 +23,7 @@ import { homedir } from 'node:os'
|
||||
import { getExplicitWorktreeIdSelector } from './runtime-worktree-selection'
|
||||
import { WORKTREE_ID_SEPARATOR } from '../../shared/worktree/id'
|
||||
import { WorktreeIdRequiresFullPathError } from './runtime-worktree-lineage-resolution'
|
||||
import { triggerTerminalSpawnPushTargetMaterialization } from './runtime-terminal-spawn-push-target-materialization'
|
||||
|
||||
export class OrcaRuntimeWithResolveBrowserNetworkExecutionHostForWorktree extends OrcaRuntimeWithTransitionGraphReloadToTerminalState {
|
||||
protected resolveBrowserNetworkExecutionHostForWorktree(worktree?: {
|
||||
@@ -124,6 +125,14 @@ export class OrcaRuntimeWithResolveBrowserNetworkExecutionHostForWorktree extend
|
||||
const worktreeSelector = parsed?.type === 'worktree' ? `id:${parsed.worktreeId}` : selector
|
||||
const worktree = await this.resolveWorktreeSelector(worktreeSelector)
|
||||
const repo = this.store?.getRepo(worktree.repoId) ?? null
|
||||
triggerTerminalSpawnPushTargetMaterialization(
|
||||
worktree.path,
|
||||
worktree.pushTarget,
|
||||
repo,
|
||||
this.store,
|
||||
worktree.repoId,
|
||||
worktree.id
|
||||
)
|
||||
return {
|
||||
scope: {
|
||||
id: worktree.id,
|
||||
|
||||
@@ -404,27 +404,36 @@ describe('OrcaRuntimeService', () => {
|
||||
wslDistro: 'Ubuntu'
|
||||
}
|
||||
)
|
||||
expect(gitSpy).toHaveBeenCalledWith(
|
||||
// Why: a fork remote is deferred to first push/pull/fetch/fast-forward
|
||||
// (#17828) instead of being added/fetched at create time, so the create
|
||||
// path must not run check-ref-format, the fork fetch, or set-upstream-to
|
||||
// -- the metadata is persisted untouched for on-demand materialization.
|
||||
expect(gitSpy).not.toHaveBeenCalledWith(
|
||||
['check-ref-format', '--branch', 'contributor/runtime-wsl'],
|
||||
{ cwd: TEST_REPO_PATH, wslDistro: 'Ubuntu' }
|
||||
expect.anything()
|
||||
)
|
||||
expect(gitSpy).toHaveBeenCalledWith(
|
||||
expect(gitSpy).not.toHaveBeenCalledWith(
|
||||
[
|
||||
'fetch',
|
||||
'pr-contributor-orca',
|
||||
'+refs/heads/contributor/runtime-wsl*:refs/remotes/pr-contributor-orca/contributor/runtime-wsl*'
|
||||
],
|
||||
{ cwd: TEST_REPO_PATH, wslDistro: 'Ubuntu' }
|
||||
expect.anything()
|
||||
)
|
||||
expect(gitSpy).toHaveBeenCalledWith(
|
||||
expect(gitSpy).not.toHaveBeenCalledWith(
|
||||
[
|
||||
'branch',
|
||||
'--set-upstream-to',
|
||||
'pr-contributor-orca/contributor/runtime-wsl',
|
||||
'runtime-wsl'
|
||||
],
|
||||
{ cwd: createdWorktree.path, wslDistro: 'Ubuntu' }
|
||||
expect.anything()
|
||||
)
|
||||
expect(result.worktree.pushTarget).toEqual({
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/runtime-wsl',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
})
|
||||
expect(listWorktrees).toHaveBeenCalledWith(TEST_REPO_PATH, { wslDistro: 'Ubuntu' })
|
||||
} finally {
|
||||
gitSpy.mockRestore()
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { GlobalSettings } from '../../shared/global-settings-types'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import type { GitWorktreeInfo, Worktree } from '../../shared/worktree/types'
|
||||
import type { GitPushTarget, GitWorktreeInfo, Worktree } from '../../shared/worktree/types'
|
||||
import type { GitRuntimeOptions } from '../git/git-runtime-options'
|
||||
import type { CommitMessageAgentEnvironmentResolvers } from '../text-generation/commit-message-agent-environment'
|
||||
import type { PullRequestLinkedIssueMeta } from '../source-control/pull-request-linked-issue'
|
||||
@@ -22,6 +22,11 @@ export type RuntimeGitCommandHost = {
|
||||
/** `undefined` keeps cached metadata; `null` is the authoritative unlinked answer. */
|
||||
getWorktreeLinkedIssue?(worktreeId: string): number | null | undefined
|
||||
getWorktreeLinkedIssueMeta?(worktreeId: string): PullRequestLinkedIssueMeta | null | undefined
|
||||
/** Why (#17828 review follow-up): RuntimeGitSyncCommands deliberately materializes with
|
||||
* no store (avoids unrelated ownership-inheritance/refspec-migration side effects), so a
|
||||
* lazily-minted remote still needs a way back into the store's `pushTarget.remoteCreated`
|
||||
* for #17842's orphan sweep. Called only when materialize reports `remoteCreated: true`. */
|
||||
persistMaterializedPushTarget?(worktreeId: string, pushTarget: GitPushTarget): void
|
||||
}
|
||||
|
||||
export function localGitOptionsForTarget(target: RuntimeGitTarget): GitRuntimeOptions {
|
||||
|
||||
@@ -9,11 +9,32 @@ import {
|
||||
getSshGitProvider,
|
||||
SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE
|
||||
} from '../providers/ssh-git-dispatch'
|
||||
import { localGitOptionsForTarget, type RuntimeGitCommandHost } from './runtime-git-command-target'
|
||||
import {
|
||||
materializeWorktreePushTargetRemote,
|
||||
materializeWorktreePushTargetRemoteSsh
|
||||
} from '../ipc/worktree-remote'
|
||||
import {
|
||||
localGitOptionsForTarget,
|
||||
type RuntimeGitCommandHost,
|
||||
type RuntimeGitTarget
|
||||
} from './runtime-git-command-target'
|
||||
|
||||
export class RuntimeGitSyncCommands {
|
||||
constructor(private readonly host: RuntimeGitCommandHost) {}
|
||||
|
||||
// Why (#17828 review follow-up): this class deliberately materializes with no store (see
|
||||
// the `undefined` args below) to avoid unrelated ownership-inheritance/refspec-migration
|
||||
// side effects on the RPC path -- so persistence goes through the host callback instead,
|
||||
// using `target.worktree.id` already resolved here rather than threading a store through.
|
||||
private persistMaterializedPushTargetIfCreated(
|
||||
target: RuntimeGitTarget,
|
||||
materialized: GitPushTarget | undefined
|
||||
): void {
|
||||
if (materialized?.remoteCreated) {
|
||||
this.host.persistMaterializedPushTarget?.(target.worktree.id, materialized)
|
||||
}
|
||||
}
|
||||
|
||||
async abortRuntimeGitMerge(worktreeSelector: string): Promise<{ ok: true }> {
|
||||
const target = await this.host.resolveRuntimeGitTarget(worktreeSelector)
|
||||
const provider = target.connectionId ? getSshGitProvider(target.connectionId) : null
|
||||
@@ -73,10 +94,24 @@ export class RuntimeGitSyncCommands {
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
await provider.fetchRemote(target.worktree.path, pushTarget)
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(provider, target.worktree.path, pushTarget)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await provider.fetchRemote(target.worktree.path, materializedPushTarget)
|
||||
return { ok: true }
|
||||
}
|
||||
await gitFetch(target.worktree.path, pushTarget, {
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
target.worktree.path,
|
||||
pushTarget,
|
||||
undefined,
|
||||
target.repo?.id,
|
||||
localGitOptionsForTarget(target)
|
||||
)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await gitFetch(target.worktree.path, materializedPushTarget, {
|
||||
...localGitOptionsForTarget(target),
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
@@ -111,10 +146,24 @@ export class RuntimeGitSyncCommands {
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
await provider.pullBranch(target.worktree.path, pushTarget)
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(provider, target.worktree.path, pushTarget)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await provider.pullBranch(target.worktree.path, materializedPushTarget)
|
||||
return { ok: true }
|
||||
}
|
||||
await gitPull(target.worktree.path, pushTarget, {
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
target.worktree.path,
|
||||
pushTarget,
|
||||
undefined,
|
||||
target.repo?.id,
|
||||
localGitOptionsForTarget(target)
|
||||
)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await gitPull(target.worktree.path, materializedPushTarget, {
|
||||
...localGitOptionsForTarget(target),
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
@@ -131,10 +180,24 @@ export class RuntimeGitSyncCommands {
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
await provider.fastForwardBranch(target.worktree.path, pushTarget)
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(provider, target.worktree.path, pushTarget)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await provider.fastForwardBranch(target.worktree.path, materializedPushTarget)
|
||||
return { ok: true }
|
||||
}
|
||||
await gitFastForward(target.worktree.path, pushTarget, {
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
target.worktree.path,
|
||||
pushTarget,
|
||||
undefined,
|
||||
target.repo?.id,
|
||||
localGitOptionsForTarget(target)
|
||||
)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await gitFastForward(target.worktree.path, materializedPushTarget, {
|
||||
...localGitOptionsForTarget(target),
|
||||
admissionTier: 'interactive'
|
||||
})
|
||||
@@ -170,12 +233,26 @@ export class RuntimeGitSyncCommands {
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
await provider.pushBranch(target.worktree.path, publish === true, pushTarget, {
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemoteSsh(provider, target.worktree.path, pushTarget)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await provider.pushBranch(target.worktree.path, publish === true, materializedPushTarget, {
|
||||
forceWithLease: forceWithLease === true
|
||||
})
|
||||
return { ok: true }
|
||||
}
|
||||
await gitPush(target.worktree.path, publish === true, pushTarget, {
|
||||
const materializedPushTarget = pushTarget
|
||||
? await materializeWorktreePushTargetRemote(
|
||||
target.worktree.path,
|
||||
pushTarget,
|
||||
undefined,
|
||||
target.repo?.id,
|
||||
localGitOptionsForTarget(target)
|
||||
)
|
||||
: undefined
|
||||
this.persistMaterializedPushTargetIfCreated(target, materializedPushTarget)
|
||||
await gitPush(target.worktree.path, publish === true, materializedPushTarget, {
|
||||
forceWithLease: forceWithLease === true,
|
||||
...localGitOptionsForTarget(target),
|
||||
admissionTier: 'interactive'
|
||||
|
||||
@@ -2,10 +2,7 @@ import type { GitPushTarget, GitWorktreeInfo } from '../../shared/worktree/types
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import { resolveCreatedWorktree } from '../ipc/created-worktree-reconciliation'
|
||||
import { normalizeSparseDirectories } from '../ipc/sparse-checkout-directories'
|
||||
import {
|
||||
configureCreatedWorktreePushTarget,
|
||||
prepareWorktreePushTarget
|
||||
} from '../ipc/worktree-remote'
|
||||
import { configureCreatedWorktreePushTarget } from '../ipc/worktree-remote'
|
||||
import {
|
||||
addSparseWorktree,
|
||||
addWorktree,
|
||||
@@ -129,15 +126,11 @@ export async function createRuntimeLocalGitWorktree(args: {
|
||||
if (args.request.sparseCheckout && sparseDirectories.length === 0) {
|
||||
throw new Error('Sparse checkout requires at least one repo-relative directory.')
|
||||
}
|
||||
// Why: defer the remote add + fetch (fork case) or the redundant re-fetch
|
||||
// (same-repo case, already fetched while resolving the PR start point) to
|
||||
// first use -- push/pull/fetch/fast-forward materialize it on demand
|
||||
// (#17828). Metadata is persisted untouched; only the git mutation defers.
|
||||
const preparedPushTarget = args.request.pushTarget
|
||||
? await prepareWorktreePushTarget(
|
||||
args.repo.path,
|
||||
args.request.pushTarget,
|
||||
args.store,
|
||||
args.repo.id,
|
||||
args.localWorktreeGitOptions
|
||||
)
|
||||
: undefined
|
||||
const suggestLocalBaseRefUpdate =
|
||||
!args.settings.refreshLocalBaseRefOnWorktreeCreate &&
|
||||
!args.settings.localBaseRefSuggestionDismissed &&
|
||||
@@ -242,14 +235,18 @@ export async function createRuntimeLocalGitWorktree(args: {
|
||||
args.effectiveSanitizedName!
|
||||
)
|
||||
}
|
||||
const configuredPushTarget = preparedPushTarget
|
||||
? await configureCreatedWorktreePushTarget(
|
||||
args.worktreePath,
|
||||
args.branchName,
|
||||
preparedPushTarget,
|
||||
args.localWorktreeGitOptions
|
||||
)
|
||||
: undefined
|
||||
// Why: `--set-upstream-to` requires the remote to already exist -- safe for a
|
||||
// same-repo target (its remote, e.g. `origin`, always exists) but not for a
|
||||
// deferred fork remote, which is materialized lazily at first push/pull/fetch.
|
||||
const configuredPushTarget =
|
||||
preparedPushTarget && !preparedPushTarget.remoteUrl
|
||||
? await configureCreatedWorktreePushTarget(
|
||||
args.worktreePath,
|
||||
args.branchName,
|
||||
preparedPushTarget,
|
||||
args.localWorktreeGitOptions
|
||||
)
|
||||
: preparedPushTarget
|
||||
const { created } = await resolveCreatedWorktree(
|
||||
args.repo.path,
|
||||
args.worktreePath,
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitPushTarget } from '../../shared/worktree/types'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import type { Store } from '../persistence'
|
||||
|
||||
const {
|
||||
materializeLocalMock,
|
||||
materializeSshMock,
|
||||
getSshGitProviderMock,
|
||||
getLocalProjectWorktreeGitOptionsMock
|
||||
} = vi.hoisted(() => ({
|
||||
materializeLocalMock: vi.fn(),
|
||||
materializeSshMock: vi.fn(),
|
||||
getSshGitProviderMock: vi.fn(),
|
||||
getLocalProjectWorktreeGitOptionsMock: vi.fn()
|
||||
}))
|
||||
vi.mock('../ipc/worktree-remote', () => ({
|
||||
materializeWorktreePushTargetRemote: materializeLocalMock,
|
||||
materializeWorktreePushTargetRemoteSsh: materializeSshMock
|
||||
}))
|
||||
vi.mock('../providers/ssh-git-dispatch', () => ({
|
||||
getSshGitProvider: getSshGitProviderMock
|
||||
}))
|
||||
vi.mock('../project-runtime-git-options', () => ({
|
||||
getLocalProjectWorktreeGitOptions: getLocalProjectWorktreeGitOptionsMock
|
||||
}))
|
||||
|
||||
import { triggerTerminalSpawnPushTargetMaterialization } from './runtime-terminal-spawn-push-target-materialization'
|
||||
|
||||
const WORKTREE_PATH = '/repo/worktree'
|
||||
const FORK_URL = 'git@github.com:contributor/orca.git'
|
||||
const REPO_ID = 'repo-1'
|
||||
const STORE = {} as Store
|
||||
const LOCAL_REPO = { id: REPO_ID, path: '/repo', connectionId: null } as unknown as Repo
|
||||
const SSH_REPO = { id: REPO_ID, path: '/repo', connectionId: 'conn-1' } as unknown as Repo
|
||||
|
||||
function forkTarget(overrides: Partial<GitPushTarget> = {}): GitPushTarget {
|
||||
return {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: FORK_URL,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
// Flush the fire-and-forget microtask queue so assertions see the dispatched call.
|
||||
const flush = (): Promise<void> => new Promise((resolve) => setImmediate(resolve))
|
||||
|
||||
describe('triggerTerminalSpawnPushTargetMaterialization', () => {
|
||||
let warnSpy: ReturnType<typeof vi.spyOn>
|
||||
|
||||
beforeEach(() => {
|
||||
materializeLocalMock.mockReset().mockResolvedValue(undefined)
|
||||
materializeSshMock.mockReset().mockResolvedValue(undefined)
|
||||
getSshGitProviderMock.mockReset()
|
||||
getLocalProjectWorktreeGitOptionsMock.mockReset().mockReturnValue({})
|
||||
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
warnSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('is a no-op when there is no push target', () => {
|
||||
triggerTerminalSpawnPushTargetMaterialization(WORKTREE_PATH, undefined, LOCAL_REPO, STORE)
|
||||
expect(materializeLocalMock).not.toHaveBeenCalled()
|
||||
expect(materializeSshMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('is a no-op for a same-repo push target with no remoteUrl', () => {
|
||||
triggerTerminalSpawnPushTargetMaterialization(
|
||||
WORKTREE_PATH,
|
||||
forkTarget({ remoteUrl: undefined }),
|
||||
LOCAL_REPO,
|
||||
STORE
|
||||
)
|
||||
expect(materializeLocalMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('is a no-op when the target already reports remoteCreated', () => {
|
||||
triggerTerminalSpawnPushTargetMaterialization(
|
||||
WORKTREE_PATH,
|
||||
forkTarget({ remoteCreated: true }),
|
||||
LOCAL_REPO,
|
||||
STORE
|
||||
)
|
||||
expect(materializeLocalMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('materializes over the local transport with resolved WSL git options, repoId and worktreeId, fire-and-forget', () => {
|
||||
getLocalProjectWorktreeGitOptionsMock.mockReturnValue({ wslDistro: 'Ubuntu' })
|
||||
const target = forkTarget()
|
||||
const result = triggerTerminalSpawnPushTargetMaterialization(
|
||||
WORKTREE_PATH,
|
||||
target,
|
||||
LOCAL_REPO,
|
||||
STORE,
|
||||
REPO_ID,
|
||||
'worktree-1'
|
||||
)
|
||||
expect(result).toBeUndefined()
|
||||
expect(getLocalProjectWorktreeGitOptionsMock).toHaveBeenCalledWith(STORE, LOCAL_REPO)
|
||||
expect(materializeLocalMock).toHaveBeenCalledWith(
|
||||
WORKTREE_PATH,
|
||||
target,
|
||||
STORE,
|
||||
REPO_ID,
|
||||
{ wslDistro: 'Ubuntu' },
|
||||
'worktree-1'
|
||||
)
|
||||
expect(materializeSshMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('materializes over SSH when the repo has a connectionId and a provider is registered', () => {
|
||||
const provider = { exec: vi.fn() }
|
||||
getSshGitProviderMock.mockReturnValue(provider)
|
||||
const target = forkTarget()
|
||||
triggerTerminalSpawnPushTargetMaterialization(
|
||||
WORKTREE_PATH,
|
||||
target,
|
||||
SSH_REPO,
|
||||
STORE,
|
||||
REPO_ID,
|
||||
'worktree-1'
|
||||
)
|
||||
expect(getSshGitProviderMock).toHaveBeenCalledWith('conn-1')
|
||||
expect(materializeSshMock).toHaveBeenCalledWith(
|
||||
provider,
|
||||
WORKTREE_PATH,
|
||||
target,
|
||||
STORE,
|
||||
undefined,
|
||||
'worktree-1'
|
||||
)
|
||||
expect(materializeLocalMock).not.toHaveBeenCalled()
|
||||
expect(getLocalProjectWorktreeGitOptionsMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('is a no-op when the SSH connection has dropped (no registered provider)', () => {
|
||||
getSshGitProviderMock.mockReturnValue(undefined)
|
||||
triggerTerminalSpawnPushTargetMaterialization(WORKTREE_PATH, forkTarget(), SSH_REPO, STORE)
|
||||
expect(materializeSshMock).not.toHaveBeenCalled()
|
||||
expect(materializeLocalMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('falls back to default git options when WSL project runtime resolution throws', () => {
|
||||
getLocalProjectWorktreeGitOptionsMock.mockImplementation(() => {
|
||||
throw new Error('repair-required')
|
||||
})
|
||||
triggerTerminalSpawnPushTargetMaterialization(WORKTREE_PATH, forkTarget(), LOCAL_REPO, STORE)
|
||||
expect(materializeLocalMock).toHaveBeenCalledWith(
|
||||
WORKTREE_PATH,
|
||||
forkTarget(),
|
||||
STORE,
|
||||
undefined,
|
||||
{},
|
||||
undefined
|
||||
)
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('failed to resolve local git options'),
|
||||
expect.any(Error)
|
||||
)
|
||||
})
|
||||
|
||||
it('swallows a materialize rejection instead of crashing the caller', async () => {
|
||||
materializeLocalMock.mockRejectedValue(new Error('remote add failed'))
|
||||
expect(() =>
|
||||
triggerTerminalSpawnPushTargetMaterialization(WORKTREE_PATH, forkTarget(), LOCAL_REPO, STORE)
|
||||
).not.toThrow()
|
||||
await flush()
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('failed to materialize push target remote'),
|
||||
expect.any(Error)
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,84 @@
|
||||
import { getSshGitProvider } from '../providers/ssh-git-dispatch'
|
||||
import {
|
||||
materializeWorktreePushTargetRemote,
|
||||
materializeWorktreePushTargetRemoteSsh
|
||||
} from '../ipc/worktree-remote'
|
||||
import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options'
|
||||
import type { GitPushTarget } from '../../shared/worktree/types'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import type { Store } from '../persistence'
|
||||
|
||||
// Why (#17828): a fork-PR remote deferred at worktree-create time must exist before an
|
||||
// autonomous agent's raw git commands run in a freshly opened terminal -- "sync through
|
||||
// Orca first" isn't an option mid-task. Fires on every terminal spawn into the worktree;
|
||||
// materialize() is already a no-op once the remote exists, so repeat spawns cost one probe.
|
||||
// Never awaited by callers: terminal spawn must not block on remote-add/fetch network I/O.
|
||||
export function triggerTerminalSpawnPushTargetMaterialization(
|
||||
worktreePath: string,
|
||||
pushTarget: GitPushTarget | undefined,
|
||||
repo: Repo | null | undefined,
|
||||
store: Store | undefined,
|
||||
repoId?: string,
|
||||
worktreeId?: string
|
||||
): void {
|
||||
if (!pushTarget?.remoteUrl || pushTarget.remoteCreated) {
|
||||
return
|
||||
}
|
||||
const connectionId = repo?.connectionId ?? undefined
|
||||
const materialized = connectionId
|
||||
? materializeOverSsh(connectionId, worktreePath, pushTarget, store, worktreeId)
|
||||
: materializeWorktreePushTargetRemote(
|
||||
worktreePath,
|
||||
pushTarget,
|
||||
store,
|
||||
repoId,
|
||||
localGitOptionsForTerminalSpawn(store, repo),
|
||||
worktreeId
|
||||
)
|
||||
materialized.catch((error: unknown) => {
|
||||
console.warn(
|
||||
`[terminal-spawn] failed to materialize push target remote for ${worktreePath}:`,
|
||||
error
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
function materializeOverSsh(
|
||||
connectionId: string,
|
||||
worktreePath: string,
|
||||
pushTarget: GitPushTarget,
|
||||
store: Store | undefined,
|
||||
worktreeId: string | undefined
|
||||
): Promise<GitPushTarget> {
|
||||
const provider = getSshGitProvider(connectionId)
|
||||
if (!provider) {
|
||||
// Why: connection dropped -- the next Orca-driven sync action will retry via its own dispatch.
|
||||
return Promise.resolve(pushTarget)
|
||||
}
|
||||
return materializeWorktreePushTargetRemoteSsh(
|
||||
provider,
|
||||
worktreePath,
|
||||
pushTarget,
|
||||
store,
|
||||
undefined,
|
||||
worktreeId
|
||||
)
|
||||
}
|
||||
|
||||
function localGitOptionsForTerminalSpawn(
|
||||
store: Store | undefined,
|
||||
repo: Repo | null | undefined
|
||||
): { wslDistro?: string } {
|
||||
if (!store || !repo) {
|
||||
return {}
|
||||
}
|
||||
try {
|
||||
// Why: a WSL-hosted repo's remote add/fetch must run under the same distro as
|
||||
// the terminal, or it can target the wrong git binary entirely (repair-required
|
||||
// project runtimes throw here -- fall back to host git rather than crash spawn).
|
||||
return getLocalProjectWorktreeGitOptions(store, repo)
|
||||
} catch (error) {
|
||||
console.warn(`[terminal-spawn] failed to resolve local git options for ${repo.path}:`, error)
|
||||
return {}
|
||||
}
|
||||
}
|
||||
@@ -67,6 +67,7 @@ export const gitApi = {
|
||||
}): Promise<GitUpstreamStatus> => ipcRenderer.invoke('git:upstreamStatus', args),
|
||||
fetch: (args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}): Promise<void> => ipcRenderer.invoke('git:fetch', args),
|
||||
@@ -77,6 +78,7 @@ export const gitApi = {
|
||||
}): Promise<GitForkSyncResult> => ipcRenderer.invoke('git:syncFork', args),
|
||||
push: (args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
publish?: boolean
|
||||
forceWithLease?: boolean
|
||||
connectionId?: string
|
||||
@@ -84,11 +86,13 @@ export const gitApi = {
|
||||
}): Promise<void> => ipcRenderer.invoke('git:push', args),
|
||||
pull: (args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}): Promise<void> => ipcRenderer.invoke('git:pull', args),
|
||||
fastForward: (args: {
|
||||
worktreePath: string
|
||||
worktreeId?: string
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}): Promise<void> => ipcRenderer.invoke('git:fastForward', args),
|
||||
|
||||
@@ -34,6 +34,21 @@ export class GitHandlerExecOperations extends GitHandlerOperationContext {
|
||||
)
|
||||
}
|
||||
|
||||
// Why: generic git.exec blocks all `git config` writes outright (CONFIG_READ_ONLY_FLAGS),
|
||||
// so a deferred fork remote's provenance marker (#17828) needs its own narrow RPC that
|
||||
// only ever writes this fixed key shape, mirroring renameCurrentBranch below.
|
||||
async markRemoteOrcaCreated(params: Record<string, unknown>) {
|
||||
const repoPath = params.repoPath
|
||||
const remoteName = params.remoteName
|
||||
if (typeof repoPath !== 'string' || typeof remoteName !== 'string' || !remoteName) {
|
||||
throw new Error('Invalid remote provenance marker request.')
|
||||
}
|
||||
if (!/^[A-Za-z0-9._-]+$/.test(remoteName)) {
|
||||
throw new Error('Invalid remote name for provenance marker.')
|
||||
}
|
||||
await this.git(['config', `remote.${remoteName}.orca-created`, 'true'], repoPath)
|
||||
}
|
||||
|
||||
async renameCurrentBranch(params: Record<string, unknown>) {
|
||||
return this.runWithGitReadCacheClear(async () => {
|
||||
const worktreePath = params.worktreePath
|
||||
|
||||
@@ -65,6 +65,7 @@ export function registerGitHandlers(
|
||||
dispatcher.onRequest('git.refreshLocalBaseRefForWorktreeCreate', (p) =>
|
||||
handlers.worktree.refreshLocalBaseRefForWorktreeCreate(p)
|
||||
)
|
||||
dispatcher.onRequest('git.markRemoteOrcaCreated', (p) => handlers.exec.markRemoteOrcaCreated(p))
|
||||
dispatcher.onRequest('git.renameCurrentBranch', (p) => handlers.exec.renameCurrentBranch(p))
|
||||
dispatcher.onRequest('git.forceDeletePreservedBranch', (p) =>
|
||||
handlers.exec.forceDeletePreservedBranch(p)
|
||||
|
||||
@@ -73,6 +73,7 @@ describe('GitHandler', () => {
|
||||
expect(methods).toContain('git.removeWorktree')
|
||||
expect(methods).toContain('git.worktreeIsClean')
|
||||
expect(methods).toContain('git.refreshLocalBaseRefForWorktreeCreate')
|
||||
expect(methods).toContain('git.markRemoteOrcaCreated')
|
||||
expect(methods).toContain('git.renameCurrentBranch')
|
||||
expect(methods).toContain('git.forceDeletePreservedBranch')
|
||||
expect(methods).toContain('git.exec')
|
||||
@@ -197,6 +198,37 @@ describe('GitHandler', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('markRemoteOrcaCreated', () => {
|
||||
it('writes the provenance marker via config, not the generic git.exec path', async () => {
|
||||
gitInit(tmpDir)
|
||||
execFileSync('git', ['remote', 'add', 'pr-contributor-orca', 'https://example.com/x.git'], {
|
||||
cwd: tmpDir
|
||||
})
|
||||
|
||||
await dispatcher.callRequest('git.markRemoteOrcaCreated', {
|
||||
repoPath: tmpDir,
|
||||
remoteName: 'pr-contributor-orca'
|
||||
})
|
||||
|
||||
const value = execFileSync(
|
||||
'git',
|
||||
['config', '--get', 'remote.pr-contributor-orca.orca-created'],
|
||||
{ cwd: tmpDir, encoding: 'utf-8' }
|
||||
).trim()
|
||||
expect(value).toBe('true')
|
||||
})
|
||||
|
||||
it('rejects a remote name that is not a plain config-key segment', async () => {
|
||||
gitInit(tmpDir)
|
||||
await expect(
|
||||
dispatcher.callRequest('git.markRemoteOrcaCreated', {
|
||||
repoPath: tmpDir,
|
||||
remoteName: 'bad name; rm -rf'
|
||||
})
|
||||
).rejects.toThrow('Invalid remote name for provenance marker.')
|
||||
})
|
||||
})
|
||||
|
||||
describe('renameCurrentBranch', () => {
|
||||
it('renames only the checked-out branch through the narrow RPC', async () => {
|
||||
gitInit(tmpDir)
|
||||
|
||||
@@ -72,6 +72,7 @@ export async function fetchRuntimeGit(
|
||||
await window.api.git.fetch({
|
||||
worktreePath: resolveLocalWorktreePath(context),
|
||||
connectionId: context.connectionId,
|
||||
...(context.worktreeId ? { worktreeId: context.worktreeId } : {}),
|
||||
...(pushTarget ? { pushTarget } : {})
|
||||
})
|
||||
return
|
||||
@@ -116,6 +117,7 @@ export async function pullRuntimeGit(
|
||||
await window.api.git.pull({
|
||||
worktreePath: resolveLocalWorktreePath(context),
|
||||
connectionId: context.connectionId,
|
||||
...(context.worktreeId ? { worktreeId: context.worktreeId } : {}),
|
||||
...(pushTarget ? { pushTarget } : {})
|
||||
})
|
||||
return
|
||||
@@ -140,6 +142,7 @@ export async function fastForwardRuntimeGit(
|
||||
await window.api.git.fastForward({
|
||||
worktreePath: resolveLocalWorktreePath(context),
|
||||
connectionId: context.connectionId,
|
||||
...(context.worktreeId ? { worktreeId: context.worktreeId } : {}),
|
||||
...(pushTarget ? { pushTarget } : {})
|
||||
})
|
||||
return
|
||||
@@ -185,6 +188,7 @@ export async function pushRuntimeGit(
|
||||
await window.api.git.push({
|
||||
worktreePath: resolveLocalWorktreePath(context),
|
||||
connectionId: context.connectionId,
|
||||
...(context.worktreeId ? { worktreeId: context.worktreeId } : {}),
|
||||
...(args.publish !== undefined ? { publish: args.publish } : {}),
|
||||
...(args.pushTarget !== undefined ? { pushTarget: args.pushTarget } : {}),
|
||||
...(args.forceWithLease !== undefined ? { forceWithLease: args.forceWithLease } : {})
|
||||
|
||||
@@ -138,7 +138,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
|
||||
expect(gitPullMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(toastErrorMock).not.toHaveBeenCalled()
|
||||
})
|
||||
@@ -155,6 +156,7 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
worktreePath: '/repo',
|
||||
publish: false,
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1',
|
||||
pushTarget: undefined,
|
||||
forceWithLease: undefined
|
||||
})
|
||||
@@ -193,6 +195,7 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitFastForwardMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1',
|
||||
pushTarget
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
@@ -284,6 +287,7 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1',
|
||||
pushTarget
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
@@ -337,7 +341,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitPushMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
publish: true,
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(store.getState().isRemoteOperationActive).toBe(false)
|
||||
})
|
||||
@@ -361,7 +366,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitStatusMock).not.toHaveBeenCalled()
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
@@ -389,7 +395,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitStatusMock).not.toHaveBeenCalled()
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
@@ -450,7 +457,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitStatusMock).not.toHaveBeenCalled()
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
@@ -476,7 +484,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitStatusMock).not.toHaveBeenCalled()
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
@@ -507,7 +516,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitStatusMock).not.toHaveBeenCalled()
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
@@ -534,7 +544,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
@@ -590,7 +601,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
@@ -634,7 +646,8 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(store.getState().isRemoteOperationActive).toBe(false)
|
||||
expect(toastErrorMock).not.toHaveBeenCalled()
|
||||
@@ -728,16 +741,19 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
|
||||
expect(gitFetchMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(gitPullMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
// ahead=1 in the default mock, so sync pushes.
|
||||
expect(gitPushMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1'
|
||||
})
|
||||
expect(toastErrorMock).not.toHaveBeenCalled()
|
||||
expect(store.getState().isRemoteOperationActive).toBe(false)
|
||||
@@ -786,6 +802,7 @@ describe('createEditorSlice remote branch actions', () => {
|
||||
expect(gitPushMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined,
|
||||
worktreeId: 'wt-1',
|
||||
forceWithLease: true
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledTimes(2)
|
||||
|
||||
Reference in New Issue
Block a user