mirror of
https://github.com/stablyai/orca.git
synced 2026-10-07 08:02:21 +00:00
* Clarify PR panel guidance: classify errors and confirm-only composer Replace the ambiguous GitHub hosted-review boolean with a four-state evidence model (found/positive_unresolved/not_found/unknown) so "No PR found" never appears without an accepted lookup result. Classify GitHub refresh failures into types (rate_limited, auth, network, permission, repo_unavailable, gh_unavailable, unknown) for stable, honest copy. Confirmed-only composer: preserve drafts across transient failures; hide Create during hard errors and positive-unresolved evidence. Hard errors clear only when an eligibility request starts after the error and returns an accepted outcome. Propagate error types and unified retry schedule through the store. Sync mobile parity with shouldOpenChecksPanelCreateComposer gating. Localize all new copy. * Clarify PR panel guidance: classify errors and confirm-only composer Add reviewLookupOutcome to hosted-review eligibility and thread it through the panel so it never claims "No PR found" without accepted evidence. A failed lookup is unavailable, not a settled no-PR. Fail closed on positive unresolved evidence, hard refresh errors, and unavailable lookups. Add structured GitHub refresh-error classification with Retry-After parsing. Implement confirmed-only composer gating based on fresh, matching-context eligibility with hard-error clearing. Mobile gates on reviewLookupOutcome to prevent false Create claims. Surface throwOnFailure variants for each provider so transport failures cross the RPC boundary instead of collapsing to null. (Design success criteria 1–4; invariant 8.) * Add exec-error helpers for subprocess error classification Extracts stderr/stdout parsing and Retry-After detection into a lightweight module that can be imported without pulling in the heavier runner machinery. Supports PR-refresh error classification and proper rate-limit handling for gh commands. * test(mobile): include reviewLookupOutcome in create eligibility fixtures Create / Push & Create now fails closed unless the lookup is not_found. Update mobile test fixtures so accepted-no-PR cases can still proceed. * Add OrThrow mock variants to forge-provider test mocks forge-provider resolves branch reviews via the OrThrow variant so lookup failures surface as unavailable instead of "no PR found".
145 lines
5.3 KiB
TypeScript
145 lines
5.3 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
import {
|
|
classifyPRRefreshError,
|
|
safePRRefreshErrorMessage
|
|
} from './pr-refresh-error-classification'
|
|
|
|
describe('classifyPRRefreshError', () => {
|
|
it('classifies an HTTP 429 as rate_limited even without a rate-limit body', () => {
|
|
expect(classifyPRRefreshError(new Error('HTTP 429 Too Many Requests'))).toBe('rate_limited')
|
|
})
|
|
|
|
it('classifies secondary rate limit markers as rate_limited, not permission', () => {
|
|
for (const message of [
|
|
'You have exceeded a secondary rate limit',
|
|
'abuse detection mechanism triggered',
|
|
'abuse-rate-limits',
|
|
'you have triggered an abuse detection mechanism'
|
|
]) {
|
|
expect(classifyPRRefreshError(new Error(message))).toBe('rate_limited')
|
|
}
|
|
})
|
|
|
|
it('classifies a 403 carrying Retry-After as rate_limited, not permission', () => {
|
|
expect(classifyPRRefreshError(new Error('HTTP 403 Forbidden; Retry-After: 60'))).toBe(
|
|
'rate_limited'
|
|
)
|
|
})
|
|
|
|
it('classifies the primary breaker language as rate_limited', () => {
|
|
expect(classifyPRRefreshError(new Error('API rate limit exceeded for user'))).toBe(
|
|
'rate_limited'
|
|
)
|
|
})
|
|
|
|
it('classifies a plain 403 resource denial as permission', () => {
|
|
expect(
|
|
classifyPRRefreshError(new Error('HTTP 403: Resource not accessible by integration'))
|
|
).toBe('permission')
|
|
})
|
|
|
|
it('classifies network failures', () => {
|
|
for (const message of ['ETIMEDOUT', 'could not resolve host github.com', 'network is down']) {
|
|
expect(classifyPRRefreshError(new Error(message))).toBe('network')
|
|
}
|
|
})
|
|
|
|
it('classifies 404 / could not resolve repository as repo_unavailable', () => {
|
|
expect(classifyPRRefreshError(new Error('HTTP 404 Not Found'))).toBe('repo_unavailable')
|
|
expect(
|
|
classifyPRRefreshError(new Error('Could not resolve to a Repository with the name'))
|
|
).toBe('repo_unavailable')
|
|
})
|
|
|
|
it('classifies an ENOENT spawn failure as gh_unavailable', () => {
|
|
const err = Object.assign(new Error('spawn gh ENOENT'), { code: 'ENOENT' })
|
|
expect(classifyPRRefreshError(err)).toBe('gh_unavailable')
|
|
expect(classifyPRRefreshError(new Error("'gh' is not recognized as an internal command"))).toBe(
|
|
'gh_unavailable'
|
|
)
|
|
})
|
|
|
|
it('classifies auth failures after rate-limit and permission checks', () => {
|
|
expect(classifyPRRefreshError(new Error('authentication failed: bad credentials'))).toBe('auth')
|
|
})
|
|
|
|
it('falls back to unknown', () => {
|
|
expect(classifyPRRefreshError(new Error('something unexpected happened'))).toBe('unknown')
|
|
})
|
|
|
|
it('does not classify a message that merely contains "author" as auth', () => {
|
|
for (const message of [
|
|
'PR author octocat has no write access to the fork',
|
|
'authored 3 commits, none pushed',
|
|
'unexpected failure from author service'
|
|
]) {
|
|
expect(classifyPRRefreshError(new Error(message))).toBe('unknown')
|
|
}
|
|
})
|
|
|
|
it('does not classify a repository name containing "network" as a network failure', () => {
|
|
// A repo/branch called "network-*" is not a connectivity failure; without a
|
|
// structured code or a real connectivity phrase this must stay unknown.
|
|
expect(classifyPRRefreshError(new Error('operation failed for repo acme/network-tools'))).toBe(
|
|
'unknown'
|
|
)
|
|
})
|
|
|
|
it('classifies a 404 repository error even when the message mentions network', () => {
|
|
// 404 ranks before the network branch so the repo error is not misread.
|
|
expect(
|
|
classifyPRRefreshError(
|
|
new Error('Could not resolve to a Repository named acme/network-proxy (HTTP 404)')
|
|
)
|
|
).toBe('repo_unavailable')
|
|
})
|
|
|
|
it('classifies a 401 as auth', () => {
|
|
expect(classifyPRRefreshError(new Error('HTTP 401 Unauthorized'))).toBe('auth')
|
|
})
|
|
|
|
it('classifies from the real runner error shape where the signal is on .stderr', () => {
|
|
// Why: the runner rejects with a generic message and puts the gh diagnostic
|
|
// on `.stderr` (string or Buffer). Reading `.message` alone would misclassify
|
|
// these hard errors as `unknown` and treat them as transient.
|
|
const permission = Object.assign(new Error('gh exited with 1.'), {
|
|
stderr: 'HTTP 403: Resource not accessible by integration'
|
|
})
|
|
expect(classifyPRRefreshError(permission)).toBe('permission')
|
|
|
|
const repo = Object.assign(new Error('gh exited with 1.'), {
|
|
stderr: Buffer.from('HTTP 404: Not Found')
|
|
})
|
|
expect(classifyPRRefreshError(repo)).toBe('repo_unavailable')
|
|
|
|
const secondary = Object.assign(new Error('gh exited with 1.'), {
|
|
stderr: 'You have exceeded a secondary rate limit'
|
|
})
|
|
expect(classifyPRRefreshError(secondary)).toBe('rate_limited')
|
|
|
|
const auth = Object.assign(new Error('gh exited with 1.'), {
|
|
stdout: '',
|
|
stderr: 'gh auth login required: bad credentials'
|
|
})
|
|
expect(classifyPRRefreshError(auth)).toBe('auth')
|
|
})
|
|
})
|
|
|
|
describe('safePRRefreshErrorMessage', () => {
|
|
it('returns non-empty copy for every classified type without leaking raw errors', () => {
|
|
for (const type of [
|
|
'rate_limited',
|
|
'auth',
|
|
'network',
|
|
'permission',
|
|
'repo_unavailable',
|
|
'gh_unavailable',
|
|
'unknown'
|
|
] as const) {
|
|
const message = safePRRefreshErrorMessage(type)
|
|
expect(message.length).toBeGreaterThan(0)
|
|
expect(message).not.toContain('ENOENT')
|
|
}
|
|
})
|
|
})
|