Files
orca/src/main/github/conflict-summary.test.ts
T
Brennan BensonandClaude 0fea878680 fix(git): keep merge-tree's throwaway trees out of the real object store (#22356)
* fix(git): keep merge-tree's throwaway trees out of the real object store

The PR conflict summary and the squash-merged branch cleanup both run
`git merge-tree --write-tree` only to read its output, but every divergent
run left unreachable loose objects behind. Run it with a per-call scratch
object directory (inside objects/, alternates pointing at the real store)
that is deleted afterwards, on native, WSL and SSH relay hosts alike.

* fix(git): keep partial-clone downloads and clean up stranded scratch stores

- A partial clone fetches missing blobs during merge-tree; those packs landed in the
  scratch store and were deleted, so every check re-downloaded them. Move fetched packs
  (index last) into the real store before removing the scratch dir.
- Sweep this app's own scratch dirs older than an hour, once per objects dir per process,
  so a crash or failed delete cannot strand one forever in repos where gc never runs.
- Remove with Windows transient-lock retries; the shared core now owns all filesystem
  work and host adapters only resolve the objects dir.

* fix(git): quote Windows alternates paths containing ';' and log scratch cleanup failures

* fix(git): sweep a merge-tree scratch dir only when its owner is provably gone

Another Orca process on the same repo (a second instance, dev and release
builds) could delete a scratch object dir still in use once its mtime passed
an hour. Each scratch dir now records its owner (pid, hostname, platform,
Linux pid namespace); the sweep removes it only when that owner shares this
pid space and has exited, or, with no checkable owner, at Git's own two-week
tmp_ expiry.

Also:
- A fetched pack whose later rename fails is rolled back, so the real store
  never keeps an index-less pack; a pack already installed is left alone.
- WSL Git gets the /mnt/<drive> spelling of a drive-path common dir (Git
  < 2.31 prints it relative, resolved against the C:\ repo path).
- conflict-summary tests no longer create scratch dirs in the cwd.

* fix(git): cap every merge-tree scratch dir at two weeks, live-looking owner included

A reused pid held by a long-lived process could keep a leftover scratch dir
forever in a repo where git gc never runs. The two-week age (git gc's own
tmp_ expiry) now removes any scratch dir; a younger one is still swept only
when its owner shares this pid space and has exited.

* fix(git): sweep merge-tree scratch dirs by age alone, off the check's path

- The stale sweep now removes any scratch dir at least two weeks old, Git's
  own expiry for its objects/tmp_* dirs; no merge-tree runs that long. The
  owner file and pid-space probe are gone, and with them the cases where a
  sweeper misjudged another process's pid.
- The sweep no longer delays the user's check: it starts without being
  awaited and can only touch dirs far older than the current run's.
- A fetched pack's .keep never moves into the real store, so a transient one
  from an aborted lazy fetch cannot stop gc from repacking that pack.
- Tests: the legacy merge-tree fallback gets the scratch env; the local
  quarantine's WSL decisions (distro from a UNC path, drvfs spelling,
  host-Git linked-worktree skip, WSLENV hand-off).

* fix(git): let quarantined merge-tree see exactly the object stores Git sees without it

The scratch object store must not change what merge-tree can read, or the
conflict list and the squash-merged branch cleanup differ from main:

- An inherited GIT_ALTERNATE_OBJECT_DIRECTORIES (from Orca's or the SSH
  relay's environment) was replaced; it is now kept and the real store
  appended after it, as Git's own temporary object stores do.
- An inherited GIT_OBJECT_DIRECTORY is the store Git reads and writes, so the
  command now runs unquarantined. On the WSL route a Windows value only
  counts if WSLENV forwards it, and then the quarantine is skipped too.
- The real store's own info/alternates entries are listed before it, so an
  alternates chain is linked at the same depth as without the quarantine
  (a 6-deep chain no longer hits Git's nesting limit). An alternates file
  that cannot be read or holds a quoted entry runs unquarantined.

* fix(git): run merge-tree unquarantined when the repo's store has its own alternates

Mirroring Git's alternates resolution could not match it in every shape: a
path back to the real store (mutual alternates, or an inherited alternates
value naming the repo itself) still linked its chain one level deeper, and a
NUL byte in info/alternates broke the spawn. When objects/info/alternates
exists, or cannot be checked, the command now runs exactly as before. Repos
cloned with --reference or --shared keep writing throwaway objects, as they
always have. Inherited alternates are still appended to, and an inherited
GIT_OBJECT_DIRECTORY still skips the quarantine.

* refactor(git): drop the stale-folder sweep and pack-move rollback from merge-tree's scratch store

A scratch folder left by a crash is a tmp_* dir inside objects/, which
Git's own gc prunes after gc.pruneExpire, as it does Git's own leftovers,
so Orca's 14-day sweep and its test hooks go.

Packs a partial clone fetched into the scratch store are moved in Git's
migration order: pack- files only, index last, existing names left in
place, stopping at the first failed rename with no rollback. A failed
move is logged and never stops the scratch dir's removal or the result.

Adds a real-Git test that a partial clone lists its conflicts offline
once an earlier check downloaded the contents.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(git): quarantine merge-tree only once the host is known to support --write-tree

Git before 2.35 cannot prune a leftover scratch folder under objects/tmp_*, so
a crash in the moment one exists makes every later `git gc` warn and stops
automatic cleanup. On Git before 2.38 merge-tree --write-tree is rejected, yet
the first check per host per session still made and removed a folder around it.

Branch cleanup and the PR conflict list now use the scratch object store only
when that host's capability cache already knows merge-tree --write-tree works.
Until then the command runs exactly as it does on main: on a Git that
rejects it no folder is ever created, and every Git that does get one is 2.38+,
whose gc prunes it. Also reword two comments in the quarantine module.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(git): learn merge-tree support from a conflicting branch-cleanup check

Branch cleanup quarantines merge-tree only once the host's capability cache
knows --write-tree works. A conflicting merge-tree exits 1, so the check threw
and support was never recorded; on a host whose only checks conflict (likely
over SSH, where the relay has no conflict summary to teach its cache) every
check kept writing loose objects into the real store.

A failure whose stdout starts with a tree ID proves --write-tree ran (old Git
that rejects the option prints nothing there), so it now resolves to the same
"not merged" result and lets the cache record support. Every other failure is
rethrown unchanged. Also shorten two comments and soften a third.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(git): put WSL Git's scratch object store on its command line

A WSL login profile that set GIT_ALTERNATE_OBJECT_DIRECTORIES replaced the
WSLENV-forwarded value, so a quarantined merge-tree lost the real store and
failed where main succeeds. The WSL runner now passes both variables as
command-line assignments after the profile, appending to its alternates.

* test(git): drop quarantine tests the real-Git and compatibility tests already pin

The cold-capability gating, scratch-folder lifecycle and WSL path spelling
are covered end to end by merge-tree-object-quarantine-real-git.test.ts, the
real-binary compatibility contract (Git 2.25/2.38/2.49) and the
localGitObjectsDirectory tests, so the unit copies only mirrored them.

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-10-15 15:23:18 -07:00

461 lines
16 KiB
TypeScript

import { mkdirSync, mkdtempSync, readdirSync, realpathSync, rmSync } from 'node:fs'
import type * as NodeFsPromises from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { isAbsolute, join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
// Why these tests exist: the conflict-summary derivation used to re-run a
// network `git fetch` plus a four-subprocess chain on every PR refresh tick
// (measured ~490 full derivations in 2.1h on one machine). They pin the
// regression contract by counting subprocess spawns: unchanged inputs must
// cost zero subprocesses, and the base fetch must run at most once per
// throttle window.
const gitExecFileAsyncMock = vi.hoisted(() => vi.fn())
const scratchFolderPrefixes = vi.hoisted((): string[] => [])
vi.mock('../git/runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock }))
// Why: off Windows a WSL objects path is relative, so a real scratch dir would land in the cwd.
vi.mock('node:fs/promises', async (importOriginal) => {
const actual = await importOriginal<typeof NodeFsPromises>()
return {
...actual,
mkdtemp: async (prefix: string) => {
scratchFolderPrefixes.push(prefix)
if (!isAbsolute(prefix)) {
throw new Error('no object store in this test')
}
return actual.mkdtemp(prefix)
}
}
})
import { GIT_CAPABILITY_RETRY_INTERVAL_MS } from '../../shared/git-capability-cache'
import { getLocalGitCapabilityCache } from '../git/git-capability-state'
import { CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS } from './conflict-summary-cache'
import { __resetPRConflictSummaryCachesForTests, getPRConflictSummary } from './conflict-summary'
type GitResult = { stdout: string }
type GitHandler = (argv: string[]) => Promise<GitResult>
// Why its own key: once merge-tree is known to work, its quarantine reads the common dir per derivation.
const COMMON_DIR_COMMAND = 'rev-parse --git-common-dir'
const defaultHandlers: Record<string, GitHandler> = {
fetch: async () => ({ stdout: '' }),
'rev-parse': async () => ({ stdout: 'base-tip-1\n' }),
'merge-base': async () => ({ stdout: 'merge-base-1\n' }),
'rev-list': async () => ({ stdout: '3\n' }),
'merge-tree': async () => ({ stdout: 'tree-oid\u0000src/conflict.ts\u0000' }),
[COMMON_DIR_COMMAND]: async () => ({ stdout: '.git\n' })
}
function commandKey(argv: string[]): string {
return argv[0] === 'rev-parse' && argv.includes('--git-common-dir') ? COMMON_DIR_COMMAND : argv[0]
}
function mockGitDispatch(overrides: Record<string, GitHandler> = {}): void {
gitExecFileAsyncMock.mockImplementation((argv: string[]) => {
const key = commandKey(argv)
const handler = overrides[key] ?? defaultHandlers[key]
if (!handler) {
return Promise.reject(new Error(`unexpected git command: ${argv.join(' ')}`))
}
return handler(argv)
})
}
function spawnCount(command?: string): number {
const calls = gitExecFileAsyncMock.mock.calls
if (!command) {
return calls.length
}
return calls.filter(([argv]) => Array.isArray(argv) && commandKey(argv) === command).length
}
const expectedSummary = {
baseRef: 'main',
baseCommit: 'base-ti',
commitsBehind: 3,
files: ['src/conflict.ts']
}
function deriveSummary(headRefOid = 'head-oid-1', wslDistro?: string) {
return getPRConflictSummary(
'/repo-root',
'main',
'github-base-oid',
headRefOid,
wslDistro ? { wslDistro } : {}
)
}
describe('getPRConflictSummary caching', () => {
beforeEach(() => {
vi.useFakeTimers()
vi.setSystemTime(1_750_000_000_000)
gitExecFileAsyncMock.mockReset()
scratchFolderPrefixes.length = 0
__resetPRConflictSummaryCachesForTests()
})
afterEach(() => {
vi.useRealTimers()
})
it('runs exactly one fetch and one derivation chain, then zero subprocesses on repeat calls', async () => {
mockGitDispatch()
const first = await deriveSummary()
expect(first).toEqual(expectedSummary)
expect(spawnCount('fetch')).toBe(1)
expect(spawnCount()).toBe(5)
const second = await deriveSummary()
const third = await deriveSummary()
expect(second).toEqual(expectedSummary)
expect(third).toEqual(expectedSummary)
expect(spawnCount()).toBe(5)
})
it('re-derives when headRefOid changes without re-fetching inside the throttle window', async () => {
mockGitDispatch()
await deriveSummary('head-oid-1')
const afterPush = await deriveSummary('head-oid-2')
expect(afterPush).toEqual(expectedSummary)
expect(spawnCount('fetch')).toBe(1)
expect(spawnCount('rev-parse')).toBe(1)
expect(spawnCount('merge-base')).toBe(2)
expect(spawnCount('rev-list')).toBe(2)
expect(spawnCount('merge-tree')).toBe(2)
})
it('fetches again after the throttle window and skips derivation when the base tip is unchanged', async () => {
mockGitDispatch()
await deriveSummary()
vi.setSystemTime(1_750_000_000_000 + CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS + 1_000)
const afterExpiry = await deriveSummary()
expect(afterExpiry).toEqual(expectedSummary)
expect(spawnCount('fetch')).toBe(2)
expect(spawnCount('rev-parse')).toBe(2)
// Why: same (headRefOid, latestBaseOid) pair — the summary cache still hits.
expect(spawnCount('merge-base')).toBe(1)
expect(spawnCount('rev-list')).toBe(1)
expect(spawnCount('merge-tree')).toBe(1)
})
it('re-derives after the throttle window when the base tip moved', async () => {
mockGitDispatch()
await deriveSummary()
vi.setSystemTime(1_750_000_000_000 + CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS + 1_000)
mockGitDispatch({
'rev-parse': async () => ({ stdout: 'base-tip-2\n' }),
'rev-list': async () => ({ stdout: '5\n' })
})
const afterBaseMove = await deriveSummary()
expect(afterBaseMove).toEqual({ ...expectedSummary, baseCommit: 'base-ti', commitsBehind: 5 })
expect(spawnCount('fetch')).toBe(2)
expect(spawnCount('merge-base')).toBe(2)
expect(spawnCount('rev-list')).toBe(2)
expect(spawnCount('merge-tree')).toBe(2)
})
it('dedupes concurrent identical calls onto one in-flight subprocess chain', async () => {
let releaseFetch: ((value: GitResult) => void) | undefined
mockGitDispatch({
fetch: () =>
new Promise<GitResult>((resolve) => {
releaseFetch = resolve
})
})
const firstCall = deriveSummary()
const secondCall = deriveSummary()
releaseFetch?.({ stdout: '' })
const [first, second] = await Promise.all([firstCall, secondCall])
expect(first).toEqual(expectedSummary)
expect(second).toEqual(expectedSummary)
expect(spawnCount()).toBe(5)
})
it('dedupes overlapping derivations after resolving the same live base tip', async () => {
let releaseFetch: ((value: GitResult) => void) | undefined
mockGitDispatch({
fetch: () =>
new Promise<GitResult>((resolve) => {
releaseFetch = resolve
})
})
const backgroundRefresh = getPRConflictSummary(
'/repo-root',
'main',
'older-github-base-oid',
'head-oid-1',
{}
)
const manualRefresh = getPRConflictSummary(
'/repo-root',
'main',
'newer-github-base-oid',
'head-oid-1',
{}
)
releaseFetch?.({ stdout: '' })
const [backgroundSummary, manualSummary] = await Promise.all([backgroundRefresh, manualRefresh])
expect(backgroundSummary).toEqual(expectedSummary)
expect(manualSummary).toEqual(expectedSummary)
expect(spawnCount('fetch')).toBe(1)
expect(spawnCount('rev-parse')).toBe(1)
expect(spawnCount('merge-base')).toBe(1)
expect(spawnCount('rev-list')).toBe(1)
expect(spawnCount('merge-tree')).toBe(1)
})
it('shares one base fetch across concurrent PRs on the same base branch', async () => {
let releaseFetch: ((value: GitResult) => void) | undefined
mockGitDispatch({
fetch: () =>
new Promise<GitResult>((resolve) => {
releaseFetch = resolve
})
})
const prA = deriveSummary('head-oid-a')
const prB = deriveSummary('head-oid-b')
releaseFetch?.({ stdout: '' })
const [summaryA, summaryB] = await Promise.all([prA, prB])
expect(summaryA).toEqual(expectedSummary)
expect(summaryB).toEqual(expectedSummary)
expect(spawnCount('fetch')).toBe(1)
expect(spawnCount('rev-parse')).toBe(1)
expect(spawnCount('merge-base')).toBe(2)
})
it('keeps concurrent fallback base OIDs isolated per PR when the base ref is unavailable', async () => {
let releaseFetch: ((value: GitResult) => void) | undefined
mockGitDispatch({
fetch: () =>
new Promise<GitResult>((resolve) => {
releaseFetch = resolve
}),
'rev-parse': () => Promise.reject(new Error('missing remote-tracking ref')),
'rev-list': async (argv) => ({
stdout: argv[2]?.includes('bbbb2222-base') ? '7\n' : '4\n'
}),
'merge-tree': async (argv) => ({
stdout: argv.includes('bbbb2222-base')
? 'tree-oid\u0000src/b.ts\u0000'
: 'tree-oid\u0000src/a.ts\u0000'
})
})
const prA = getPRConflictSummary('/repo-root', 'main', 'aaaa1111-base', 'head-oid-a', {})
const prB = getPRConflictSummary('/repo-root', 'main', 'bbbb2222-base', 'head-oid-b', {})
releaseFetch?.({ stdout: '' })
const [summaryA, summaryB] = await Promise.all([prA, prB])
expect(summaryA).toEqual({
baseRef: 'main',
baseCommit: 'aaaa111',
commitsBehind: 4,
files: ['src/a.ts']
})
expect(summaryB).toEqual({
baseRef: 'main',
baseCommit: 'bbbb222',
commitsBehind: 7,
files: ['src/b.ts']
})
expect(spawnCount('fetch')).toBe(1)
expect(spawnCount('merge-base')).toBe(2)
})
it('falls back to the local remote-tracking ref when fetch fails', async () => {
mockGitDispatch({
fetch: () => Promise.reject(new Error('offline'))
})
const summary = await deriveSummary()
expect(summary).toEqual(expectedSummary)
expect(spawnCount('fetch')).toBe(1)
// Why: a failed fetch attempt must also be throttled — offline machines
// were the worst case, paying the 10s fetch timeout on every tick.
const repeat = await deriveSummary()
expect(repeat).toEqual(expectedSummary)
expect(spawnCount('fetch')).toBe(1)
expect(spawnCount()).toBe(5)
})
it("falls back to GitHub's baseRefOid when fetch and remote-tracking refs are unavailable", async () => {
mockGitDispatch({
fetch: () => Promise.reject(new Error('offline')),
'rev-parse': () => Promise.reject(new Error('unknown revision'))
})
const summary = await deriveSummary()
expect(summary).toEqual({ ...expectedSummary, baseCommit: 'github-' })
expect(
gitExecFileAsyncMock.mock.calls.some(
([argv]) => argv[0] === 'merge-base' && argv.includes('github-base-oid')
)
).toBe(true)
})
it('keeps WSL-distro derivations isolated from host derivations', async () => {
mockGitDispatch()
await deriveSummary()
await deriveSummary('head-oid-1', 'Ubuntu')
expect(spawnCount('fetch')).toBe(2)
expect(
gitExecFileAsyncMock.mock.calls.filter(
([argv, options]) =>
argv[0] === 'fetch' && (options as { wslDistro?: string })?.wslDistro === 'Ubuntu'
)
).toHaveLength(1)
})
it('preserves background admission across the complete WSL derivation chain', async () => {
mockGitDispatch()
// Known support adds the quarantine's common-dir read to the chain.
getLocalGitCapabilityCache({ wslDistro: 'Ubuntu' }).rememberSupported('merge-tree-write-tree')
await getPRConflictSummary('/repo-root', 'main', 'github-base-oid', 'head-oid-1', {
wslDistro: 'Ubuntu',
admissionTier: 'background'
})
expect(spawnCount(COMMON_DIR_COMMAND)).toBe(1)
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(6)
for (const [, options] of gitExecFileAsyncMock.mock.calls) {
expect(options).toEqual(
expect.objectContaining({
cwd: '/repo-root',
wslDistro: 'Ubuntu',
admissionTier: 'background'
})
)
}
})
it('keeps identities distinct when paths or ref names contain a joiner character', async () => {
mockGitDispatch()
// Why: these two calls alias under naive pipe-joined keys — the segment
// boundary shifts between repoPath and baseRefName.
await getPRConflictSummary('/repo|x', 'main', 'github-base-oid', 'head-oid-1', {})
await getPRConflictSummary('/repo', 'x|main', 'github-base-oid', 'head-oid-1', {})
expect(spawnCount('fetch')).toBe(2)
expect(spawnCount('merge-base')).toBe(2)
})
it('caches failed derivations only until the throttle window expires', async () => {
mockGitDispatch({
'merge-base': () => Promise.reject(new Error('bad object'))
})
await expect(deriveSummary()).resolves.toBeUndefined()
const failureSpawns = spawnCount()
// Within the window the failure is negative-cached: zero subprocesses.
await expect(deriveSummary()).resolves.toBeUndefined()
expect(spawnCount()).toBe(failureSpawns)
// After expiry the derivation retries (a fetch may have brought objects in).
vi.setSystemTime(1_750_000_000_000 + CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS + 1_000)
mockGitDispatch()
await expect(deriveSummary()).resolves.toEqual(expectedSummary)
expect(spawnCount('merge-base')).toBe(2)
})
it('does not repeat merge-tree --write-tree after an old-Git rejection', async () => {
mockGitDispatch({
'merge-tree': () =>
Promise.reject(
Object.assign(new Error('unknown option'), {
stdout: 'usage: git merge-tree <base-tree> <branch1> <branch2>'
})
)
})
await expect(deriveSummary('head-oid-1')).resolves.toBeUndefined()
await expect(deriveSummary('head-oid-2')).resolves.toBeUndefined()
expect(spawnCount('merge-base')).toBe(2)
expect(spawnCount('merge-tree')).toBe(1)
})
it('makes no scratch folder for a Git that rejects merge-tree --write-tree, retries included', async () => {
// Git 2.25's exact rejection of both argument shapes.
const rejected = Object.assign(new Error('Command failed: git merge-tree --write-tree'), {
code: 129,
stdout: '',
stderr: 'usage: git merge-tree <base-tree> <branch1> <branch2>\n'
})
mockGitDispatch({ 'merge-tree': () => Promise.reject(rejected) })
await expect(deriveSummary('head-oid-1')).resolves.toBeUndefined()
vi.setSystemTime(1_750_000_000_000 + GIT_CAPABILITY_RETRY_INTERVAL_MS + 1_000)
await expect(deriveSummary('head-oid-2')).resolves.toBeUndefined()
expect(spawnCount('merge-tree')).toBe(2)
expect(spawnCount(COMMON_DIR_COMMAND)).toBe(0)
expect(scratchFolderPrefixes).toEqual([])
expect(gitExecFileAsyncMock.mock.calls.filter(([, options]) => options?.env)).toEqual([])
})
it('runs the legacy merge-tree fallback against the scratch object store too', async () => {
const commonDir = join(
realpathSync(mkdtempSync(join(tmpdir(), 'orca-conflict-summary-'))),
'.git'
)
mkdirSync(join(commonDir, 'objects'), { recursive: true })
try {
mockGitDispatch({
[COMMON_DIR_COMMAND]: async () => ({ stdout: `${commonDir}\n` }),
'merge-tree': async (argv) => {
if (argv.includes('--merge-base')) {
throw Object.assign(new Error("error: unknown option `merge-base'"), { stderr: '' })
}
return { stdout: 'tree-oid\u0000src/conflict.ts\u0000' }
}
})
// The first check learns that this Git writes trees; the next one is quarantined.
await expect(deriveSummary('head-oid-1')).resolves.toEqual(expectedSummary)
await expect(deriveSummary('head-oid-2')).resolves.toEqual(expectedSummary)
const legacy = gitExecFileAsyncMock.mock.calls.filter(
([argv]) => argv[0] === 'merge-tree' && !argv.includes('--merge-base')
)
expect(legacy).toHaveLength(2)
expect(legacy[0]?.[1]?.env).toBeUndefined()
expect(legacy[1]?.[1]?.env).toMatchObject({
GIT_OBJECT_DIRECTORY: expect.stringContaining(
join(commonDir, 'objects', 'tmp_objdir-orca-merge-tree-')
),
GIT_ALTERNATE_OBJECT_DIRECTORIES: join(commonDir, 'objects')
})
expect(spawnCount('merge-tree')).toBe(3)
expect(readdirSync(join(commonDir, 'objects'))).toEqual([])
} finally {
rmSync(join(commonDir, '..'), { recursive: true, force: true })
}
})
})