From 13b91c22b7ad049f9bf851849a318333a01febe1 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 23 Aug 2026 13:15:47 -0700 Subject: [PATCH] fix(rate-limits): safely surface Codex RPC exits --- ...codex-fetcher-rpc-exit-diagnostics.test.ts | 20 +++++--- src/main/rate-limits/codex-fetcher.ts | 46 +++++-------------- .../commit-message-text-generation.ts | 37 +-------------- .../codex-account-auth-warning.test.ts | 6 +-- src/shared/commit-message-agent-output.ts | 27 +++++++++++ src/shared/commit-message-prompt.ts | 3 +- 6 files changed, 57 insertions(+), 82 deletions(-) diff --git a/src/main/rate-limits/codex-fetcher-rpc-exit-diagnostics.test.ts b/src/main/rate-limits/codex-fetcher-rpc-exit-diagnostics.test.ts index dfaef2ae761..ee874ef5942 100644 --- a/src/main/rate-limits/codex-fetcher-rpc-exit-diagnostics.test.ts +++ b/src/main/rate-limits/codex-fetcher-rpc-exit-diagnostics.test.ts @@ -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 '" - ) + 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') + }) }) diff --git a/src/main/rate-limits/codex-fetcher.ts b/src/main/rate-limits/codex-fetcher.ts index f87ed986a6e..dc18e715fdd 100644 --- a/src/main/rate-limits/codex-fetcher.ts +++ b/src/main/rate-limits/codex-fetcher.ts @@ -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\\]+\\(?:[^\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.` } diff --git a/src/renderer/src/components/settings/codex-account-auth-warning.test.ts b/src/renderer/src/components/settings/codex-account-auth-warning.test.ts index d5892a6061e..d0062884c4c 100644 --- a/src/renderer/src/components/settings/codex-account-auth-warning.test.ts +++ b/src/renderer/src/components/settings/codex-account-auth-warning.test.ts @@ -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', diff --git a/src/shared/commit-message-agent-output.ts b/src/shared/commit-message-agent-output.ts index cbda4a42bde..27c0d201a7d 100644 --- a/src/shared/commit-message-agent-output.ts +++ b/src/shared/commit-message-agent-output.ts @@ -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. diff --git a/src/shared/commit-message-prompt.ts b/src/shared/commit-message-prompt.ts index 164f4da6327..7fb9da76d66 100644 --- a/src/shared/commit-message-prompt.ts +++ b/src/shared/commit-message-prompt.ts @@ -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