mirror of
https://github.com/stablyai/orca.git
synced 2026-10-25 08:04:05 +00:00
fix(rate-limits): safely surface Codex RPC exits
This commit is contained in:
@@ -82,9 +82,9 @@ describe('Codex RPC exit diagnostics', () => {
|
||||
2
|
||||
)
|
||||
|
||||
expect(result.error).toBe(
|
||||
"Codex RPC process exited (exit code 2): error: invalid value 'untrusted' for '--ask-for-approval <APPROVAL_POLICY>'"
|
||||
)
|
||||
expect(result.error).toContain('Codex RPC process exited (exit code 2)')
|
||||
expect(result.error).toContain("invalid value 'untrusted'")
|
||||
expect(result.error).toContain("try '--help'")
|
||||
})
|
||||
|
||||
it('names the exit code when the child says nothing', async () => {
|
||||
@@ -101,9 +101,7 @@ describe('Codex RPC exit diagnostics', () => {
|
||||
1
|
||||
)
|
||||
|
||||
expect(result.error).toBe(
|
||||
'ERROR: The ChatGPT access token could not be refreshed; please sign in again.'
|
||||
)
|
||||
expect(result.error).toBe('Your ChatGPT session could not be refreshed. Please sign in again.')
|
||||
// The renderer's re-auth warning gates on exactly this predicate;
|
||||
// codex-account-auth-warning.test.ts asserts the surfaced string end-to-end.
|
||||
expect(isCodexAuthError(result.error)).toBe(true)
|
||||
@@ -129,4 +127,14 @@ describe('Codex RPC exit diagnostics', () => {
|
||||
expect(result.error).not.toContain('sk-aaaa')
|
||||
expect(result.error?.length ?? 0).toBeLessThan(500)
|
||||
})
|
||||
|
||||
it('keeps the final failure after startup banners', async () => {
|
||||
const result = await runStub(
|
||||
'Welcome to WSL\nLoading shell profile\nERROR: unknown option --obsolete\n',
|
||||
2
|
||||
)
|
||||
|
||||
expect(result.error).toContain('Welcome to WSL')
|
||||
expect(result.error).toContain('ERROR: unknown option --obsolete')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -25,8 +25,10 @@ import { getCmdExePath, getSpawnArgsForWindows } from '../win32-utils'
|
||||
import { cleanupHiddenRateLimitPty, registerHiddenRateLimitPty } from './hidden-pty-cleanup'
|
||||
import { parseWslUncPath } from '../../shared/wsl-paths'
|
||||
import { extractCodexAuthError, isCodexAuthError } from '../../shared/codex-auth-errors'
|
||||
import { stripAnsiControlSequences } from '../../shared/commit-message-agent-output'
|
||||
import { sanitizeCrashReportString } from '../../shared/crash-reporting'
|
||||
import {
|
||||
excerptAgentFailureOutput,
|
||||
sanitizeAgentFailureDetail
|
||||
} from '../../shared/commit-message-agent-output'
|
||||
import { redactString } from '../observability/redactor'
|
||||
import { buildWslExecArgs, buildWslLoginShellCommand } from '../../shared/wsl-login-shell-command'
|
||||
import {
|
||||
@@ -66,8 +68,6 @@ const BACKEND_TIMEOUT_MS = 10_000
|
||||
// Why: redeeming a reset credit is an explicit user action, not a poll — allow more time for a slow backend.
|
||||
const REDEEM_BACKEND_TIMEOUT_MS = 30_000
|
||||
const MAX_DIAGNOSTIC_OUTPUT_LENGTH = 100_000
|
||||
// Why: one surfaced line, not a log dump — the full capture stays in-process.
|
||||
const MAX_EXIT_DETAIL_LENGTH = 200
|
||||
|
||||
export type FetchCodexRateLimitsOptions = {
|
||||
codexHomePath?: string | null
|
||||
@@ -881,23 +881,21 @@ async function fetchViaRpc(options?: FetchCodexRateLimitsOptions): Promise<Provi
|
||||
})
|
||||
}
|
||||
|
||||
// Why: this string is the whole diagnosis the user gets, and a hardcoded one
|
||||
// hid both the exit status and the child's own auth complaint — so an
|
||||
// app-server that quit on a dead refresh token never classified as
|
||||
// re-authenticable and never prompted sign-in (worst on Windows, which has no
|
||||
// PTY fallback to recover the reason).
|
||||
// Why: surfaced text drives re-auth classification, so diagnosis and classification must use the same sanitized value.
|
||||
function describeCodexRpcExit(
|
||||
code: number | null,
|
||||
signal: NodeJS.Signals | null,
|
||||
stderr: string
|
||||
): string {
|
||||
const authError = extractCodexAuthError(stderr)
|
||||
if (authError) {
|
||||
return redactCodexChildOutput(authError)
|
||||
if (extractCodexAuthError(stderr)) {
|
||||
// Fixed copy cannot leak paths/tokens and is exactly what the renderer classifies.
|
||||
return 'Your ChatGPT session could not be refreshed. Please sign in again.'
|
||||
}
|
||||
const reason =
|
||||
code !== null ? `exit code ${code}` : signal ? `signal ${signal}` : 'no exit status'
|
||||
const detail = firstReportedStderrLine(stderr)
|
||||
const detail = sanitizeAgentFailureDetail(
|
||||
redactString(excerptAgentFailureOutput('', stderr) ?? '')
|
||||
)
|
||||
return withMacTailscaleDnsHint(
|
||||
detail
|
||||
? `Codex RPC process exited (${reason}): ${detail}`
|
||||
@@ -906,28 +904,6 @@ function describeCodexRpcExit(
|
||||
)
|
||||
}
|
||||
|
||||
function firstReportedStderrLine(stderr: string): string | null {
|
||||
for (const rawLine of stripAnsiControlSequences(stderr).split(/\r\n|\n|\r/)) {
|
||||
const line = rawLine.trim()
|
||||
if (line) {
|
||||
return redactCodexChildOutput(line)
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
// Why: redactString kills token shapes without eating prose, while the
|
||||
// crash-report pass adds path scrubbing but is greedy enough to swallow the
|
||||
// rest of a line — so it is dropped when it would erase the auth phrase the
|
||||
// re-auth warning classifies on.
|
||||
function redactCodexChildOutput(text: string): string {
|
||||
const withoutSecrets = redactString(text)
|
||||
const withoutPaths = sanitizeCrashReportString(withoutSecrets, MAX_EXIT_DETAIL_LENGTH)
|
||||
return isCodexAuthError(withoutSecrets) && !isCodexAuthError(withoutPaths)
|
||||
? withoutSecrets.slice(0, MAX_EXIT_DETAIL_LENGTH)
|
||||
: withoutPaths
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// PTY fallback — spawn `codex`, send `/status`, parse rendered output
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -19,7 +19,8 @@ import {
|
||||
} from '../../shared/pull-request-generation'
|
||||
import {
|
||||
cleanGeneratedCommitMessage,
|
||||
excerptAgentFailureOutput
|
||||
excerptAgentFailureOutput,
|
||||
sanitizeAgentFailureDetail
|
||||
} from '../../shared/commit-message-prompt'
|
||||
import {
|
||||
captureAgentGenerationFailureOutput,
|
||||
@@ -196,40 +197,6 @@ function formatAgentCliFailureMessage(
|
||||
: withMacTailscaleDnsHint(message, detail)
|
||||
}
|
||||
|
||||
function sanitizeAgentFailureDetail(detail: string | null): string | null {
|
||||
// Cf covers bidi overrides (U+202E etc.) that could visually reorder the
|
||||
// persisted, client-synced detail.
|
||||
const trimmed = detail
|
||||
?.replace(/[\p{Cc}\p{Cf}]+/gu, ' ')
|
||||
.replace(/\s+/g, ' ')
|
||||
.trim()
|
||||
if (!trimmed) {
|
||||
return null
|
||||
}
|
||||
// Why: agent stderr often includes local or SSH repo paths. Persisting those
|
||||
// into worktree metadata leaks environment details into synced renderer state.
|
||||
const redacted = trimmed
|
||||
.replace(
|
||||
/\\\\[^\s"'`<>\\]+\\(?:[^\s"'`<>\\]+(?:\s+[^\s"'`<>\\]+)*(?=\\)\\)*[^\s"'`<>\\]+/g,
|
||||
'[path]'
|
||||
)
|
||||
// Only backslashes may repeat: JSON provider bodies double them
|
||||
// (`C:\\Users\\name\\…`), while a URL's `://` must stay single so remedy
|
||||
// links like `https://…` survive redaction.
|
||||
.replace(
|
||||
/[A-Za-z]:(?:\\+|\/)(?:[^\s"'`<>\\/|:*?]+(?:\s+[^\s"'`<>\\/|:*?]+)*(?=[\\/])(?:\\+|\/))*[^\s"'`<>\\/|:*?]+/g,
|
||||
'[path]'
|
||||
)
|
||||
// Why: require ≥2 segments (one internal `/`) so provider remedy tokens like
|
||||
// `/login` survive while multi-segment paths (`/Users/name/repo`) still redact.
|
||||
// `=:,` prefixes catch key=/path value:/path list,/path shapes in provider bodies.
|
||||
.replace(
|
||||
/(^|[\s"'`(=:,])\/(?:[^\s"'`<>/]+(?:\s+[^\s"'`<>/]+)*(?=\/)\/)+[^\s"'`<>/]+/g,
|
||||
'$1[path]'
|
||||
)
|
||||
return redacted.length > 240 ? `${redacted.slice(0, 240).trimEnd()}...` : redacted
|
||||
}
|
||||
|
||||
function userFacingUnsafeWindowsBatchArgs(label: string): string {
|
||||
return `${label} cannot be run as a Windows batch command with the prompt in argv. Remove {prompt} so Orca sends the prompt on stdin.`
|
||||
}
|
||||
|
||||
@@ -38,14 +38,10 @@ describe('codex account auth warning', () => {
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
// Strings produced verbatim by the RPC-exit path in
|
||||
// codex-fetcher-rpc-exit-diagnostics.test.ts.
|
||||
it('warns re-auth when the app-server quits over a dead refresh token', () => {
|
||||
expect(
|
||||
getCodexAccountAuthWarning({
|
||||
limits: codexLimits(
|
||||
'ERROR: The ChatGPT access token could not be refreshed; please sign in again.'
|
||||
),
|
||||
limits: codexLimits('Your ChatGPT session could not be refreshed. Please sign in again.'),
|
||||
target: { runtime: 'host', wslDistro: null },
|
||||
runtime: { runtime: 'host' },
|
||||
activeAccountId: 'account-1',
|
||||
|
||||
@@ -158,6 +158,33 @@ export function excerptAgentFailureOutput(stdout: string, stderr: string): strin
|
||||
return composeTwoEndExcerpt(headLines, tailLine)
|
||||
}
|
||||
|
||||
export function sanitizeAgentFailureDetail(detail: string | null): string | null {
|
||||
// Cf covers bidi overrides that could visually reorder persisted, client-synced text.
|
||||
const trimmed = detail
|
||||
?.replace(/[\p{Cc}\p{Cf}]+/gu, ' ')
|
||||
.replace(/\s+/g, ' ')
|
||||
.trim()
|
||||
if (!trimmed) {
|
||||
return null
|
||||
}
|
||||
const redacted = trimmed
|
||||
.replace(
|
||||
/\\\\[^\s"'`<>\\]+\\(?:[^\s"'`<>\\]+(?:\s+[^\s"'`<>\\]+)*(?=\\)\\)*[^\s"'`<>\\]+/g,
|
||||
'[path]'
|
||||
)
|
||||
// JSON may double Windows separators; URL separators must remain single.
|
||||
.replace(
|
||||
/[A-Za-z]:(?:\\+|\/)(?:[^\s"'`<>\\/|:*?]+(?:\s+[^\s"'`<>\\/|:*?]+)*(?=[\\/])(?:\\+|\/))*[^\s"'`<>\\/|:*?]+/g,
|
||||
'[path]'
|
||||
)
|
||||
// Keep single-segment remedies such as /login while redacting filesystem paths.
|
||||
.replace(
|
||||
/(^|[\s"'`(=:,])\/(?:[^\s"'`<>/]+(?:\s+[^\s"'`<>/]+)*(?=\/)\/)+[^\s"'`<>/]+/g,
|
||||
'$1[path]'
|
||||
)
|
||||
return redacted.length > 240 ? `${redacted.slice(0, 240).trimEnd()}...` : redacted
|
||||
}
|
||||
|
||||
function composeTwoEndExcerpt(headLines: string[], tailLine: string | null): string {
|
||||
const headPart = truncateExcerptPart(headLines.join(' '), FAILURE_EXCERPT_HEAD_BUDGET)
|
||||
// Repeated lines (spinner/retry frames) would otherwise show twice.
|
||||
|
||||
@@ -19,7 +19,8 @@ Staged diff:
|
||||
|
||||
export {
|
||||
cleanGeneratedCommitMessage,
|
||||
excerptAgentFailureOutput
|
||||
excerptAgentFailureOutput,
|
||||
sanitizeAgentFailureDetail
|
||||
} from './commit-message-agent-output'
|
||||
|
||||
/** Builds the final prompt sent to the agent. The custom suffix is appended verbatim
|
||||
|
||||
Reference in New Issue
Block a user