Skip to content

fix:Codex app server approval and turn completion fallbacks #921

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

Open
zew1me wants to merge 1 commit into
base: main
Choose a base branch
from
+242 −4
Open

fix:Codex app server approval and turn completion fallbacks #921

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
229 changes: 229 additions & 0 deletions packages/happy-cli/src/codex/codexAppServerClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type MockRpcMessage = {
id?: number;
method?: string;
params?: any;
result?: any;
};

function pushJsonLine(stdout: NodeJS.ReadableStream & { push: (chunk: string) => void }, payload: unknown) {
Expand Down Expand Up @@ -709,6 +710,161 @@ describe('CodexAppServerClient sandbox integration', () => {
await client.disconnect();
});

it('uses itemId for command approvals and maps approved_for_session to v2 wire responses', async () => {
const approvals: Array<Record<string, unknown>> = [];
const requests: MockRpcMessage[] = [];
const proc = createMockProcess({
pid: 3005,
onRequest: (msg, stdout) => {
requests.push(msg);
if (msg.method === 'thread/start' && msg.id != null) {
setTimeout(() => {
pushJsonLine(stdout, {
id: msg.id,
result: {
thread: { id: 'thread-raw-5', path: '/tmp/thread-raw-5' },
model: 'gpt-test',
modelProvider: 'openai',
cwd: '/tmp/project',
approvalPolicy: 'on-request',
sandbox: { type: 'workspaceWrite', writableRoots: [], networkAccess: true, excludeTmpdirEnvVar: false, excludeSlashTmp: false },
reasoningEffort: null,
},
});
pushJsonLine(stdout, {
id: 99,
method: 'item/commandExecution/requestApproval',
params: {
threadId: 'thread-raw-5',
turnId: 'turn-raw-5',
itemId: 'exec-approval-1',
command: 'gh --version',
cwd: '/tmp/project',
reason: null,
},
});
}, 0);
}
},
});

mockSpawn.mockImplementation(() => proc);

const { CodexAppServerClient } = await import('./codexAppServerClient');
const client = new CodexAppServerClient();
client.setApprovalHandler(async (params) => {
approvals.push(params as Record<string, unknown>);
return 'approved_for_session';
});

await client.connect();
await client.startThread({
model: 'gpt-test',
cwd: '/tmp/project',
approvalPolicy: 'on-request',
sandbox: 'workspace-write',
});

await waitFor(() => approvals.length === 1);
await waitFor(() => requests.some((msg) => msg.id === 99 && msg.result?.decision === 'acceptForSession'));

expect(approvals[0]).toEqual(expect.objectContaining({
type: 'exec',
callId: 'exec-approval-1',
command: ['gh --version'],
cwd: '/tmp/project',
reason: null,
}));
expect(requests).toEqual(expect.arrayContaining([
expect.objectContaining({
id: 99,
result: expect.objectContaining({
decision: 'acceptForSession',
}),
}),
]));

await client.disconnect();
});

it('falls back to legacy exec callId when itemId is unavailable', async () => {
const approvals: Array<Record<string, unknown>> = [];
const requests: MockRpcMessage[] = [];
const proc = createMockProcess({
pid: 3006,
onRequest: (msg, stdout) => {
requests.push(msg);
if (msg.method === 'thread/start' && msg.id != null) {
setTimeout(() => {
pushJsonLine(stdout, {
id: msg.id,
result: {
thread: { id: 'thread-raw-6', path: '/tmp/thread-raw-6' },
model: 'gpt-test',
modelProvider: 'openai',
cwd: '/tmp/project',
approvalPolicy: 'on-request',
sandbox: { type: 'workspaceWrite', writableRoots: [], networkAccess: true, excludeTmpdirEnvVar: false, excludeSlashTmp: false },
reasoningEffort: null,
},
});
pushJsonLine(stdout, {
id: 41,
method: 'execCommandApproval',
params: {
conversationId: 'thread-raw-6',
callId: 'legacy-exec-1',
approvalId: null,
command: 'git status',
cwd: '/tmp/project',
reason: 'needs approval',
parsedCmd: [],
},
});
}, 0);
}
},
});

mockSpawn.mockImplementation(() => proc);

const { CodexAppServerClient } = await import('./codexAppServerClient');
const client = new CodexAppServerClient();
client.setApprovalHandler(async (params) => {
approvals.push(params as Record<string, unknown>);
return 'approved_for_session';
});

await client.connect();
await client.startThread({
model: 'gpt-test',
cwd: '/tmp/project',
approvalPolicy: 'on-request',
sandbox: 'workspace-write',
});

await waitFor(() => approvals.length === 1);
await waitFor(() => requests.some((msg) => msg.id === 41 && msg.result?.decision === 'approved_for_session'));

expect(approvals[0]).toEqual(expect.objectContaining({
type: 'exec',
callId: 'legacy-exec-1',
command: ['git status'],
cwd: '/tmp/project',
reason: 'needs approval',
}));
expect(requests).toEqual(expect.arrayContaining([
expect.objectContaining({
id: 41,
result: expect.objectContaining({
decision: 'approved_for_session',
}),
}),
]));

await client.disconnect();
});

it('falls back to final answer completion when raw turn/completed is missing', async () => {
const proc = createMockProcess({
pid: 3002,
Expand Down Expand Up @@ -789,4 +945,77 @@ describe('CodexAppServerClient sandbox integration', () => {

await client.disconnect();
});

it('falls back to final answer completion when turn/started is also missing', async () => {
const proc = createMockProcess({
pid: 3003,
onRequest: (msg, stdout) => {
if (msg.method === 'thread/start' && msg.id != null) {
setTimeout(() => {
pushJsonLine(stdout, {
id: msg.id,
result: {
thread: { id: 'thread-raw-3', path: '/tmp/thread-raw-3' },
model: 'gpt-test',
modelProvider: 'openai',
cwd: '/tmp/project',
approvalPolicy: 'never',
sandbox: { type: 'dangerFullAccess' },
reasoningEffort: null,
},
});
}, 0);
}

if (msg.method === 'turn/start' && msg.id != null) {
setTimeout(() => {
pushJsonLine(stdout, {
id: msg.id,
result: {
turn: { id: 'turn-raw-3', items: [], status: 'inProgress', error: null },
},
});
pushJsonLine(stdout, {
method: 'item/completed',
params: {
threadId: 'thread-raw-3',
turnId: 'turn-raw-3',
item: {
type: 'agentMessage',
id: 'msg-3',
text: 'still completes',
phase: 'final_answer',
},
},
});
}, 0);
}
},
});

mockSpawn.mockImplementation(() => proc);

const { CodexAppServerClient } = await import('./codexAppServerClient');
const client = new CodexAppServerClient();
const events: Array<Record<string, unknown>> = [];
client.setEventHandler((msg) => {
events.push(msg as Record<string, unknown>);
});

await client.connect();
await client.startThread({
model: 'gpt-test',
cwd: '/tmp/project',
approvalPolicy: 'never',
sandbox: 'danger-full-access',
});

await expect(client.sendTurnAndWait('say hi again')).resolves.toEqual({ aborted: false });
expect(events).toEqual(expect.arrayContaining([
expect.objectContaining({ type: 'agent_message', message: 'still completes' }),
expect.objectContaining({ type: 'task_complete', turn_id: 'turn-raw-3' }),
]));

await client.disconnect();
});
});
15 changes: 12 additions & 3 deletions packages/happy-cli/src/codex/codexAppServerClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,17 @@ export class CodexAppServerClient {
});
}

if (item.phase === 'final_answer' && this.pendingTurnCompletion?.started) {
const turnId = this.extractTurnId(params) ?? this._turnId;
if (item.phase === 'final_answer' && this.pendingTurnCompletion) {
// Some app-server builds emit the final answer item but omit the
// corresponding task_started/turn/started lifecycle event. If we
// already know the turn id from turn/start, treat final_answer as
// sufficient evidence that the active turn completed.
if (turnId) {
this.markPendingTurnStarted(turnId);
}
this.emitRawTurnCompletion(
this.extractTurnId(params),
turnId,
'completed',
null,
`${method}:final_answer`,
Expand Down Expand Up @@ -1024,9 +1032,10 @@ export class CodexAppServerClient {
// Command execution approval
if (method === 'item/commandExecution/requestApproval' || method === 'execCommandApproval') {
const legacy = method === 'execCommandApproval';
const callId = params.itemId ?? params.callId ?? String(id);
const decision = await this.handleApproval({
type: 'exec',
callId: params.itemId ?? String(id),
callId,
command: params.command != null ? [params.command] : [],
cwd: params.cwd,
reason: params.reason,
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13533,7 +13533,7 @@ typed-emitter@^2.1.0:
optionalDependencies:
rxjs "^7.5.2"

typescript@5.9.3:
typescript@5.9.3, typescript@^5.9.3:
version "5.9.3"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.9.3.tgz#5b4f59e15310ab17a216f5d6cf53ee476ede670f"
integrity sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==
Expand Down