Skip to content

[#758] fix-codex-reconnect-retry #767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nrslib merged 1 commit into from
May 29, 2026
Merged

[#758] fix-codex-reconnect-retry #767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
339 changes: 339 additions & 0 deletions src/__tests__/codex-client-retry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ let runPlanIndex = 0;
let startThreadCalls: Array<Record<string, unknown> | undefined> = [];
let resumeThreadCalls: Array<{ threadId: string; options?: Record<string, unknown> }> = [];
const CODEX_STREAM_IDLE_TIMEOUT_MS = 10 * 60 * 1000;
const CODEX_RECONNECT_FAILURE_MESSAGE = 'Reconnecting... 2/5 (timeout waiting for child process to exit)';
const CODEX_RECONNECT_RETRYABLE_MESSAGES = [
'Reconnecting... 2/5',
'timeout waiting for child process to exit',
CODEX_RECONNECT_FAILURE_MESSAGE,
];

function createEvents(events: MockEvent[]) {
return (async function* () {
Expand Down Expand Up @@ -47,6 +53,21 @@ function createIdleTimeoutPlan(onThreadStarted?: () => void): RunPlan {
};
}

function createReconnectCommandFailureEvents(message: string, command: string): MockEvent[] {
return [
{ type: 'thread.started', thread_id: 'thread-1' },
{
type: 'item.started',
item: {
id: 'cmd-e2e',
type: 'command_execution',
command,
},
},
{ type: 'turn.failed', error: { message } },
];
}

function createThread(id: string) {
return {
id,
Expand Down Expand Up @@ -96,6 +117,7 @@ describe('CodexClient retry', () => {
});

afterEach(() => {
vi.clearAllTimers();
vi.useRealTimers();
});

Expand Down Expand Up @@ -255,6 +277,323 @@ describe('CodexClient retry', () => {
expect(result.content).toBe('Selected model is at capacity. Please try a different model.');
});

it.each(CODEX_RECONNECT_RETRYABLE_MESSAGES)(
'turn.failed の %s provider_error を 1 秒後に retry して成功を返す',
async (reconnectMessage) => {
vi.useFakeTimers();

runPlans = [
{
type: 'events',
events: [
{ type: 'thread.started', thread_id: 'thread-1' },
{ type: 'turn.failed', error: { message: reconnectMessage } },
],
},
{
type: 'events',
events: [
{ type: 'thread.started', thread_id: 'thread-1' },
{ type: 'item.completed', item: { id: 'msg-reconnect', type: 'agent_message', text: 'reconnect retry succeeded' } },
{ type: 'turn.completed', usage: { input_tokens: 1, output_tokens: 2 } },
],
},
];

const client = new CodexClient();

const resultPromise = client.call('coder', 'prompt', { cwd: '/tmp' });

await vi.advanceTimersByTimeAsync(999);
expect(resumeThreadCalls).toHaveLength(0);

await vi.advanceTimersByTimeAsync(1);
const result = await resultPromise;

expect(startThreadCalls).toHaveLength(1);
expect(resumeThreadCalls).toEqual([
{
threadId: 'thread-1',
options: expect.objectContaining({ workingDirectory: '/tmp' }),
},
]);
expect(result.status).toBe('done');
expect(result.content).toBe('reconnect retry succeeded');
},
);

it('stream error event の Reconnecting provider_error を 1 秒後に retry して成功を返す', async () => {
vi.useFakeTimers();

runPlans = [
{
type: 'events',
events: [
{ type: 'thread.started', thread_id: 'thread-1' },
{ type: 'error', message: CODEX_RECONNECT_FAILURE_MESSAGE },
],
},
{
type: 'events',
events: [
{ type: 'thread.started', thread_id: 'thread-1' },
{ type: 'item.completed', item: { id: 'msg-reconnect-event-error', type: 'agent_message', text: 'stream error retry succeeded' } },
{ type: 'turn.completed', usage: { input_tokens: 1, output_tokens: 2 } },
],
},
];

const client = new CodexClient();

const resultPromise = client.call('coder', 'prompt', { cwd: '/tmp' });

await vi.advanceTimersByTimeAsync(999);
expect(resumeThreadCalls).toHaveLength(0);

await vi.advanceTimersByTimeAsync(1);
const result = await resultPromise;

expect(startThreadCalls).toHaveLength(1);
expect(resumeThreadCalls).toEqual([
{
threadId: 'thread-1',
options: expect.objectContaining({ workingDirectory: '/tmp' }),
},
]);
expect(result.status).toBe('done');
expect(result.content).toBe('stream error retry succeeded');
});

it.each(CODEX_RECONNECT_RETRYABLE_MESSAGES)(
'例外経路の %s provider_error を 1 秒後に retry して成功を返す',
async (reconnectMessage) => {
vi.useFakeTimers();

runPlans = [
{ type: 'throw', error: new Error(reconnectMessage) },
{
type: 'events',
events: [
{ type: 'thread.started', thread_id: 'thread-1' },
{ type: 'item.completed', item: { id: 'msg-reconnect-exception', type: 'agent_message', text: 'exception retry succeeded' } },
{ type: 'turn.completed', usage: { input_tokens: 2, output_tokens: 3 } },
],
},
];

const client = new CodexClient();

const resultPromise = client.call('coder', 'prompt', { cwd: '/tmp' });

await vi.advanceTimersByTimeAsync(999);
expect(resumeThreadCalls).toHaveLength(0);

await vi.advanceTimersByTimeAsync(1);
const result = await resultPromise;

expect(startThreadCalls).toHaveLength(1);
expect(resumeThreadCalls).toEqual([
{
threadId: 'thread-1',
options: expect.objectContaining({ workingDirectory: '/tmp' }),
},
]);
expect(result.status).toBe('done');
expect(result.content).toBe('exception retry succeeded');
},
);

it('Reconnecting 系 provider_error の retry を使い切った場合は実行中 command の結果不明を診断する', async () => {
vi.useFakeTimers();

runPlans = Array.from({ length: 9 }, () => ({
type: 'events' as const,
events: [
...createReconnectCommandFailureEvents(
CODEX_RECONNECT_FAILURE_MESSAGE,
'npm run test:e2e:mock',
),
],
}));

const client = new CodexClient();
const onStream = vi.fn();
const resultPromise = client.call('coder', 'prompt', { cwd: '/tmp', onStream });
await vi.runAllTimersAsync();
const result = await resultPromise;

expect(startThreadCalls).toHaveLength(1);
expect(resumeThreadCalls).toHaveLength(8);
expect(result.status).toBe('error');
expect(result.failureCategory).toBe('provider_error');
expect(result.content).toContain('provider reconnect failure');
expect(result.content).toContain(CODEX_RECONNECT_FAILURE_MESSAGE);
expect(result.content).toContain('Active tool: Bash');
expect(result.content).toContain('Bash command: npm run test:e2e:mock');
expect(result.content).toContain('Command result: unknown');
Comment on lines +429 to +433

@coderabbitai coderabbitai Bot May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

診断契約は部分一致ではなく契約文言そのものを検証してください

現在の assertion は断片的な toContain の組み合わせで、最終契約文言の退行を取りこぼします。provider reconnect failure / command result unknown の完全フレーズを直接検証してください。

💡 Proposed fix 
     expect(result.content).toContain('provider reconnect failure');
+    expect(result.content).toContain('provider reconnect failure / command result unknown');
@@
     expect(result.content).toContain('provider reconnect failure');
+    expect(result.content).toContain('provider reconnect failure / command result unknown');

As per coding guidelines, "Confirm tests validate the post-fix behavior (not merely the presence of strings): unit tests should cover both normal and exception paths and assert the required final diagnostic semantics."

Also applies to: 472-476

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/codex-client-retry.test.ts` around lines 429 - 433, The test
currently asserts fragments using multiple expect(...).toContain calls which can
miss regressions; instead build the full expected diagnostic phrase (e.g.,
combine CODEX_RECONNECT_FAILURE_MESSAGE with the provider/tool/command/result
text into one string) and assert against result.content with a single precise
check (e.g., const expected = `${CODEX_RECONNECT_FAILURE_MESSAGE} - provider
reconnect failure; Active tool: Bash; Bash command: npm run test:e2e:mock;
Command result: unknown` and then expect(result.content).toContain(expected) or
expect(result.content).toMatch(expected) / toEqual(expected) depending on exact
formatting), and apply the same change for the similar assertions around lines
472-476 to validate the entire contract phrase rather than partial fragments.

expect(onStream).toHaveBeenCalledWith({
type: 'result',
data: expect.objectContaining({
success: false,
error: expect.stringContaining('provider reconnect failure'),
failureCategory: 'provider_error',
}),
});
});

it('例外経路の Reconnecting 系 provider_error の retry を使い切った場合も実行中 command の結果不明を診断する', async () => {
vi.useFakeTimers();

runPlans = Array.from({ length: 9 }, () => ({
type: 'stream' as const,
createEvents: async function* () {
yield { type: 'thread.started', thread_id: 'thread-1' };
yield {
type: 'item.started',
item: {
id: 'cmd-exception',
type: 'command_execution',
command: 'npm run test:e2e:mock',
},
};
throw new Error(CODEX_RECONNECT_FAILURE_MESSAGE);
},
}));

const client = new CodexClient();
const resultPromise = client.call('coder', 'prompt', { cwd: '/tmp' });
await vi.runAllTimersAsync();
const result = await resultPromise;

expect(startThreadCalls).toHaveLength(1);
expect(resumeThreadCalls).toHaveLength(8);
expect(result.status).toBe('error');
expect(result.failureCategory).toBe('provider_error');
expect(result.content).toContain('provider reconnect failure');
expect(result.content).toContain(CODEX_RECONNECT_FAILURE_MESSAGE);
expect(result.content).toContain('Active tool: Bash');
expect(result.content).toContain('Bash command: npm run test:e2e:mock');
expect(result.content).toContain('Command result: unknown');
});

it('Reconnecting 系 provider_error の command 診断は機密値をマスクする', async () => {
vi.useFakeTimers();

const secretCommand = [
'curl -H "Authorization: Bearer sk-abcdefghijklmnopqrstuvwxyz"',
'curl -H "Authorization: Basic dXNlcjpwYXNz"',
'curl -H "Authorization: Bearer leading-space-token"',
'curl -H "Cookie: sessionid=plain-session-id; theme=dark"',
'curl -H "Set-Cookie: sessionid=set-cookie-secret; Path=/"',
'curl -u user:plain-password',
'curl -uuser:compact-password',
'curl --user other:other-password',
'curl --user=third:third-password',
'curl --proxy-user proxy:proxy-password',
'curl --proxy-user=proxy-eq:proxy-eq-password',
'curl https://url-user:url-password@example.test/path',
'https://example.test?api_key=query-secret',
'--token ghp_abcdefghijklmnopqrstuvwxyz1234567890',
'OPENAI_API_KEY=sk-proj-secret_1234567890',
"PASSWORD='correct horse battery staple'",
"AWS_SECRET_ACCESS_KEY='aws secret phrase with spaces'",
'SERVICE_PRIVATE_KEY="private key phrase with spaces"',
'--aws-access-key-id access-key-secret',
'PASSWORD="abc\\" double assignment leaked tail"',
"SECRET='abc\\' single assignment leaked tail'",
'--token "abc\\" double option leaked tail"',
"--private-key 'abc\\' single option leaked tail'",
].join(' ');

runPlans = Array.from({ length: 9 }, () => ({
type: 'events' as const,
events: [
{ type: 'thread.started', thread_id: 'thread-1' },
{
type: 'item.started',
item: {
id: 'cmd-secret',
type: 'command_execution',
command: secretCommand,
},
},
{ type: 'turn.failed', error: { message: CODEX_RECONNECT_FAILURE_MESSAGE } },
],
}));

const client = new CodexClient();
const resultPromise = client.call('coder', 'prompt', { cwd: '/tmp' });
await vi.runAllTimersAsync();
const result = await resultPromise;

expect(result.status).toBe('error');
expect(result.content).toContain('Bash command:');
expect(result.content).toContain('Authorization: Bearer [REDACTED]');
expect(result.content).toContain('Authorization: Basic [REDACTED]');
expect(result.content).toContain('Authorization: Bearer [REDACTED]');
expect(result.content).toContain('Cookie: [REDACTED]');
expect(result.content).toContain('Set-Cookie: [REDACTED]');
expect(result.content).toContain('-u [REDACTED]');
expect(result.content).toContain('-u[REDACTED]');
expect(result.content).toContain('--user [REDACTED]');
expect(result.content).toContain('--user=[REDACTED]');
expect(result.content).toContain('--proxy-user [REDACTED]');
expect(result.content).toContain('--proxy-user=[REDACTED]');
expect(result.content).toContain('https://[REDACTED]@example.test/path');
expect(result.content).toContain('api_key=[REDACTED]');
expect(result.content).toContain('--token [REDACTED]');
expect(result.content).toContain('OPENAI_API_KEY=[REDACTED]');
expect(result.content).toContain("PASSWORD='[REDACTED]'");
expect(result.content).toContain("AWS_SECRET_ACCESS_KEY='[REDACTED]'");
expect(result.content).toContain('SERVICE_PRIVATE_KEY="[REDACTED]"');
expect(result.content).toContain('--aws-access-key-id [REDACTED]');
expect(result.content).toContain('PASSWORD="[REDACTED]"');
expect(result.content).toContain("SECRET='[REDACTED]'");
expect(result.content).toContain('--private-key [REDACTED]');
expect(result.content).not.toContain('sk-abcdefghijklmnopqrstuvwxyz');
expect(result.content).not.toContain('dXNlcjpwYXNz');
expect(result.content).not.toContain('leading-space-token');
expect(result.content).not.toContain('plain-session-id');
expect(result.content).not.toContain('set-cookie-secret');
expect(result.content).not.toContain('plain-password');
expect(result.content).not.toContain('compact-password');
expect(result.content).not.toContain('other-password');
expect(result.content).not.toContain('third-password');
expect(result.content).not.toContain('proxy-password');
expect(result.content).not.toContain('proxy-eq-password');
expect(result.content).not.toContain('url-password');
expect(result.content).not.toContain('query-secret');
expect(result.content).not.toContain('ghp_abcdefghijklmnopqrstuvwxyz1234567890');
expect(result.content).not.toContain('sk-proj-secret_1234567890');
expect(result.content).not.toContain('correct horse battery staple');
expect(result.content).not.toContain('aws secret phrase with spaces');
expect(result.content).not.toContain('private key phrase with spaces');
expect(result.content).not.toContain('access-key-secret');
expect(result.content).not.toContain('double assignment leaked tail');
expect(result.content).not.toContain('single assignment leaked tail');
expect(result.content).not.toContain('double option leaked tail');
expect(result.content).not.toContain('single option leaked tail');
expect(result.error).not.toContain('correct horse battery staple');
expect(result.error).not.toContain('dXNlcjpwYXNz');
expect(result.error).not.toContain('leading-space-token');
expect(result.error).not.toContain('plain-session-id');
expect(result.error).not.toContain('set-cookie-secret');
expect(result.error).not.toContain('plain-password');
expect(result.error).not.toContain('compact-password');
expect(result.error).not.toContain('other-password');
expect(result.error).not.toContain('third-password');
expect(result.error).not.toContain('proxy-password');
expect(result.error).not.toContain('proxy-eq-password');
expect(result.error).not.toContain('url-password');
expect(result.error).not.toContain('aws secret phrase with spaces');
expect(result.error).not.toContain('private key phrase with spaces');
expect(result.error).not.toContain('access-key-secret');
expect(result.error).not.toContain('double assignment leaked tail');
expect(result.error).not.toContain('single assignment leaked tail');
expect(result.error).not.toContain('double option leaked tail');
expect(result.error).not.toContain('single option leaked tail');
});

it('ストリームの idle timeout を 1 回 retry して成功を返す', async () => {
vi.useFakeTimers();

Expand Down
Loading
Loading