starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-30 16:47:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf(git): bound ref and worktree scans (#17655)

Browse Source 
* perf(git): bound ref and worktree scans

* fix(repo-search): clamp oversized ref limits

* fix(worktree): keep strict worktree listing unshared

The shared-scan re-export flipped every `listWorktreesStrict` caller from an
isolated subprocess to the coalesced scan. `git worktree prune` in the removal
recovery path does not bump the scan generation, so a post-prune verification
could join a pre-prune scan, see the stale row, and report a successful removal
as a stale registration. The same gap defeats the post-archive-hook rechecks
that exist to catch an external Git client locking the row.

Restore the unshared export and make coalescing opt-in via
`listWorktreesSharedStrict`, which existing callers already use deliberately.

* fix(git): separate a proven absent ref from a failed probe

`show-ref --verify --quiet` exits 1 for a missing ref, but so does `wsl.exe`
when its own launch fails, so reading any exit 1 as absence collapsed
`unverifiable` into `exited`. A genuine miss prints nothing while a wrapper
failure always explains itself, so require empty stderr alongside the exit
code; a runner that reports no stderr at all keeps its exit-code contract.

That same signal removes a spawn regression: `show-ref` is a direct-git read
under WSL, and the runner retried any numeric exit through the user's
interactive login shell. The replaced `for-each-ref` exited 0 on a miss, so
absence never retried; every absent probe now would. Treat a quiet exit 1 as
Git control flow and skip the fallback.

Also narrow the hosted-review suffix fallback: the replaced
`refs/remotes/*/<base>` could not cross a slash, but `show-ref -- <base>`
matches at any depth, so `origin/feature/main` answered a query for `main`
and submitted a review against a base the provider rejects.

Refresh the real-binary compatibility contract to the shipped excludes, and
assert exact probe concurrency rather than an upper bound so a regression to
serial probing fails.
This commit is contained in:
Neil
2026-08-31 16:27:28 -07:00
committed by GitHub
parent a5be10815c
commit 8ac1c6e2ac
48 changed files with 2750 additions and 222 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/text-generation/pull-request-context-errors.test.ts
+150 -6
View File
@@ -27,8 +27,8 @@ describe('getPullRequestDraftContext error handling', () => {
if (args[0] === 'remote') {
return { stdout: 'origin\n', stderr: '' }
}
if (args[0] === 'for-each-ref') {
return { stdout: 'origin/main\n', stderr: '' }
if (args[0] === 'show-ref') {
return { stdout: '', stderr: '' }
}
if (args[0] === 'branch') {
return { stdout: 'feature\n', stderr: '' }
@@ -59,8 +59,8 @@ describe('getPullRequestDraftContext error handling', () => {
if (args[0] === 'remote') {
return { stdout: 'origin\nstale-fork\n', stderr: '' }
}
if (args[0] === 'for-each-ref') {
return { stdout: 'origin/main\nstale-fork/main\n', stderr: '' }
if (args[0] === 'show-ref') {
throw Object.assign(new Error('missing exact ref'), { code: 1 })
}
if (args[0] === 'fetch') {
if (args[2] !== 'origin') {
@@ -84,8 +84,8 @@ describe('getPullRequestDraftContext error handling', () => {
if (args[0] === 'remote') {
return { stdout: `${'\r\n'.repeat(10_000)}origin\r\n`, stderr: '' }
}
if (args[0] === 'for-each-ref') {
return { stdout: `${'\r\n'.repeat(10_000)}origin/main\r\n`, stderr: '' }
if (args[0] === 'show-ref') {
throw Object.assign(new Error('missing exact ref'), { code: 1 })
}
if (args[0] === 'fetch') {
throw new Error(
@@ -115,4 +115,148 @@ describe('getPullRequestDraftContext error handling', () => {
)
expect(execGit).not.toHaveBeenCalled()
})
it.each(['origin/feature*', 'origin/feature?', 'origin/feature:other', 'origin/feature\u0000'])(
'does not turn unsafe characters in a qualified base into a fetch refspec (%s)',
async (base) => {
const execGit = vi.fn<GitExec>()
await expect(getPullRequestDraftContext(execGit, createContextInput(base))).resolves.toBe(
null
)
expect(execGit).not.toHaveBeenCalled()
}
)
it('ignores stale tracking refs whose remote name could be parsed as a fetch option', async () => {
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'remote') {
return { stdout: '--upload-pack=x\n' }
}
if (args[0] === 'show-ref' && args.includes('--verify')) {
throw Object.assign(new Error('missing exact ref'), { code: 1 })
}
if (args[0] === 'show-ref') {
return { stdout: 'abc refs/remotes/--upload-pack=x/main\n' }
}
if (args[0] === 'branch') {
return { stdout: 'feature\n' }
}
if (args[0] === 'merge-base') {
expect(args[1]).toBe('main')
return { stdout: 'abc123\n' }
}
if (args[0] === 'log' || args[0] === 'diff') {
return { stdout: 'change\n' }
}
throw new Error(`Unexpected git args: ${args.join(' ')}`)
})
await expect(getPullRequestDraftContext(execGit, createContextInput())).resolves.toMatchObject({
branch: 'feature'
})
expect(execGit).not.toHaveBeenCalledWith(expect.arrayContaining(['fetch']), expect.any(Object))
})
it('preserves the existing bare HEAD remote-base behavior', async () => {
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'remote') {
return { stdout: 'origin\n' }
}
if (args[0] === 'show-ref') {
throw Object.assign(new Error('missing exact ref'), { code: 1 })
}
if (args[0] === 'fetch') {
return { stdout: '' }
}
if (args[0] === 'branch') {
return { stdout: 'feature\n' }
}
if (args[0] === 'merge-base') {
expect(args[1]).toBe('origin/HEAD')
return { stdout: 'abc123\n' }
}
if (args[0] === 'log' || args[0] === 'diff') {
return { stdout: 'change\n' }
}
throw new Error(`Unexpected git args: ${args.join(' ')}`)
})
await expect(
getPullRequestDraftContext(execGit, createContextInput('HEAD'))
).resolves.toMatchObject({ base: 'HEAD', branch: 'feature' })
expect(execGit).toHaveBeenCalledWith(
['fetch', '--no-tags', 'origin', '+refs/heads/HEAD:refs/remotes/origin/HEAD'],
expect.any(Object)
)
})
it('does not resolve bare HEAD to a nested branch ending in HEAD', async () => {
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'remote') {
return { stdout: 'fork\n' }
}
if (args[0] === 'show-ref' && args.some((arg) => arg.startsWith('refs/remotes/'))) {
throw Object.assign(new Error('missing exact ref'), { code: 1 })
}
if (args[0] === 'show-ref') {
expect(args).toEqual(['show-ref', '--', 'HEAD'])
return { stdout: 'abc123 refs/remotes/fork/feature/HEAD\n' }
}
if (args[0] === 'branch') {
return { stdout: 'feature\n' }
}
if (args[0] === 'merge-base') {
expect(args[1]).toBe('HEAD')
return { stdout: 'abc123\n' }
}
if (args[0] === 'log' || args[0] === 'diff') {
return { stdout: 'change\n' }
}
throw new Error(`Unexpected git args: ${args.join(' ')}`)
})
await expect(
getPullRequestDraftContext(execGit, createContextInput('HEAD'))
).resolves.toMatchObject({ base: 'HEAD', branch: 'feature' })
expect(execGit).not.toHaveBeenCalledWith(
['fetch', '--no-tags', 'fork', '+refs/heads/feature/HEAD:refs/remotes/fork/feature/HEAD'],
expect.any(Object)
)
})
it('preserves an explicitly qualified remote HEAD ref', async () => {
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'remote') {
return { stdout: 'origin\n' }
}
if (args[0] === 'show-ref') {
return args.at(-1) === 'refs/remotes/origin/HEAD'
? { stdout: '' }
: Promise.reject(Object.assign(new Error('missing exact ref'), { code: 1 }))
}
if (args[0] === 'fetch') {
return { stdout: '' }
}
if (args[0] === 'branch') {
return { stdout: 'feature\n' }
}
if (args[0] === 'merge-base') {
expect(args[1]).toBe('origin/HEAD')
return { stdout: 'abc123\n' }
}
if (args[0] === 'log' || args[0] === 'diff') {
return { stdout: 'change\n' }
}
throw new Error(`Unexpected git args: ${args.join(' ')}`)
})
await expect(
getPullRequestDraftContext(execGit, createContextInput('origin/HEAD'))
).resolves.toMatchObject({ base: 'origin/HEAD', branch: 'feature' })
expect(execGit).toHaveBeenCalledWith(
['fetch', '--no-tags', 'origin', '+refs/heads/HEAD:refs/remotes/origin/HEAD'],
expect.any(Object)
)
})
})