mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 00:03:20 +00:00
* Fix automatic branch rename for non-English git locales A gettext-enabled git (Homebrew git, most Linux distro gits) under a non-English locale translates every diagnostic, including the `fatal:` prefix, so Orca's stderr phrase parsers stop matching. The first-message branch auto-rename was the headline casualty: isNoUpstreamError missed the translated no-upstream error, branchHasUpstream failed closed to "has upstream", and the rename settled silently and permanently. - Force LC_ALL=C on all Orca-spawned machine-parsed git: the local prompt-guard env chokepoint, the three relay git spawn sites, and both local clone spawns (progress + failure-message parsing). User terminals are untouched. - Replace the boolean upstream check with a tri-state probe: rename proceeds only on a proven missing upstream; an unreadable probe now raises the rename-failed badge and retries instead of settling. Fixes #7808 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * Consolidate untranslated-git-locale into runner and relay primitives Replace the five per-site LC_ALL=C patches with one shared UNTRANSLATED_GIT_OUTPUT_ENV (LANGUAGE=en LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8) injected inside the git runner primitives (promptGuardGitEnv, gitSpawn, gitExecFileSync, gitExecFileAsyncBuffer) and a relay buildRelayGitEnv() helper, so every current and future machine-parsed git spawn is covered by construction — including the fs-handler-git-fallback sites the per-site approach missed. The UTF-8 English locale keeps a UTF-8 LC_CTYPE for hooks git spawns; LANGUAGE is pinned because gettext consults it before LC_ALL. WSL-routed git gets the same values as a shell assignment prefix built in resolveCommand, since spawn env cannot cross the wsl.exe boundary — closing the WSL gap the first pass accepted. Also scrub credential-bearing remote URLs from the probe-failed message surfaced on the worktree card. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * Scrub credential-bearing URLs from clone failure messages --------- Co-authored-by: Brennan Benson <brennanbenson@Brennans-MacBook-Pro.local> Co-authored-by: Claude <noreply@anthropic.com>
244 lines
8.9 KiB
TypeScript
244 lines
8.9 KiB
TypeScript
// Why: covers two recent classifier fixes — Retry-After honoring on 429
|
|
// (transient detection must propagate, not silently retry on 250ms cadence)
|
|
// and stderr extraction from execFile rejections (err.message is unreliable).
|
|
import { afterEach, describe, expect, it, vi } from 'vitest'
|
|
import {
|
|
appendGitConfigEnv,
|
|
extractExecError,
|
|
isTransientGhError,
|
|
nonInteractiveGitEnv,
|
|
parseRetryAfterMs,
|
|
promptGuardGitEnv,
|
|
redirectPortedHostnameToEnv,
|
|
untranslatedGitOutputEnv
|
|
} from './runner'
|
|
|
|
// Reads git config injected via the GIT_CONFIG_COUNT/KEY/VALUE env protocol
|
|
// back into a plain key→value map so tests can assert on it directly.
|
|
function readGitConfigEnv(env: NodeJS.ProcessEnv): Record<string, string> {
|
|
const count = Number.parseInt(env.GIT_CONFIG_COUNT ?? '0', 10)
|
|
const config: Record<string, string> = {}
|
|
for (let i = 0; i < count; i++) {
|
|
const key = env[`GIT_CONFIG_KEY_${i}`]
|
|
const value = env[`GIT_CONFIG_VALUE_${i}`]
|
|
if (key !== undefined && value !== undefined) {
|
|
config[key] = value
|
|
}
|
|
}
|
|
return config
|
|
}
|
|
|
|
describe('redirectPortedHostnameToEnv', () => {
|
|
it('moves a ported --hostname into GITLAB_HOST and strips the flag', () => {
|
|
const { args, options } = redirectPortedHostnameToEnv(
|
|
['api', '--hostname', 'gitlab.example.com:8443', 'projects/foo%2Fbar/issues'],
|
|
{ cwd: '/repo' }
|
|
)
|
|
expect(args).toEqual(['api', 'projects/foo%2Fbar/issues'])
|
|
expect(options.env?.GITLAB_HOST).toBe('gitlab.example.com:8443')
|
|
expect(options.cwd).toBe('/repo')
|
|
})
|
|
|
|
it('leaves a port-less --hostname untouched', () => {
|
|
const input = ['api', '--hostname', 'gitlab.com', 'user']
|
|
const { args, options } = redirectPortedHostnameToEnv(input, {})
|
|
expect(args).toEqual(input)
|
|
expect(options.env).toBeUndefined()
|
|
})
|
|
|
|
it('is a no-op when no --hostname is present', () => {
|
|
const input = ['auth', 'status']
|
|
const { args, options } = redirectPortedHostnameToEnv(input, { env: { A: '1' } })
|
|
expect(args).toEqual(input)
|
|
expect(options.env).toEqual({ A: '1' })
|
|
})
|
|
|
|
it('preserves existing env entries alongside GITLAB_HOST', () => {
|
|
const { options } = redirectPortedHostnameToEnv(
|
|
['auth', 'status', '--hostname', 'gl.example.org:3001'],
|
|
{ env: { PATH: '/usr/bin' } }
|
|
)
|
|
expect(options.env?.PATH).toBe('/usr/bin')
|
|
expect(options.env?.GITLAB_HOST).toBe('gl.example.org:3001')
|
|
})
|
|
})
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks()
|
|
})
|
|
|
|
describe('parseRetryAfterMs', () => {
|
|
it('returns null when no Retry-After is present', () => {
|
|
expect(parseRetryAfterMs('HTTP 429 Too Many Requests')).toBeNull()
|
|
})
|
|
|
|
it('parses integer seconds', () => {
|
|
expect(parseRetryAfterMs('HTTP 429\nRetry-After: 30\n')).toBe(30_000)
|
|
})
|
|
|
|
it('handles case-insensitive header name and surrounding whitespace', () => {
|
|
expect(parseRetryAfterMs(' retry-after: 12 \n')).toBe(12_000)
|
|
})
|
|
|
|
it('parses large stderr output without full-string retry-after matching', () => {
|
|
const matchSpy = vi.spyOn(String.prototype, 'match')
|
|
const stderr = `${'noise\n'.repeat(10_000)}Retry-After: 12\n`
|
|
|
|
expect(parseRetryAfterMs(stderr)).toBe(12_000)
|
|
const usedRetryAfterMatch = matchSpy.mock.calls.some(
|
|
([pattern]) =>
|
|
pattern instanceof RegExp &&
|
|
pattern.source.startsWith('retry-after:') &&
|
|
pattern.source.includes('[^\\r\\n]')
|
|
)
|
|
expect(usedRetryAfterMatch).toBe(false)
|
|
})
|
|
|
|
it('returns null for malformed values', () => {
|
|
expect(parseRetryAfterMs('Retry-After: not-a-date')).toBeNull()
|
|
})
|
|
})
|
|
|
|
describe('isTransientGhError', () => {
|
|
it('retries 5xx errors', () => {
|
|
expect(isTransientGhError('HTTP 502 Bad Gateway')).toBe(true)
|
|
expect(isTransientGhError('http 503')).toBe(true)
|
|
})
|
|
|
|
it('retries network resets', () => {
|
|
expect(isTransientGhError('connect ECONNRESET 10.0.0.1:443')).toBe(true)
|
|
expect(isTransientGhError('socket hang up')).toBe(true)
|
|
})
|
|
|
|
it('retries 429 without Retry-After', () => {
|
|
expect(isTransientGhError('HTTP 429 Too Many Requests')).toBe(true)
|
|
})
|
|
|
|
it('does NOT retry 429 with Retry-After', () => {
|
|
// Why: when GitHub returns Retry-After, the server is telling us how long
|
|
// to wait. Retrying on our 250ms cadence just earns another 429 and burns
|
|
// the retry budget.
|
|
expect(isTransientGhError('HTTP 429 Too Many Requests\nRetry-After: 60\n')).toBe(false)
|
|
})
|
|
|
|
it("does NOT retry 4xx that aren't 429", () => {
|
|
expect(isTransientGhError('HTTP 401 Unauthorized')).toBe(false)
|
|
expect(isTransientGhError('HTTP 404 Not Found')).toBe(false)
|
|
expect(isTransientGhError('HTTP 422 Unprocessable Entity')).toBe(false)
|
|
})
|
|
})
|
|
|
|
describe('extractExecError', () => {
|
|
it('reads stderr and stdout from explicit fields', () => {
|
|
const err = Object.assign(new Error('Command failed'), {
|
|
stderr: 'real stderr content',
|
|
stdout: '{"data": null}'
|
|
})
|
|
expect(extractExecError(err)).toEqual({
|
|
stderr: 'real stderr content',
|
|
stdout: '{"data": null}'
|
|
})
|
|
})
|
|
|
|
it('decodes Buffer stderr/stdout', () => {
|
|
const err = Object.assign(new Error('Command failed'), {
|
|
stderr: Buffer.from('buf-stderr', 'utf-8'),
|
|
stdout: Buffer.from('buf-stdout', 'utf-8')
|
|
})
|
|
expect(extractExecError(err)).toEqual({
|
|
stderr: 'buf-stderr',
|
|
stdout: 'buf-stdout'
|
|
})
|
|
})
|
|
|
|
it('falls back to err.message when stderr/stdout are absent', () => {
|
|
const err = new Error('Some message')
|
|
expect(extractExecError(err)).toEqual({
|
|
stderr: 'Some message',
|
|
stdout: ''
|
|
})
|
|
})
|
|
|
|
it('handles non-Error rejections', () => {
|
|
expect(extractExecError('plain string error')).toEqual({
|
|
stderr: 'plain string error',
|
|
stdout: ''
|
|
})
|
|
})
|
|
})
|
|
|
|
describe('appendGitConfigEnv', () => {
|
|
it('injects entries starting at count 0 when none exist', () => {
|
|
const env = appendGitConfigEnv({ PATH: '/usr/bin' }, [['credential.interactive', 'false']])
|
|
expect(env.GIT_CONFIG_COUNT).toBe('1')
|
|
expect(env.GIT_CONFIG_KEY_0).toBe('credential.interactive')
|
|
expect(env.GIT_CONFIG_VALUE_0).toBe('false')
|
|
expect(env.PATH).toBe('/usr/bin')
|
|
})
|
|
|
|
it('composes with an existing count instead of clobbering caller config', () => {
|
|
const env = appendGitConfigEnv(
|
|
{ GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'core.quotePath', GIT_CONFIG_VALUE_0: 'false' },
|
|
[['credential.guiPrompt', 'false']]
|
|
)
|
|
expect(env.GIT_CONFIG_COUNT).toBe('2')
|
|
// Existing entry preserved.
|
|
expect(env.GIT_CONFIG_KEY_0).toBe('core.quotePath')
|
|
expect(env.GIT_CONFIG_VALUE_0).toBe('false')
|
|
// New entry appended at the next index.
|
|
expect(env.GIT_CONFIG_KEY_1).toBe('credential.guiPrompt')
|
|
expect(env.GIT_CONFIG_VALUE_1).toBe('false')
|
|
})
|
|
})
|
|
|
|
describe('promptGuardGitEnv credential-interactivity disable (STA-1292)', () => {
|
|
it('disables the GCM GUI prompt without nuking the credential helper', () => {
|
|
const env = promptGuardGitEnv({ PATH: '/usr/bin' })
|
|
// GCM: never show the GUI, but still serve cached credentials.
|
|
expect(env.GCM_INTERACTIVE).toBe('never')
|
|
const config = readGitConfigEnv(env)
|
|
expect(config['credential.interactive']).toBe('false')
|
|
expect(config['credential.guiPrompt']).toBe('false')
|
|
// Regression guard: we must NOT clear the helper — that would break
|
|
// cached-credential auth for private repos.
|
|
expect(config['credential.helper']).toBeUndefined()
|
|
// Existing prompt guards remain intact.
|
|
expect(env.GIT_TERMINAL_PROMPT).toBe('0')
|
|
})
|
|
})
|
|
|
|
describe('nonInteractiveGitEnv credential-interactivity disable (STA-1292)', () => {
|
|
it('carries the credential-interactivity disable through from promptGuardGitEnv', () => {
|
|
const env = nonInteractiveGitEnv({ PATH: '/usr/bin' })
|
|
expect(env.GCM_INTERACTIVE).toBe('never')
|
|
const config = readGitConfigEnv(env)
|
|
expect(config['credential.interactive']).toBe('false')
|
|
expect(config['credential.guiPrompt']).toBe('false')
|
|
expect(config['credential.helper']).toBeUndefined()
|
|
// Its own BatchMode SSH guard is still applied and unaffected.
|
|
expect(env.GIT_SSH_COMMAND).toBe('ssh -o BatchMode=yes')
|
|
})
|
|
})
|
|
|
|
describe('git env forces untranslated diagnostics (issue #7808)', () => {
|
|
it('overrides an inherited non-English locale so stderr parsers keep working', () => {
|
|
// A gettext-enabled git under de_DE translates even the `fatal:` prefix,
|
|
// breaking isNoUpstreamError and every other stderr phrase match.
|
|
const env = promptGuardGitEnv({ PATH: '/usr/bin', LC_ALL: 'de_DE.UTF-8' })
|
|
expect(env.LC_ALL).toBe('en_US.UTF-8')
|
|
expect(env.LANG).toBe('en_US.UTF-8')
|
|
})
|
|
|
|
it('pins LANGUAGE, which outranks LC_ALL in gettext lookups', () => {
|
|
const env = untranslatedGitOutputEnv({ PATH: '/usr/bin', LANGUAGE: 'de:en' })
|
|
expect(env.LANGUAGE).toBe('en')
|
|
expect(env.LC_ALL).toBe('en_US.UTF-8')
|
|
})
|
|
|
|
it('applies to nonInteractiveGitEnv as well', () => {
|
|
const env = nonInteractiveGitEnv({ PATH: '/usr/bin', LANG: 'fr_FR.UTF-8' })
|
|
expect(env.LC_ALL).toBe('en_US.UTF-8')
|
|
expect(env.LANGUAGE).toBe('en')
|
|
})
|
|
})
|