fix(codex): preserve native safety behavior (#1024)

* test: reproduce issue #1020 * fix: preserve Codex safety behavior (closes #1020) * test: cover Codex safety buffering dismissal * test: cover Codex safety retry edge cases * fix: keep dismissed safety buffering prompts hidden
2026-11-08 01:43:07 +00:00 · 2026-07-12 18:39:56 +08:00
parent d160203bb2
commit d9ea9db507
8 changed files with 1166 additions and 14 deletions
@@ -72,7 +72,12 @@ describe('AppServerEventConverter', () => {
        expect(interrupted).toEqual([{ type: 'turn_aborted', turn_id: 'turn-1' }]);

        const failed = converter.handleNotification('turn/completed', { turn: { id: 'turn-1' }, status: 'Failed', message: 'boom' });
-        expect(failed).toEqual([{ type: 'task_failed', turn_id: 'turn-1', error: 'boom' }]);
+        expect(failed).toEqual([{
+            type: 'task_failed',
+            turn_id: 'turn-1',
+            terminal_source: 'turn_completed',
+            error: 'boom'
+        }]);
    });

    it('accumulates agent message deltas', () => {
@@ -693,6 +698,120 @@ describe('AppServerEventConverter', () => {
        expect(events).toEqual([{ type: 'task_failed', error: 'fatal' }]);
    });

+    it('preserves typed non-retryable cyber-policy errors', () => {
+        const converter = new AppServerEventConverter();
+
+        const events = converter.handleNotification('error', {
+            threadId: 'thread-1',
+            turnId: 'turn-1',
+            error: {
+                message: 'This content was flagged for possible cybersecurity risk.',
+                codexErrorInfo: 'cyberPolicy'
+            },
+            willRetry: false
+        });
+
+        expect(events).toEqual([{
+            type: 'task_failed',
+            thread_id: 'thread-1',
+            turn_id: 'turn-1',
+            terminal_source: 'error',
+            retryable: false,
+            codex_error_info: 'cyberPolicy',
+            error: 'This content was flagged for possible cybersecurity risk.'
+        }]);
+    });
+
+    it('preserves cyber-policy metadata from wrapped and completed-turn errors', () => {
+        const converter = new AppServerEventConverter();
+
+        expect(converter.handleNotification('codex/event/error', {
+            msg: {
+                type: 'error',
+                thread_id: 'thread-1',
+                turn_id: 'turn-1',
+                message: 'wrapped policy failure',
+                codex_error_info: 'cyber_policy',
+                will_retry: false
+            }
+        })).toEqual([{
+            type: 'task_failed',
+            thread_id: 'thread-1',
+            turn_id: 'turn-1',
+            retryable: false,
+            codex_error_info: 'cyber_policy',
+            error: 'wrapped policy failure'
+        }]);
+
+        expect(converter.handleNotification('turn/completed', {
+            threadId: 'thread-1',
+            turn: {
+                id: 'turn-1',
+                status: 'failed',
+                error: {
+                    message: 'completed policy failure',
+                    codexErrorInfo: 'CyberPolicy'
+                }
+            }
+        })).toEqual([{
+            type: 'task_failed',
+            thread_id: 'thread-1',
+            turn_id: 'turn-1',
+            terminal_source: 'turn_completed',
+            codex_error_info: 'CyberPolicy',
+            error: 'completed policy failure'
+        }]);
+    });
+
+    it('maps Codex model safety notifications', () => {
+        const converter = new AppServerEventConverter();
+
+        expect(converter.handleNotification('model/safetyBuffering/updated', {
+            threadId: 'thread-1',
+            turnId: 'turn-1',
+            model: 'gpt-5.4',
+            useCases: ['cyber'],
+            reasons: ['review'],
+            showBufferingUi: true,
+            fasterModel: 'gpt-5.4-mini'
+        })).toEqual([{
+            type: 'model_safety_buffering',
+            thread_id: 'thread-1',
+            turn_id: 'turn-1',
+            model: 'gpt-5.4',
+            use_cases: ['cyber'],
+            reasons: ['review'],
+            show_buffering_ui: true,
+            faster_model: 'gpt-5.4-mini'
+        }]);
+
+        expect(converter.handleNotification('model/rerouted', {
+            threadId: 'thread-1',
+            turnId: 'turn-1',
+            fromModel: 'gpt-5.4',
+            toModel: 'gpt-5.4-codex',
+            reason: 'highRiskCyberActivity'
+        })).toEqual([{
+            type: 'model_rerouted',
+            thread_id: 'thread-1',
+            turn_id: 'turn-1',
+            from_model: 'gpt-5.4',
+            to_model: 'gpt-5.4-codex',
+            reason: 'highRiskCyberActivity'
+        }]);
+
+        expect(converter.handleNotification('model/verification', {
+            threadId: 'thread-1',
+            turnId: 'turn-1',
+            verifications: ['trustedAccessForCyber']
+        })).toEqual([{
+            type: 'model_verification',
+            thread_id: 'thread-1',
+            turn_id: 'turn-1',
+            verifications: ['trustedAccessForCyber']
+        }]);
+    });
+
    it('maps thread/compacted notifications', () => {
        const converter = new AppServerEventConverter();
        const events = converter.handleNotification('thread/compacted', {
@@ -306,6 +306,18 @@ function extractStringArray(value: unknown): string[] {
        : [];
 }

+function extractCodexErrorInfo(
+    record: Record<string, unknown>,
+    errorRecord: Record<string, unknown> | null
+): string | null {
+    return asString(
+        record.codexErrorInfo
+        ?? record.codex_error_info
+        ?? errorRecord?.codexErrorInfo
+        ?? errorRecord?.codex_error_info
+    );
+}
+
 function buildCollabAgentInput(item: Record<string, unknown>, toolName: string): Record<string, unknown> {
    const targets = extractStringArray(item.receiverThreadIds ?? item.receiver_thread_ids ?? item.targets);
    const input: Record<string, unknown> = {};
@@ -526,12 +538,19 @@ export class AppServerEventConverter {

        if (msgType === 'error') {
            const errorRecord = asRecord(msg.error);
-            const willRetry = asBoolean(msg.will_retry ?? msg.willRetry ?? errorRecord?.will_retry ?? errorRecord?.willRetry) ?? false;
+            const retryable = asBoolean(msg.will_retry ?? msg.willRetry ?? errorRecord?.will_retry ?? errorRecord?.willRetry);
+            const willRetry = retryable ?? false;
            if (willRetry) {
                return [];
            }
            const error = asString(msg.message ?? msg.reason ?? errorRecord?.message);
-            return error ? addEventScope([{ type: 'task_failed', error }], msgScope) : [];
+            const codexErrorInfo = extractCodexErrorInfo(msg, errorRecord);
+            return error ? addEventScope([{
+                type: 'task_failed',
+                ...(retryable !== null ? { retryable } : {}),
+                ...(codexErrorInfo ? { codex_error_info: codexErrorInfo } : {}),
+                error
+            }], msgScope) : [];
        }

        if (msgType === 'plan_update') {
@@ -672,7 +691,9 @@ export class AppServerEventConverter {
            const statusRaw = asString(paramsRecord.status ?? turn.status);
            const status = statusRaw?.toLowerCase();
            const turnId = asString(turn.turnId ?? turn.turn_id ?? turn.id);
-            const errorMessage = asString(paramsRecord.error ?? paramsRecord.message ?? paramsRecord.reason);
+            const turnError = asRecord(paramsRecord.error ?? turn.error);
+            const errorMessage = asString(paramsRecord.error ?? paramsRecord.message ?? paramsRecord.reason)
+                ?? asString(turnError?.message);

            if (status === 'interrupted' || status === 'cancelled' || status === 'canceled') {
                events.push(scoped({ type: 'turn_aborted', ...(turnId ? { turn_id: turnId } : {}) }));
@@ -680,7 +701,14 @@ export class AppServerEventConverter {
            }

            if (status === 'failed' || status === 'error') {
-                events.push(scoped({ type: 'task_failed', ...(turnId ? { turn_id: turnId } : {}), ...(errorMessage ? { error: errorMessage } : {}) }));
+                const codexErrorInfo = extractCodexErrorInfo(paramsRecord, turnError);
+                events.push(scoped({
+                    type: 'task_failed',
+                    ...(turnId ? { turn_id: turnId } : {}),
+                    terminal_source: 'turn_completed',
+                    ...(codexErrorInfo ? { codex_error_info: codexErrorInfo } : {}),
+                    ...(errorMessage ? { error: errorMessage } : {})
+                }));
                return events;
            }

@@ -702,12 +730,66 @@ export class AppServerEventConverter {
            return events;
        }

+        if (method === 'model/safetyBuffering/updated') {
+            const model = asString(paramsRecord.model);
+            const showBufferingUi = asBoolean(paramsRecord.showBufferingUi ?? paramsRecord.show_buffering_ui);
+            if (!model || showBufferingUi === null) {
+                return events;
+            }
+            events.push(scoped({
+                type: 'model_safety_buffering',
+                model,
+                use_cases: extractStringArray(paramsRecord.useCases ?? paramsRecord.use_cases),
+                reasons: extractStringArray(paramsRecord.reasons),
+                show_buffering_ui: showBufferingUi,
+                faster_model: asString(paramsRecord.fasterModel ?? paramsRecord.faster_model)
+            }));
+            return events;
+        }
+
+        if (method === 'model/rerouted') {
+            const fromModel = asString(paramsRecord.fromModel ?? paramsRecord.from_model);
+            const toModel = asString(paramsRecord.toModel ?? paramsRecord.to_model);
+            const reason = asString(paramsRecord.reason);
+            if (fromModel && toModel && reason) {
+                events.push(scoped({
+                    type: 'model_rerouted',
+                    from_model: fromModel,
+                    to_model: toModel,
+                    reason
+                }));
+            }
+            return events;
+        }
+
+        if (method === 'model/verification') {
+            events.push(scoped({
+                type: 'model_verification',
+                verifications: extractStringArray(paramsRecord.verifications)
+            }));
+            return events;
+        }
+
        if (method === 'error') {
-            const willRetry = asBoolean(paramsRecord.will_retry ?? paramsRecord.willRetry) ?? false;
+            const errorRecord = asRecord(paramsRecord.error);
+            const retryable = asBoolean(
+                paramsRecord.will_retry
+                ?? paramsRecord.willRetry
+                ?? errorRecord?.will_retry
+                ?? errorRecord?.willRetry
+            );
+            const willRetry = retryable ?? false;
            if (willRetry) return events;
-            const message = asString(paramsRecord.message) ?? asString(asRecord(paramsRecord.error)?.message);
+            const message = asString(paramsRecord.message) ?? asString(errorRecord?.message);
            if (message) {
-                events.push(scoped({ type: 'task_failed', error: message }));
+                const codexErrorInfo = extractCodexErrorInfo(paramsRecord, errorRecord);
+                events.push(scoped({
+                    type: 'task_failed',
+                    terminal_source: 'error',
+                    ...(retryable !== null ? { retryable } : {}),
+                    ...(codexErrorInfo ? { codex_error_info: codexErrorInfo } : {}),
+                    error: message
+                }));
            }
            return events;
        }
@@ -723,6 +805,7 @@ export class AppServerEventConverter {
                this.lastAgentMessageDeltaByItemId.set(itemId, delta);
                const prev = this.agentMessageBuffers.get(itemId) ?? '';
                this.agentMessageBuffers.set(itemId, prev + delta);
+                events.push(scoped({ type: 'agent_message_delta' }));
            }
            return events;
        }
@@ -172,4 +172,31 @@ describe('CodexPermissionHandler', () => {
            }
        });
    });
+
+    it('cancels one request_user_input without resetting other pending requests', async () => {
+        const { handler, getAgentState } = createHarness('default');
+        const first = handler.handleUserInputRequest('input-1', {
+            questions: [{ id: 'action', question: 'First?' }]
+        });
+        const second = handler.handleUserInputRequest('input-2', {
+            questions: [{ id: 'action', question: 'Second?' }]
+        });
+
+        handler.cancelUserInputRequest('input-1', 'No longer relevant');
+
+        await expect(first).rejects.toThrow('No longer relevant');
+        expect(getAgentState().requests).toMatchObject({
+            'input-2': { tool: 'request_user_input' }
+        });
+        expect(getAgentState().requests).not.toHaveProperty('input-1');
+        expect(getAgentState().completedRequests).toMatchObject({
+            'input-1': {
+                status: 'canceled',
+                reason: 'No longer relevant'
+            }
+        });
+
+        handler.reset();
+        await expect(second).rejects.toThrow('Session reset');
+    });
 });
@@ -155,6 +155,22 @@ export class CodexPermissionHandler extends BasePermissionHandler<PermissionResp
        });
    }

+    cancelUserInputRequest(toolCallId: string, reason: string): void {
+        const pending = this.pendingRequests.get(toolCallId);
+        if (!pending || pending.toolName !== 'request_user_input') {
+            return;
+        }
+
+        this.pendingRequests.delete(toolCallId);
+        pending.reject(new Error(reason));
+        this.finalizeRequest(toolCallId, {
+            status: 'canceled',
+            reason,
+            decision: 'abort'
+        });
+        logger.debug(`[Codex] User-input request canceled (${toolCallId}): ${reason}`);
+    }
+
    /**
     * Handle permission responses
     */