Skip to content

Commit d0fea22

Browse files
committed
review 3
1 parent 6ce6c0b commit d0fea22

File tree

6 files changed

+67
-104
lines changed

6 files changed

+67
-104
lines changed

cli/src/components/codex-connect-banner.tsx

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,33 @@ export const CodexConnectBanner = () => {
2828
const [isDisconnectHovered, setIsDisconnectHovered] = useState(false)
2929
const [isConnectHovered, setIsConnectHovered] = useState(false)
3030

31+
const onOAuthStatusChange = (callbackStatus: 'waiting' | 'success' | 'error', message?: string) => {
32+
if (callbackStatus === 'success') {
33+
setFlowState('connected')
34+
} else if (callbackStatus === 'error') {
35+
setError(message ?? 'Authorization failed')
36+
setFlowState('error')
37+
} else if (callbackStatus === 'waiting' && message) {
38+
setManualUrl(message)
39+
}
40+
}
41+
42+
const startFlow = () => {
43+
setFlowState('waiting-for-code')
44+
setManualUrl(null)
45+
startOAuthFlowWithCallback(onOAuthStatusChange).catch((err) => {
46+
setError(err instanceof Error ? err.message : 'Failed to start OAuth flow')
47+
setFlowState('error')
48+
})
49+
}
50+
3151
// Check initial connection status and auto-open browser if not connected
3252
useEffect(() => {
3353
const status = getCodexOAuthStatus()
3454
if (status.connected) {
3555
setFlowState('connected')
3656
} else {
37-
// Automatically start OAuth flow when not connected
38-
setFlowState('waiting-for-code')
39-
startOAuthFlowWithCallback((callbackStatus, message) => {
40-
if (callbackStatus === 'success') {
41-
setFlowState('connected')
42-
} else if (callbackStatus === 'error') {
43-
setError(message ?? 'Authorization failed')
44-
setFlowState('error')
45-
} else if (callbackStatus === 'waiting' && message) {
46-
setManualUrl(message)
47-
}
48-
}).catch((err) => {
49-
setError(err instanceof Error ? err.message : 'Failed to start OAuth flow')
50-
setFlowState('error')
51-
})
57+
startFlow()
5258
}
5359

5460
// Cleanup: stop the callback server when the component unmounts
@@ -57,27 +63,8 @@ export const CodexConnectBanner = () => {
5763
}
5864
}, [])
5965

60-
const handleConnect = async () => {
61-
try {
62-
setFlowState('waiting-for-code')
63-
setManualUrl(null)
64-
await startOAuthFlowWithCallback((callbackStatus, message) => {
65-
if (callbackStatus === 'success') {
66-
setFlowState('connected')
67-
} else if (callbackStatus === 'error') {
68-
setError(message ?? 'Authorization failed')
69-
setFlowState('error')
70-
} else if (callbackStatus === 'waiting' && message) {
71-
setManualUrl(message)
72-
}
73-
})
74-
} catch (err) {
75-
setError(err instanceof Error ? err.message : 'Failed to start OAuth flow')
76-
setFlowState('error')
77-
}
78-
}
79-
8066
const handleDisconnect = () => {
67+
stopCallbackServer()
8168
disconnectCodexOAuth()
8269
setFlowState('not-connected')
8370
}
@@ -159,7 +146,7 @@ export const CodexConnectBanner = () => {
159146
<text style={{ fg: theme.muted }}>Use your ChatGPT Plus/Pro subscription</text>
160147
<text style={{ fg: theme.muted }}>·</text>
161148
<Button
162-
onClick={handleConnect}
149+
onClick={startFlow}
163150
onMouseOver={() => setIsConnectHovered(true)}
164151
onMouseOut={() => setIsConnectHovered(false)}
165152
>

cli/src/utils/codex-oauth.ts

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import open from 'open'
88
import {
99
CODEX_OAUTH_CLIENT_ID,
1010
CODEX_OAUTH_AUTHORIZE_URL,
11+
CODEX_OAUTH_TOKEN_URL,
1112
CODEX_OAUTH_SCOPES,
1213
CODEX_OAUTH_REDIRECT_URI,
1314
} from '@codebuff/common/constants/codex-oauth'
@@ -22,8 +23,8 @@ import {
2223
import type { CodexOAuthCredentials } from '@codebuff/sdk'
2324
import type { Server } from 'http'
2425

25-
// Port for the local OAuth callback server
26-
const OAUTH_CALLBACK_PORT = 1455
26+
// Port for the local OAuth callback server (derived from redirect URI)
27+
const OAUTH_CALLBACK_PORT = Number(new URL(CODEX_OAUTH_REDIRECT_URI).port)
2728

2829
/**
2930
* Generate a nicely styled success HTML page for OAuth callback.
@@ -231,9 +232,8 @@ function generateState(): string {
231232
return crypto.randomBytes(16).toString('hex')
232233
}
233234

234-
// Store the code verifier and state during the OAuth flow
235+
// Store the code verifier during the OAuth flow
235236
let pendingCodeVerifier: string | null = null
236-
let pendingState: string | null = null
237237
let callbackServer: Server | null = null
238238

239239
/**
@@ -246,9 +246,8 @@ export function startOAuthFlow(): { codeVerifier: string; state: string; authUrl
246246
const codeChallenge = generateCodeChallenge(codeVerifier)
247247
const state = generateState()
248248

249-
// Store the code verifier and state for later use
249+
// Store the code verifier for later use
250250
pendingCodeVerifier = codeVerifier
251-
pendingState = state
252251

253252
// Build the authorization URL
254253
const authUrl = new URL(CODEX_OAUTH_AUTHORIZE_URL)
@@ -354,6 +353,7 @@ export function startOAuthFlowWithCallback(
354353
})
355354

356355
callbackServer.on('error', (err) => {
356+
stopCallbackServer()
357357
const nodeErr = err as NodeJS.ErrnoException
358358
if (nodeErr.code === 'EADDRINUSE') {
359359
onStatusChange?.('error', `Port ${OAUTH_CALLBACK_PORT} is already in use`)
@@ -377,16 +377,6 @@ export function startOAuthFlowWithCallback(
377377
})
378378
}
379379

380-
/**
381-
* Open the browser to start OAuth flow (legacy - for manual code entry).
382-
* @deprecated Use startOAuthFlowWithCallback instead for automatic callback handling.
383-
*/
384-
export async function openOAuthInBrowser(): Promise<string> {
385-
const { authUrl, codeVerifier } = startOAuthFlow()
386-
await open(authUrl)
387-
return codeVerifier
388-
}
389-
390380
/**
391381
* Exchange an authorization code for access and refresh tokens.
392382
*/
@@ -423,7 +413,7 @@ export async function exchangeCodeForTokens(
423413
redirect_uri: CODEX_OAUTH_REDIRECT_URI,
424414
})
425415

426-
const response = await fetch('https://auth.openai.com/oauth/token', {
416+
const response = await fetch(CODEX_OAUTH_TOKEN_URL, {
427417
method: 'POST',
428418
headers: {
429419
'Content-Type': 'application/x-www-form-urlencoded',
@@ -438,9 +428,8 @@ export async function exchangeCodeForTokens(
438428

439429
const data = await response.json()
440430

441-
// Clear the pending code verifier and state
431+
// Clear the pending code verifier
442432
pendingCodeVerifier = null
443-
pendingState = null
444433

445434
const credentials: CodexOAuthCredentials = {
446435
accessToken: data.access_token,

common/src/constants/codex-oauth.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@ export const CODEX_OAUTH_CLIENT_ID = 'app_EMoamEEZ73f0CkXaXp7hrann'
1111
export const CODEX_OAUTH_AUTHORIZE_URL = 'https://auth.openai.com/oauth/authorize'
1212
export const CODEX_OAUTH_TOKEN_URL = 'https://auth.openai.com/oauth/token'
1313

14-
// OpenAI API endpoint for direct calls (standard API, not ChatGPT backend)
15-
export const OPENAI_API_BASE_URL = 'https://api.openai.com'
16-
1714
// ChatGPT backend API endpoint for Codex requests
1815
export const CHATGPT_BACKEND_API_URL = 'https://chatgpt.com/backend-api'
1916

@@ -112,9 +109,3 @@ export function toCodexModelId(model: string): string | null {
112109
return null
113110
}
114111

115-
/**
116-
* @deprecated Use toCodexModelId instead
117-
*/
118-
export function toOpenAIModelId(openrouterModel: string): string | null {
119-
return toCodexModelId(openrouterModel)
120-
}

sdk/src/credentials.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import os from 'os'
44

55
import { env } from '@codebuff/common/env'
66
import { CLAUDE_OAUTH_CLIENT_ID } from '@codebuff/common/constants/claude-oauth'
7-
import { CODEX_OAUTH_CLIENT_ID } from '@codebuff/common/constants/codex-oauth'
7+
import { CODEX_OAUTH_CLIENT_ID, CODEX_OAUTH_TOKEN_URL } from '@codebuff/common/constants/codex-oauth'
88
import { userSchema } from '@codebuff/common/util/credentials'
99
import { z } from 'zod/v4'
1010

@@ -461,7 +461,7 @@ export const refreshCodexOAuthToken = async (
461461
})
462462

463463
const response = await fetch(
464-
'https://auth.openai.com/oauth/token',
464+
CODEX_OAUTH_TOKEN_URL,
465465
{
466466
method: 'POST',
467467
headers: {

sdk/src/impl/codex-transform.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,10 @@ export function transformMessagesToCodexInput(
111111
if (msg.content) {
112112
const textContent = typeof msg.content === 'string'
113113
? msg.content
114-
: msg.content.map(p => (p as TextContentPart).text).filter(Boolean).join('')
114+
: msg.content
115+
.filter((p): p is TextContentPart => p.type === 'text')
116+
.map(p => p.text)
117+
.join('')
115118

116119
if (textContent) {
117120
result.push({

sdk/src/impl/model-provider.ts

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -394,24 +394,41 @@ function createCodexFetch(
394394
try {
395395
const originalBody = JSON.parse(init.body)
396396

397+
// Extract system message for instructions and separate non-system messages
398+
const allMessages = originalBody.messages || []
399+
const systemMessage = allMessages.find(
400+
(m: { role: string }) => m.role === 'system',
401+
)
402+
const nonSystemMessages = allMessages.filter(
403+
(m: { role: string }) => m.role !== 'system',
404+
)
405+
406+
// Extract instructions from system message (Codex API REQUIRES this field)
407+
let instructions = 'You are a helpful assistant.'
408+
if (systemMessage) {
409+
instructions =
410+
typeof systemMessage.content === 'string'
411+
? systemMessage.content
412+
: systemMessage.content
413+
?.map((p: { text?: string }) => p.text)
414+
.filter(Boolean)
415+
.join('\n') || 'You are a helpful assistant.'
416+
}
417+
397418
// Transform from OpenAI chat format to Codex format
398419
const codexBody: Record<string, unknown> = {
399420
model: modelId,
400-
// Transform messages to Codex input format
401-
input: transformMessagesToCodexInput(originalBody.messages || []),
402-
// Codex-specific required fields
421+
instructions,
422+
input: transformMessagesToCodexInput(nonSystemMessages),
403423
store: false, // ChatGPT backend REQUIRES store=false
404424
stream: true, // Always stream
405-
// Reasoning configuration
406425
reasoning: {
407-
effort: 'medium',
426+
effort: 'high',
408427
summary: 'auto',
409428
},
410-
// Text verbosity
411429
text: {
412430
verbosity: 'medium',
413431
},
414-
// Include reasoning in response
415432
include: ['reasoning.encrypted_content'],
416433
}
417434

@@ -428,38 +445,13 @@ function createCodexFetch(
428445
name: fn.name,
429446
description: fn.description,
430447
parameters: fn.parameters,
431-
// Preserve any additional properties
432448
...(fn.strict !== undefined && { strict: fn.strict }),
433449
}
434450
}
435-
// Already in Codex format or unknown format, pass through
436451
return tool
437452
})
438453
}
439454

440-
// Extract system message for instructions (required by Codex API)
441-
const systemMessage = (originalBody.messages || []).find(
442-
(m: { role: string }) => m.role === 'system',
443-
)
444-
if (systemMessage) {
445-
codexBody.instructions =
446-
typeof systemMessage.content === 'string'
447-
? systemMessage.content
448-
: systemMessage.content
449-
?.map((p: { text?: string }) => p.text)
450-
.filter(Boolean)
451-
.join('\n') || 'You are a helpful assistant.'
452-
// Remove system message from input (it's now in instructions)
453-
codexBody.input = transformMessagesToCodexInput(
454-
(originalBody.messages || []).filter(
455-
(m: { role: string }) => m.role !== 'system',
456-
),
457-
)
458-
} else {
459-
// Codex API REQUIRES instructions field - provide default if no system message
460-
codexBody.instructions = 'You are a helpful assistant.'
461-
}
462-
463455
transformedBody = JSON.stringify(codexBody)
464456
} catch {
465457
// If parsing fails, use original body
@@ -471,6 +463,7 @@ function createCodexFetch(
471463
`${CHATGPT_BACKEND_API_URL}/codex/responses`,
472464
{
473465
method: 'POST',
466+
signal: init?.signal,
474467
headers: {
475468
'Content-Type': 'application/json',
476469
Authorization: `Bearer ${oauthToken}`,
@@ -510,10 +503,10 @@ function createCodexFetch(
510503

511504
for (const line of lines) {
512505
const trimmed = line.trim()
513-
if (!trimmed) continue
506+
// Only parse SSE data: lines, skip event:/id:/retry: and blank lines
507+
if (!trimmed.startsWith('data:')) continue
514508

515-
// Parse the SSE data line
516-
const data = trimmed.startsWith('data: ') ? trimmed.slice(6) : trimmed
509+
const data = trimmed.slice(5).trim()
517510
if (data === '[DONE]') continue
518511

519512
try {
@@ -533,9 +526,9 @@ function createCodexFetch(
533526
const lines = buffer.split('\n')
534527
for (const line of lines) {
535528
const trimmed = line.trim()
536-
if (!trimmed) continue
529+
if (!trimmed.startsWith('data:')) continue
537530

538-
const data = trimmed.startsWith('data: ') ? trimmed.slice(6) : trimmed
531+
const data = trimmed.slice(5).trim()
539532
if (data === '[DONE]') continue
540533

541534
try {

0 commit comments

Comments
 (0)