From 6fc7cb9daad27606a188aa6041d2d738b1ecca1f Mon Sep 17 00:00:00 2001 From: Suprhimp Date: Thu, 2 Jul 2026 13:16:04 +0900 Subject: [PATCH 1/2] feat: support Codex CLI login for the openai provider Fall back to OpenAI Codex CLI credentials (~/.codex/auth.json, override with CODEX_HOME) when no OPENAI_API_KEY/api_key is set. Two modes: - Sign in with ChatGPT: use the OAuth access token against the ChatGPT backend Responses endpoint (chatgpt.com/backend-api/codex), billed to the ChatGPT plan. Tokens auto-refresh on expiry and on a mid-review 401. - API-key login: use the OPENAI_API_KEY stored in auth.json. The ChatGPT backend is stateless (store: false): stream the request, aggregate output items from the stream, resend the full input (echoing encrypted reasoning) across tool rounds, and drop unsupported params (temperature, top_p, max_output_tokens, previous_response_id). Model is limited to plan-exposed slugs (default gpt-5.4). Cross-round continuity is preserved by replaying prior rounds' turns via a new conversationReplay provider capability (same mechanism as the Anthropic provider), so previous_review_id keeps working. Explicit env/request keys always take precedence over the CLI login. Add DUUL_REASONING_EFFORT (default medium) to tune reasoning effort. Co-Authored-By: Claude Opus 4.8 --- .changeset/codex-cli-login.md | 12 ++ README.ko.md | 27 ++- README.md | 28 +++- src/__tests__/codex-auth.test.ts | 115 +++++++++++++ src/services/providers/anthropic.ts | 1 + src/services/providers/codex-auth.ts | 193 ++++++++++++++++++++++ src/services/providers/google.ts | 1 + src/services/providers/openai.ts | 238 +++++++++++++++++++++++---- src/services/providers/types.ts | 8 +- src/services/reviewer.ts | 61 +++++-- 10 files changed, 633 insertions(+), 51 deletions(-) create mode 100644 .changeset/codex-cli-login.md create mode 100644 src/__tests__/codex-auth.test.ts create mode 100644 src/services/providers/codex-auth.ts diff --git a/.changeset/codex-cli-login.md b/.changeset/codex-cli-login.md new file mode 100644 index 0000000..7670b4f --- /dev/null +++ b/.changeset/codex-cli-login.md @@ -0,0 +1,12 @@ +--- +"@planningo/duul": minor +--- + +Support Codex CLI login for the `openai` provider — no `OPENAI_API_KEY` required. + +When no `OPENAI_API_KEY` (or per-request `api_key`) is set, DUUL now falls back to the OpenAI Codex CLI credentials in `~/.codex/auth.json` (override with `CODEX_HOME`): + +- **Sign in with ChatGPT:** uses the OAuth access token against the ChatGPT backend Responses endpoint (`https://chatgpt.com/backend-api/codex`), billed to your ChatGPT plan. Tokens are refreshed automatically via the OAuth endpoint (on expiry and on a mid-review 401). +- **API-key login:** uses the `OPENAI_API_KEY` stored in `auth.json`. + +The ChatGPT backend is stateless (`store: false`): DUUL streams the request, aggregates output items from the stream, resends the full input (echoing encrypted reasoning) across tool rounds, and drops unsupported params (`temperature`, `top_p`, `max_output_tokens`, `previous_response_id`). Cross-round context is preserved by replaying prior rounds' turns (new `conversationReplay` provider capability, same mechanism as the Anthropic provider), so `previous_review_id` continuity works. Add `DUUL_REASONING_EFFORT` (default `medium`) to tune reasoning effort. An explicit env/request key always takes precedence over the CLI login. diff --git a/README.ko.md b/README.ko.md index a026baa..432c38f 100644 --- a/README.ko.md +++ b/README.ko.md @@ -120,17 +120,42 @@ npm run build |------|------|--------|------| | `REVIEW_PROVIDER` | 아니오 | `openai` | 프로바이더: `openai`, `anthropic`, `google`, `openrouter`, `compatible` | | `REVIEW_MODEL` | 아니오 | 프로바이더 기본값 | 모델 ID (예: `gpt-5.4`, `claude-opus-4-20250514`, `gemini-3.1-pro-preview`) | -| `OPENAI_API_KEY` | 조건부 | -- | `openai` 또는 `compatible` 프로바이더 사용 시 필수 | +| `OPENAI_API_KEY` | 조건부 | -- | `openai`/`compatible`용 API 키. Codex CLI 로그인 시 생략 가능 (아래 참고) | | `ANTHROPIC_API_KEY` | 조건부 | -- | `anthropic` 프로바이더 사용 시 필수 | | `GOOGLE_API_KEY` | 조건부 | -- | `google` 프로바이더 사용 시 필수 | | `OPENROUTER_API_KEY` | 조건부 | -- | `openrouter` 프로바이더 사용 시 필수 | | `REVIEW_API_KEY` | 아니오 | -- | `compatible` 프로바이더용 API 키 (`OPENAI_API_KEY`로 폴백) | +| `CODEX_HOME` | 아니오 | `~/.codex` | Codex CLI `auth.json` 위치 (CLI 로그인용) | +| `DUUL_REASONING_EFFORT` | 아니오 | `medium` | ChatGPT 로그인 시 추론 강도 (`minimal`\|`low`\|`medium`\|`high`) | 프로바이더별 기본 모델: - **OpenAI:** `gpt-5.4` - **Anthropic:** `claude-opus-4-20250514` - **Google:** `gemini-3.1-pro-preview` +#### Codex CLI 로그인 사용 (API 키 불필요) + +`openai` 프로바이더는 [OpenAI Codex CLI](https://developers.openai.com/codex)에 +이미 로그인되어 있으면 `OPENAI_API_KEY` 없이도 동작합니다: + +```bash +codex login # "Sign in with ChatGPT" (Plus/Pro/Team) 또는 API 키 입력 +``` + +DUUL은 `~/.codex/auth.json`을 읽고(`CODEX_HOME`으로 경로 변경 가능): + +- **Sign in with ChatGPT:** OAuth 토큰으로 ChatGPT 백엔드 + (`https://chatgpt.com/backend-api/codex`)를 호출합니다. 토큰당 과금이 아니라 + ChatGPT 요금제로 청구되며, 만료된 토큰은 자동 갱신됩니다. +- **API 키 로그인:** `auth.json`에 저장된 `OPENAI_API_KEY`를 사용합니다. + +우선순위: 명시적 `OPENAI_API_KEY` 환경변수(또는 요청별 `api_key`)가 항상 우선이며, +키가 없을 때만 Codex 로그인으로 폴백합니다. 모델은 ChatGPT 요금제가 제공하는 것으로 +제한됩니다(예: `gpt-5.4`, `gpt-5.5`) — `REVIEW_MODEL`로 선택하세요. ChatGPT +백엔드는 무상태(stateless)라 네이티브 `previous_response_id` 체이닝 대신, 이전 +라운드의 대화 턴을 재생(replay)해 라운드 간 컨텍스트를 유지합니다(Anthropic +프로바이더와 동일한 방식) — `previous_review_id` 연속성이 정상 동작합니다. + #### 반복 제한 각 단계에는 최대 리뷰 반복 횟수가 있습니다. 초과하면 서버가 `requires_human_review: true`를 반환하여 사람에게 에스컬레이션합니다. diff --git a/README.md b/README.md index 86ce27d..e71ca5c 100644 --- a/README.md +++ b/README.md @@ -120,17 +120,43 @@ All configuration is done via environment variables, passed through the MCP `env |----------|----------|---------|-------------| | `REVIEW_PROVIDER` | No | `openai` | Provider: `openai`, `anthropic`, `google`, `openrouter`, `compatible` | | `REVIEW_MODEL` | No | Provider default | Model ID (e.g. `gpt-5.4`, `claude-opus-4-20250514`, `gemini-3.1-pro-preview`) | -| `OPENAI_API_KEY` | Conditional | -- | Required for `openai` or `compatible` provider | +| `OPENAI_API_KEY` | Conditional | -- | API key for `openai`/`compatible`. Optional if signed in with the Codex CLI (see below) | | `ANTHROPIC_API_KEY` | Conditional | -- | Required for `anthropic` provider | | `GOOGLE_API_KEY` | Conditional | -- | Required for `google` provider | | `OPENROUTER_API_KEY` | Conditional | -- | Required for `openrouter` provider | | `REVIEW_API_KEY` | No | -- | API key for `compatible` provider (falls back to `OPENAI_API_KEY`) | +| `CODEX_HOME` | No | `~/.codex` | Directory holding the Codex CLI `auth.json` (for CLI login) | +| `DUUL_REASONING_EFFORT` | No | `medium` | Reasoning effort for Sign in with ChatGPT (`minimal`\|`low`\|`medium`\|`high`) | Default models per provider: - **OpenAI:** `gpt-5.4` - **Anthropic:** `claude-opus-4-20250514` - **Google:** `gemini-3.1-pro-preview` +#### Sign in with the Codex CLI (no API key) + +For the `openai` provider you don't need an `OPENAI_API_KEY` if you're already +logged in to the [OpenAI Codex CLI](https://developers.openai.com/codex): + +```bash +codex login # "Sign in with ChatGPT" (Plus/Pro/Team) — or paste an API key +``` + +DUUL reads `~/.codex/auth.json` (override with `CODEX_HOME`) and: + +- **Sign in with ChatGPT:** uses your OAuth token against the ChatGPT backend + (`https://chatgpt.com/backend-api/codex`). Requests are billed to your ChatGPT + plan, not per-token. Expired tokens are refreshed automatically. +- **API-key login:** uses the `OPENAI_API_KEY` stored in `auth.json`. + +Precedence: an explicit `OPENAI_API_KEY` env var (or per-request `api_key`) always +wins; the Codex login is only used as a fallback when no key is set. Models are +limited to those your ChatGPT plan exposes (e.g. `gpt-5.4`, `gpt-5.5`); set +`REVIEW_MODEL` to pick one. The ChatGPT backend is stateless, so instead of +native `previous_response_id` chaining DUUL preserves cross-round context by +replaying prior rounds' turns (the same mechanism the Anthropic provider uses) — +`previous_review_id` continuity works as usual. + #### Iteration Limits Each phase has a maximum number of review iterations. When exceeded, the server returns `requires_human_review: true` so the caller can escalate to a human. diff --git a/src/__tests__/codex-auth.test.ts b/src/__tests__/codex-auth.test.ts new file mode 100644 index 0000000..91bbc90 --- /dev/null +++ b/src/__tests__/codex-auth.test.ts @@ -0,0 +1,115 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { jwtExp, isTokenExpired, loadCodexAuth, resolveCodexCredential, codexHome } from '../services/providers/codex-auth.js'; + +/** Build an unsigned JWT (header.payload.sig) with the given payload. */ +function makeJwt(payload: Record): string { + const b64 = (o: unknown) => Buffer.from(JSON.stringify(o)).toString('base64url'); + return `${b64({ alg: 'none' })}.${b64(payload)}.sig`; +} + +function withCodexHome(auth: unknown | null, fn: () => T): T { + const dir = mkdtempSync(join(tmpdir(), 'duul-codex-')); + const prev = process.env.CODEX_HOME; + process.env.CODEX_HOME = dir; + try { + if (auth !== null) writeFileSync(join(dir, 'auth.json'), JSON.stringify(auth)); + return fn(); + } finally { + if (prev === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prev; + rmSync(dir, { recursive: true, force: true }); + } +} + +test('codexHome honors CODEX_HOME override', () => { + const prev = process.env.CODEX_HOME; + process.env.CODEX_HOME = '/tmp/fake-codex'; + try { + assert.equal(codexHome(), '/tmp/fake-codex'); + } finally { + if (prev === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prev; + } +}); + +test('jwtExp decodes the exp claim', () => { + assert.equal(jwtExp(makeJwt({ exp: 123456 })), 123456); +}); + +test('jwtExp returns null for non-JWT strings', () => { + assert.equal(jwtExp('sk-not-a-jwt'), null); + assert.equal(jwtExp('a.b'), null); // payload not valid base64 JSON +}); + +test('isTokenExpired: future token is not expired', () => { + const future = Math.floor(Date.now() / 1000) + 3600; + assert.equal(isTokenExpired(makeJwt({ exp: future })), false); +}); + +test('isTokenExpired: token within skew window is expired', () => { + const soon = Math.floor(Date.now() / 1000) + 60; // < 5min skew + assert.equal(isTokenExpired(makeJwt({ exp: soon })), true); +}); + +test('isTokenExpired: unknown expiry treated as not expired', () => { + assert.equal(isTokenExpired('opaque-token'), false); +}); + +test('loadCodexAuth returns null when file absent', () => { + withCodexHome(null, () => { + assert.equal(loadCodexAuth(), null); + }); +}); + +test('loadCodexAuth parses auth.json', () => { + withCodexHome({ auth_mode: 'apikey', OPENAI_API_KEY: 'sk-test' }, () => { + assert.deepEqual(loadCodexAuth(), { auth_mode: 'apikey', OPENAI_API_KEY: 'sk-test' }); + }); +}); + +test('resolveCodexCredential: apikey mode', async () => { + const cred = await withCodexHome( + { auth_mode: 'apikey', OPENAI_API_KEY: 'sk-live-abc' }, + () => resolveCodexCredential(), + ); + assert.deepEqual(cred, { mode: 'apikey', apiKey: 'sk-live-abc' }); +}); + +test('resolveCodexCredential: chatgpt mode with valid token needs no network', async () => { + const future = Math.floor(Date.now() / 1000) + 3600; + const cred = await withCodexHome( + { + auth_mode: 'chatgpt', + OPENAI_API_KEY: null, + tokens: { access_token: makeJwt({ exp: future }), refresh_token: 'r', account_id: 'acct-1' }, + }, + () => resolveCodexCredential(), + ); + assert.equal(cred?.mode, 'chatgpt'); + if (cred?.mode === 'chatgpt') { + assert.equal(cred.accountId, 'acct-1'); + assert.ok(cred.accessToken.length > 0); + } +}); + +test('resolveCodexCredential: returns null when logged out', async () => { + const cred = await withCodexHome(null, () => resolveCodexCredential()); + assert.equal(cred, null); +}); + +test('resolveCodexCredential: chatgpt preferred even when api key present', async () => { + const future = Math.floor(Date.now() / 1000) + 3600; + const cred = await withCodexHome( + { + auth_mode: 'chatgpt', + OPENAI_API_KEY: 'sk-should-be-ignored', + tokens: { access_token: makeJwt({ exp: future }), refresh_token: 'r', account_id: 'acct-2' }, + }, + () => resolveCodexCredential(), + ); + assert.equal(cred?.mode, 'chatgpt'); +}); diff --git a/src/services/providers/anthropic.ts b/src/services/providers/anthropic.ts index cb5c9f6..b63d29a 100644 --- a/src/services/providers/anthropic.ts +++ b/src/services/providers/anthropic.ts @@ -190,6 +190,7 @@ export class AnthropicProvider implements ReviewerProvider { structuredOutputs: false, toolCalling: true, previousResponseId: true, // simulated via conversation history + conversationReplay: true, jsonSchemaStrict: false, }; diff --git a/src/services/providers/codex-auth.ts b/src/services/providers/codex-auth.ts new file mode 100644 index 0000000..76d8e8c --- /dev/null +++ b/src/services/providers/codex-auth.ts @@ -0,0 +1,193 @@ +/** + * Codex CLI login support. + * + * Lets DUUL reuse the credentials produced by `codex login` (the OpenAI Codex + * CLI) instead of requiring a raw OPENAI_API_KEY. Two auth modes are handled: + * + * 1. "apikey" — auth.json carries an OPENAI_API_KEY; we just use it. + * 2. "chatgpt" — Sign in with ChatGPT (Plus/Pro/Team). auth.json carries an + * OAuth access token + account id. Requests go to the ChatGPT + * backend Responses endpoint with a bearer token; the token is + * refreshed via the OpenAI OAuth endpoint when near expiry. + * + * Credential file: $CODEX_HOME/auth.json (defaults to ~/.codex/auth.json). + * + * Protocol constants mirror the openai/codex `codex-rs` client so DUUL speaks + * the same dialect the CLI does. + */ +import { readFileSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +/** ChatGPT-login base URL for the Responses API (POST {base}/responses). */ +export const CHATGPT_BASE_URL = 'https://chatgpt.com/backend-api/codex'; + +/** OAuth token endpoint used to refresh a ChatGPT access token. */ +const OAUTH_TOKEN_URL = process.env.CODEX_REFRESH_TOKEN_URL_OVERRIDE ?? 'https://auth.openai.com/oauth/token'; + +/** Public OAuth client id the Codex CLI registers under. */ +const OAUTH_CLIENT_ID = process.env.CODEX_APP_SERVER_LOGIN_CLIENT_ID ?? 'app_EMoamEEZ73f0CkXaXp7hrann'; + +/** Refresh the access token when it has this many seconds (or fewer) of life left. */ +const EXPIRY_SKEW_SECONDS = 5 * 60; + +export interface CodexTokens { + id_token?: string; + access_token?: string; + refresh_token?: string; + account_id?: string; +} + +export interface CodexAuth { + auth_mode?: string; + OPENAI_API_KEY?: string | null; + tokens?: CodexTokens; + last_refresh?: string; +} + +export type CodexCredential = + | { mode: 'apikey'; apiKey: string } + | { mode: 'chatgpt'; accessToken: string; accountId: string; refresh: () => Promise }; + +/** Resolve the Codex home directory ($CODEX_HOME or ~/.codex). */ +export function codexHome(): string { + return process.env.CODEX_HOME ?? join(homedir(), '.codex'); +} + +function authPath(): string { + return join(codexHome(), 'auth.json'); +} + +/** Read and parse auth.json. Returns null when the file is missing or unparsable. */ +export function loadCodexAuth(): CodexAuth | null { + try { + const raw = readFileSync(authPath(), 'utf-8'); + return JSON.parse(raw) as CodexAuth; + } catch { + return null; + } +} + +/** + * Decode the `exp` (seconds since epoch) claim from a JWT without verifying it. + * Returns null when the token is not a decodable JWT. + */ +export function jwtExp(token: string): number | null { + const parts = token.split('.'); + if (parts.length < 2) return null; + try { + let payload = parts[1].replace(/-/g, '+').replace(/_/g, '/'); + payload += '='.repeat((4 - (payload.length % 4)) % 4); + const claims = JSON.parse(Buffer.from(payload, 'base64').toString('utf-8')) as { exp?: number }; + return typeof claims.exp === 'number' ? claims.exp : null; + } catch { + return null; + } +} + +/** + * True when the token is expired or within EXPIRY_SKEW_SECONDS of expiring. + * Unknown expiry is treated as "not expired" so we don't refresh needlessly. + */ +export function isTokenExpired(token: string, nowSeconds = Math.floor(Date.now() / 1000)): boolean { + const exp = jwtExp(token); + if (exp === null) return false; + return exp - nowSeconds <= EXPIRY_SKEW_SECONDS; +} + +interface RefreshResponse { + id_token?: string; + access_token?: string; + refresh_token?: string; +} + +/** + * Exchange the stored refresh_token for a fresh access token via the OpenAI + * OAuth endpoint, then persist the rotated tokens back to auth.json. + * Returns the updated CodexAuth. Throws on network/HTTP failure. + */ +export async function refreshCodexToken(auth: CodexAuth): Promise { + const refreshToken = auth.tokens?.refresh_token; + if (!refreshToken) { + throw new Error('Codex auth has no refresh_token; run `codex login` again.'); + } + + const res = await fetch(OAUTH_TOKEN_URL, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + client_id: OAUTH_CLIENT_ID, + grant_type: 'refresh_token', + refresh_token: refreshToken, + }), + }); + + if (!res.ok) { + const body = await res.text().catch(() => ''); + throw new Error(`Codex token refresh failed (${res.status}): ${body.slice(0, 200)}`); + } + + const data = (await res.json()) as RefreshResponse; + const updated: CodexAuth = { + ...auth, + tokens: { + ...auth.tokens, + ...(data.access_token ? { access_token: data.access_token } : {}), + ...(data.id_token ? { id_token: data.id_token } : {}), + // Refresh tokens rotate; keep the old one only if none is returned. + ...(data.refresh_token ? { refresh_token: data.refresh_token } : {}), + }, + last_refresh: new Date().toISOString(), + }; + + try { + writeFileSync(authPath(), JSON.stringify(updated, null, 2), { mode: 0o600 }); + } catch (error) { + // Non-fatal: we can still use the refreshed token in-memory this run. + console.error(`[duul] Warning: could not persist refreshed Codex token: ${error instanceof Error ? error.message : error}`); + } + + return updated; +} + +/** + * Resolve a usable credential from the Codex CLI login, or null when the CLI + * is not logged in. Refreshes an expired ChatGPT access token up front. + * + * The returned `refresh` callback (chatgpt mode) re-reads auth.json and rotates + * the token, so a provider can recover from a mid-review 401. + */ +export async function resolveCodexCredential(): Promise { + const auth = loadCodexAuth(); + if (!auth) return null; + + const tokens = auth.tokens; + const chatgptCapable = !!(tokens?.access_token && tokens?.account_id); + const preferChatgpt = auth.auth_mode === 'chatgpt' || (!auth.OPENAI_API_KEY && chatgptCapable); + + if (preferChatgpt && chatgptCapable) { + let accessToken = tokens!.access_token!; + if (isTokenExpired(accessToken) && tokens!.refresh_token) { + const refreshed = await refreshCodexToken(auth); + accessToken = refreshed.tokens?.access_token ?? accessToken; + } + return { + mode: 'chatgpt', + accessToken, + accountId: tokens!.account_id!, + refresh: async () => { + const current = loadCodexAuth() ?? auth; + const refreshed = await refreshCodexToken(current); + const next = refreshed.tokens?.access_token; + if (!next) throw new Error('Codex token refresh returned no access_token'); + return next; + }, + }; + } + + if (auth.OPENAI_API_KEY) { + return { mode: 'apikey', apiKey: auth.OPENAI_API_KEY }; + } + + return null; +} diff --git a/src/services/providers/google.ts b/src/services/providers/google.ts index 6ceb4d9..5fdbdcd 100644 --- a/src/services/providers/google.ts +++ b/src/services/providers/google.ts @@ -158,6 +158,7 @@ export class GoogleProvider implements ReviewerProvider { structuredOutputs: false, toolCalling: true, previousResponseId: false, + conversationReplay: false, jsonSchemaStrict: false, }; diff --git a/src/services/providers/openai.ts b/src/services/providers/openai.ts index edde525..2b9e6ec 100644 --- a/src/services/providers/openai.ts +++ b/src/services/providers/openai.ts @@ -1,7 +1,9 @@ +import { randomUUID } from 'node:crypto'; import OpenAI from 'openai'; import { zodTextFormat } from 'openai/helpers/zod'; import type { z } from 'zod'; import { validateProjectRoot } from '../filesystem.js'; +import { CHATGPT_BASE_URL } from './codex-auth.js'; import { executeFilesystemTool, createReviewerByteBudget } from '../filesystem-tools.js'; import type { ReviewerProvider, @@ -10,6 +12,7 @@ import type { ProviderCapabilities, ExhaustionReason, TokenUsage, + ConversationTurn, } from './types.js'; import { estimateCost } from '../pricing.js'; @@ -187,38 +190,83 @@ function validateInputLength(systemPrompt: string, userMessage: string): void { } } +/** + * ChatGPT-login (Codex CLI) credentials. When present the provider talks to the + * ChatGPT backend Responses endpoint with a bearer token instead of an API key. + */ +export interface ChatgptAuth { + accessToken: string; + accountId: string; + /** Rotate the token (e.g. after a 401). Returns a fresh access token. */ + refresh?: () => Promise; +} + export class OpenAIProvider implements ReviewerProvider { readonly name = 'openai'; - readonly capabilities: ProviderCapabilities = { - structuredOutputs: true, - toolCalling: true, - previousResponseId: true, - jsonSchemaStrict: true, - }; + readonly capabilities: ProviderCapabilities; private client: OpenAI; private model: string; private temperature: number; private topP: number; - constructor(config?: { apiKey?: string; baseUrl?: string; model?: string; temperature?: number; topP?: number }) { - const apiKey = config?.apiKey ?? process.env.OPENAI_API_KEY; + /** + * ChatGPT-backend mode. The endpoint is stateless (`store: false`): it does + * not support `previous_response_id`, `temperature`/`top_p`, or + * `max_output_tokens`, and it streams. We resend the full input each turn. + */ + private readonly stateless: boolean; + private readonly baseURL?: string; + private readonly defaultHeaders?: Record; + private readonly refresh?: () => Promise; + private readonly reasoningEffort: string; + + constructor(config?: { apiKey?: string; baseUrl?: string; model?: string; temperature?: number; topP?: number; chatgpt?: ChatgptAuth }) { + const chatgpt = config?.chatgpt; + this.stateless = !!chatgpt; + this.refresh = chatgpt?.refresh; + this.reasoningEffort = process.env.DUUL_REASONING_EFFORT ?? 'medium'; + + const apiKey = chatgpt?.accessToken ?? config?.apiKey ?? process.env.OPENAI_API_KEY; if (!apiKey) { - throw new Error('OPENAI_API_KEY environment variable is not set'); + throw new Error( + 'No OpenAI credential found. Set OPENAI_API_KEY, or sign in with the Codex CLI (`codex login`).', + ); } - this.client = new OpenAI({ - apiKey, - ...(config?.baseUrl ? { baseURL: config.baseUrl } : {}), - }); + + this.baseURL = chatgpt ? CHATGPT_BASE_URL : config?.baseUrl; + this.defaultHeaders = chatgpt + ? { 'chatgpt-account-id': chatgpt.accountId, originator: 'codex_cli_rs', 'session-id': randomUUID() } + : undefined; + this.client = this.buildClient(apiKey); + this.model = config?.model ?? process.env.REVIEW_MODEL ?? 'gpt-5.4'; this.temperature = config?.temperature ?? 0.2; this.topP = config?.topP ?? 0.1; + this.capabilities = { + structuredOutputs: true, + toolCalling: true, + // Both modes support cross-round continuity: api-key mode natively via + // previous_response_id, ChatGPT mode by replaying conversation turns. + previousResponseId: true, + // ChatGPT backend is stateless — continuity comes from turn replay. + conversationReplay: this.stateless, + jsonSchemaStrict: true, + }; + } + + private buildClient(apiKey: string): OpenAI { + return new OpenAI({ + apiKey, + ...(this.baseURL ? { baseURL: this.baseURL } : {}), + ...(this.defaultHeaders ? { defaultHeaders: this.defaultHeaders } : {}), + }); } async review( options: ReviewCallOptions, ): Promise>> { - const { systemPrompt, userMessage, schemaName, outputSchema, workspaceScope, previousReviewId } = options; + const { systemPrompt, userMessage, schemaName, outputSchema, workspaceScope, previousReviewId, conversationHistory } = options; validateInputLength(systemPrompt, userMessage); @@ -260,22 +308,54 @@ export class OpenAIProvider implements ReviewerProvider { const baseParams: Record = { model: this.model, instructions: systemPrompt, - temperature: this.temperature, - top_p: this.topP, - max_output_tokens: 16384, text: { format: zodTextFormat(outputSchema, schemaName) }, ...(tools ? { tools } : {}), + ...(this.stateless + ? { + // ChatGPT backend: stateless, reasoning-only sampling, encrypted + // reasoning must be echoed back on each turn (store: false). + store: false, + reasoning: { effort: this.reasoningEffort }, + include: ['reasoning.encrypted_content'], + } + : { + temperature: this.temperature, + top_p: this.topP, + max_output_tokens: 16384, + }), }; - let response = await this.apiCallWithRetry({ - ...baseParams, - input: [{ role: 'user' as const, content: [{ type: 'input_text' as const, text: userMessage }] }], - ...(previousReviewId ? { previous_response_id: previousReviewId } : {}), - }); + // Stateless (ChatGPT backend): accumulate the full input across tool rounds + // since there is no server-side `previous_response_id` chaining. Prior rounds + // are replayed as message items (user: input_text, assistant: output_text). + const inputItems: unknown[] = []; + if (this.stateless && conversationHistory?.length) { + inputItems.push(...(conversationHistory as unknown[])); + } + inputItems.push({ role: 'user' as const, content: [{ type: 'input_text' as const, text: userMessage }] }); + + let response = this.stateless + ? await this.apiCallWithRetry({ ...baseParams, input: inputItems }) + : await this.apiCallWithRetry({ + ...baseParams, + input: inputItems, + ...(previousReviewId ? { previous_response_id: previousReviewId } : {}), + }); accumulateUsage(response); console.error(`[duul] response.id=${response.id} model=${this.model} provider=openai`); + // Continue the conversation after a tool round. Stateless mode resends the + // whole input (prior assistant output items + the new tool outputs); chained + // mode uses server-side previous_response_id and sends only the new items. + const continueConversation = async (newItems: unknown[]): Promise => { + if (this.stateless) { + inputItems.push(...response.output, ...newItems); + return this.apiCallWithRetry({ ...baseParams, input: inputItems }); + } + return this.apiCallWithRetry({ ...baseParams, previous_response_id: response.id, input: newItems }); + }; + // Agentic tool-calling loop if (effectiveRoot) { const toolReadBudget = MAX_INPUT_CHARS - (systemPrompt.length + userMessage.length); @@ -353,7 +433,7 @@ export class OpenAIProvider implements ReviewerProvider { toolResults.push({ type: 'function_call_output' as const, call_id: call.call_id, output: result }); } - response = await this.apiCallWithRetry({ ...baseParams, previous_response_id: response.id, input: toolResults }); + response = await continueConversation(toolResults); accumulateUsage(response); console.error(`[duul] response.id=${response.id} (after tool round ${round + 1})`); @@ -362,7 +442,7 @@ export class OpenAIProvider implements ReviewerProvider { type: 'function_call_output' as const, call_id: c.call_id, output: 'No more file reads allowed. You must produce your final review verdict now.', })); - response = await this.apiCallWithRetry({ ...baseParams, previous_response_id: response.id, input: stopResults }); + response = await continueConversation(stopResults); accumulateUsage(response); break; } @@ -374,7 +454,7 @@ export class OpenAIProvider implements ReviewerProvider { type: 'function_call_output' as const, call_id: c.call_id, output: 'Tool call limit reached. You must produce your final review verdict now.', })); - response = await this.apiCallWithRetry({ ...baseParams, previous_response_id: response.id, input: stopResults }); + response = await continueConversation(stopResults); accumulateUsage(response); } } @@ -384,36 +464,78 @@ export class OpenAIProvider implements ReviewerProvider { const cachedStr = usage.cached_input_tokens ? ` [cached: ${usage.cached_input_tokens}]` : ''; console.error(`[duul] Token usage: ${usage.input_tokens} in + ${usage.output_tokens} out = ${usage.total_tokens} total (${usage.api_calls} API calls)${cachedStr}${costStr}`); + // Stateless mode: record this round's user/assistant turns so the reviewer + // can replay them next round (the ChatGPT backend has no native chaining). + // Only the final Q&A is kept — replaying every tool call would bloat tokens + // and risks stale encrypted-reasoning items across separate responses. + const buildTurns = (assistantText: string): ConversationTurn[] | undefined => + this.stateless + ? [ + ...(conversationHistory ?? []), + { role: 'user' as const, content: [{ type: 'input_text', text: userMessage }] }, + { role: 'assistant' as const, content: [{ type: 'output_text', text: assistantText }] }, + ] + : undefined; + // Extract structured output + const outputText = this.getOutputText(response); const parsed = this.extractStructuredOutput(response, outputSchema); if (parsed !== null) { - return { parsed, reviewId: response.id, usage }; + return { parsed, reviewId: response.id, usage, conversationTurns: buildTurns(outputText ?? '') }; } if (options.createFallback) { const reason: ExhaustionReason = this.hasPendingFunctionCalls(response) ? 'round_limit' : 'budget'; const fallback = options.createFallback(reason, allUsedTools); console.error(`[duul] Returning structured fallback (reason: ${reason}).`); - return { parsed: fallback, reviewId: response.id, usage }; + return { parsed: fallback, reviewId: response.id, usage, conversationTurns: buildTurns(outputText ?? JSON.stringify(fallback)) }; } throw new Error('Review failed: could not obtain structured verdict after tool loop.'); } private async apiCallWithRetry(params: Record): Promise { + let refreshedOnce = false; for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 120_000); try { - const response = await this.client.responses.create( - { ...params, stream: false } as Parameters[0], - { signal: controller.signal }, - ) as OpenAI.Responses.Response; + let response: OpenAI.Responses.Response; + if (this.stateless) { + // ChatGPT backend requires streaming and leaves `response.completed`'s + // `output` empty — aggregate items from the streamed events instead. + const stream = this.client.responses.stream( + params as Parameters[0], + { signal: controller.signal }, + ); + response = await this.aggregateStream(stream); + } else { + response = (await this.client.responses.create( + { ...params, stream: false } as Parameters[0], + { signal: controller.signal }, + )) as OpenAI.Responses.Response; + } clearTimeout(timeout); return response; } catch (error: unknown) { clearTimeout(timeout); - const isRetryable = error instanceof Error && ('status' in error ? ((error as { status: number }).status === 429 || (error as { status: number }).status >= 500) : error.name === 'AbortError'); + const status = error instanceof Error && 'status' in error ? (error as { status: number }).status : undefined; + + // ChatGPT token expired mid-review: refresh once and retry immediately. + if (status === 401 && this.refresh && !refreshedOnce) { + refreshedOnce = true; + try { + const token = await this.refresh(); + this.client = this.buildClient(token); + console.error('[duul] Refreshed Codex token after 401, retrying'); + attempt--; // don't consume a retry for the refresh + continue; + } catch (refreshError) { + console.error(`[duul] Codex token refresh failed: ${refreshError instanceof Error ? refreshError.message : refreshError}`); + } + } + + const isRetryable = error instanceof Error && (status !== undefined ? (status === 429 || status >= 500) : error.name === 'AbortError'); if (isRetryable && attempt < MAX_RETRIES - 1) { const delay = 1000 * Math.pow(2, attempt); console.error(`[duul] Retry ${attempt + 1}/${MAX_RETRIES} after ${delay}ms`); @@ -426,6 +548,58 @@ export class OpenAIProvider implements ReviewerProvider { throw new Error('Unreachable: exhausted retries'); } + /** + * Aggregate a streamed Responses call into a Response object. + * + * The ChatGPT backend delivers completed output items via + * `response.output_item.done` events and returns an EMPTY `output` array on + * `response.completed`, so we collect items from the stream ourselves. Usage + * and id come from `response.completed` (falling back to `response.created`). + */ + private async aggregateStream( + stream: AsyncIterable, + ): Promise { + const output: OpenAI.Responses.ResponseOutputItem[] = []; + let id = ''; + let usage: OpenAI.Responses.ResponseUsage | undefined; + + for await (const event of stream) { + switch (event.type) { + case 'response.created': + id = event.response.id; + break; + case 'response.output_item.done': + output.push(event.item); + break; + case 'response.completed': + id = event.response.id ?? id; + usage = event.response.usage; + break; + case 'response.failed': + throw new Error(`ChatGPT backend response failed: ${event.response.error?.message ?? 'unknown error'}`); + case 'error': + throw new Error(`ChatGPT backend stream error: ${event.message ?? 'unknown error'}`); + default: + break; + } + } + + return { id, output, usage } as unknown as OpenAI.Responses.Response; + } + + /** Return the first output_text string in the response, or null. */ + private getOutputText(response: OpenAI.Responses.Response): string | null { + for (const item of response.output) { + if (item.type === 'message' && 'content' in item) { + const msg = item as { content: Array<{ type: string; text?: string }> }; + for (const content of msg.content) { + if (content.type === 'output_text' && content.text) return content.text; + } + } + } + return null; + } + private extractStructuredOutput(response: OpenAI.Responses.Response, outputSchema: T): z.infer | null { for (const item of response.output) { if (item.type === 'message' && 'content' in item) { diff --git a/src/services/providers/types.ts b/src/services/providers/types.ts index e05f276..6b2f7de 100644 --- a/src/services/providers/types.ts +++ b/src/services/providers/types.ts @@ -62,8 +62,14 @@ export interface ProviderCapabilities { structuredOutputs: boolean; /** Supports tool/function calling */ toolCalling: boolean; - /** Supports previous_response_id for conversation continuity */ + /** Supports conversation continuity across rounds (native chaining or replay) */ previousResponseId: boolean; + /** + * Continuity is achieved by replaying prior turns (conversationHistory) rather + * than native server-side chaining. When true, the reviewer stores/loads + * conversation turns per reviewId and passes them back on the next round. + */ + conversationReplay: boolean; /** Supports strict JSON schema mode */ jsonSchemaStrict: boolean; } diff --git a/src/services/reviewer.ts b/src/services/reviewer.ts index 163373f..2b7a2bc 100644 --- a/src/services/reviewer.ts +++ b/src/services/reviewer.ts @@ -7,9 +7,10 @@ import { readFile, writeFile, mkdir } from 'node:fs/promises'; import { join, dirname } from 'node:path'; import type { WorkspaceScope } from './filesystem.js'; import type { ReviewerProvider, ReviewCallResult, ExhaustionReason, TokenUsage, ConversationTurn } from './providers/types.js'; -import { OpenAIProvider } from './providers/openai.js'; +import { OpenAIProvider, type ChatgptAuth } from './providers/openai.js'; import { AnthropicProvider } from './providers/anthropic.js'; import { GoogleProvider } from './providers/google.js'; +import { resolveCodexCredential } from './providers/codex-auth.js'; export type { ReviewerProvider, ReviewCallResult, ExhaustionReason, TokenUsage }; @@ -120,6 +121,28 @@ function getProviderCacheKey( }); } +/** + * Resolve the OpenAI credential, falling back to the Codex CLI login when no + * explicit or env API key is present. Returns either an API key or a ChatGPT + * bearer credential (Sign in with ChatGPT). + */ +async function resolveOpenAiCredential( + configApiKey: string | undefined, +): Promise<{ apiKey?: string; chatgpt?: ChatgptAuth }> { + const explicitKey = configApiKey ?? process.env.OPENAI_API_KEY; + if (explicitKey) return { apiKey: explicitKey }; + + const cred = await resolveCodexCredential(); + if (!cred) return {}; // let the provider throw its standard "no credential" error + + if (cred.mode === 'apikey') { + console.error('[duul] Using OpenAI API key from Codex CLI login (~/.codex/auth.json)'); + return { apiKey: cred.apiKey }; + } + console.error('[duul] Using Sign in with ChatGPT credentials from Codex CLI login'); + return { chatgpt: { accessToken: cred.accessToken, accountId: cred.accountId, refresh: cred.refresh } }; +} + /** * Create or retrieve a cached provider instance. * @@ -127,10 +150,10 @@ function getProviderCacheKey( * `{ plan: "...", code: "...", partition: "..." }`. The resolved model * participates in the cache key so per-tool models don't collide. */ -function getProvider( +async function getProvider( reviewerConfig?: ReviewOptions['reviewerConfig'], toolName?: ReviewToolName, -): ReviewerProvider { +): Promise { const providerName = resolveProviderName(reviewerConfig?.provider); const hasEphemeralKey = !!reviewerConfig?.api_key; const resolvedModel = resolveModelForTool(reviewerConfig?.model, toolName); @@ -153,11 +176,16 @@ function getProvider( }; let provider: ReviewerProvider; + // ChatGPT-login providers hold a rotating bearer token — never cache them. + let bypassCache = hasEphemeralKey; switch (providerName) { - case 'openai': - provider = new OpenAIProvider(constructorConfig); + case 'openai': { + const cred = await resolveOpenAiCredential(reviewerConfig?.api_key); + if (cred.chatgpt) bypassCache = true; + provider = new OpenAIProvider({ ...constructorConfig, apiKey: cred.apiKey ?? apiKey, chatgpt: cred.chatgpt }); break; + } case 'anthropic': provider = new AnthropicProvider(constructorConfig); break; @@ -180,8 +208,8 @@ function getProvider( throw new Error(`Unknown provider: ${providerName}`); } - // Only cache env-based providers (not ephemeral per-request keys) - if (!hasEphemeralKey) { + // Only cache stable env-based providers (not ephemeral keys or rotating tokens) + if (!bypassCache) { // Evict oldest entry if cache is full if (providerCache.size >= MAX_CACHE_SIZE) { const oldestKey = providerCache.keys().next().value!; @@ -192,7 +220,7 @@ function getProvider( providerCache.set(cacheKey, provider); } - console.error(`[duul] Created ${providerName} provider (model: ${resolvedModel ?? 'default'}${toolName ? `, tool: ${toolName}` : ''}${hasEphemeralKey ? ', ephemeral key' : ''})`); + console.error(`[duul] Created ${providerName} provider (model: ${resolvedModel ?? 'default'}${toolName ? `, tool: ${toolName}` : ''}${bypassCache ? ', uncached' : ''})`); return provider; } @@ -293,7 +321,7 @@ async function storeConversation(reviewId: string, turns: ConversationTurn[], wo export async function callReview( options: ReviewOptions, ): Promise>> { - const provider = getProvider(options.reviewerConfig, options.toolName); + const provider = await getProvider(options.reviewerConfig, options.toolName); // Log capability warnings for non-full-featured providers if (!provider.capabilities.toolCalling && options.workspaceScope?.root) { @@ -302,19 +330,20 @@ export async function callReview( 'Reviewer will not be able to explore the workspace. Consider providing more context via relevant_code/artifact_refs.', ); } - if (!provider.capabilities.previousResponseId && options.previousReviewId) { + if (!provider.capabilities.previousResponseId && !provider.capabilities.conversationReplay && options.previousReviewId) { console.error( - `[duul] Warning: ${provider.name} provider does not support previous_response_id. ` + + `[duul] Warning: ${provider.name} provider does not support conversation continuity. ` + 'Reviewer context from previous rounds will not be available.', ); } const workspaceRoot = options.workspaceScope?.root; - // Retrieve conversation history for providers that use simulated context - // OpenAI uses native previous_response_id, so skip for it + // Retrieve conversation history for replay-based providers (Anthropic, and the + // OpenAI ChatGPT-login backend). Native-chaining providers (OpenAI api-key) + // pass previousReviewId straight through and don't need replay. let conversationHistory: ConversationTurn[] | undefined; - if (options.previousReviewId && provider.capabilities.previousResponseId && provider.name !== 'openai') { + if (options.previousReviewId && provider.capabilities.conversationReplay) { conversationHistory = await getConversationHistory(options.previousReviewId, workspaceRoot); if (conversationHistory) { console.error(`[duul] Loaded conversation history for ${options.previousReviewId} (${conversationHistory.length} turns)`); @@ -325,8 +354,8 @@ export async function callReview( const result = await provider.review({ ...options, conversationHistory }); - // Store conversation turns for future rounds (non-OpenAI providers) - if (result.conversationTurns?.length && provider.name !== 'openai') { + // Store conversation turns for future rounds (replay-based providers only) + if (result.conversationTurns?.length && provider.capabilities.conversationReplay) { await storeConversation(result.reviewId, result.conversationTurns, workspaceRoot); console.error(`[duul] Stored conversation (${result.conversationTurns.length} turns) for ${result.reviewId}`); } From b5c863c845de5b16f82ea30a273fd1dc4a8966cd Mon Sep 17 00:00:00 2001 From: Suprhimp Date: Thu, 2 Jul 2026 13:42:40 +0900 Subject: [PATCH 2/2] fix: harden Codex login + cross-round continuity per DUUL review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses blocking issues raised over five DUUL plan-review rounds: - reviewer: clear the conversation memoryCache on workspace switch so one workspace's turns are never flushed into or replayed from another's .duul/conversations.json. - codex-auth: when a ChatGPT token is expired, fall back to a stored OPENAI_API_KEY if refresh is unavailable or fails, instead of returning a credential that will 401. - codex-auth: chmodSync(auth.json, 0600) after refresh — the writeFileSync mode option only applies on create, not overwrite. - capabilities: previousResponseId now means NATIVE server-side chaining only (OpenAI api-key mode); replay-based continuity is conversationReplay. Anthropic set to previousResponseId=false/conversationReplay=true. - reviewer: extract pure continuityPlan() helper for the load/warn decision. Tests (94 total): OpenAI capability flags per mode, codex-auth refresh perms/failure/fallback, continuityPlan cases, conversation store roundtrip, and workspace-switch isolation. Co-Authored-By: Claude Opus 4.8 --- README.md | 2 +- src/__tests__/codex-auth.test.ts | 78 ++++++++++++++++++++++- src/__tests__/openai-capabilities.test.ts | 32 ++++++++++ src/__tests__/reviewer-continuity.test.ts | 77 ++++++++++++++++++++++ src/services/providers/anthropic.ts | 2 +- src/services/providers/codex-auth.ts | 29 +++++++-- src/services/providers/openai.ts | 7 +- src/services/providers/types.ts | 2 +- src/services/reviewer.ts | 42 ++++++++++-- 9 files changed, 252 insertions(+), 19 deletions(-) create mode 100644 src/__tests__/openai-capabilities.test.ts create mode 100644 src/__tests__/reviewer-continuity.test.ts diff --git a/README.md b/README.md index e71ca5c..4103210 100644 --- a/README.md +++ b/README.md @@ -510,7 +510,7 @@ When `workspace_root` is provided, the reviewer gains access to 7 file explorati **Degradation behavior:** - **No structured outputs:** JSON prompting + zod validation fallback. - **No tool calling:** Reviewer cannot explore the workspace. Provide more context via `relevant_code` and `artifact_refs`. -- **No previous response ID:** Each review call is independent (no conversation memory). +- **No previous response ID:** Native server-side chaining is unavailable. Anthropic and the OpenAI ChatGPT-login backend still preserve cross-round context by replaying prior turns (conversation replay); Google is independent per call. --- diff --git a/src/__tests__/codex-auth.test.ts b/src/__tests__/codex-auth.test.ts index 91bbc90..ed6fd6b 100644 --- a/src/__tests__/codex-auth.test.ts +++ b/src/__tests__/codex-auth.test.ts @@ -1,9 +1,9 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { mkdtempSync, writeFileSync, rmSync, statSync, chmodSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { jwtExp, isTokenExpired, loadCodexAuth, resolveCodexCredential, codexHome } from '../services/providers/codex-auth.js'; +import { jwtExp, isTokenExpired, loadCodexAuth, resolveCodexCredential, refreshCodexToken, codexHome } from '../services/providers/codex-auth.js'; /** Build an unsigned JWT (header.payload.sig) with the given payload. */ function makeJwt(payload: Record): string { @@ -101,6 +101,80 @@ test('resolveCodexCredential: returns null when logged out', async () => { assert.equal(cred, null); }); +test('resolveCodexCredential: expired token + no refresh_token falls back to api key', async () => { + const past = Math.floor(Date.now() / 1000) - 3600; + const cred = await withCodexHome( + { + auth_mode: 'chatgpt', + OPENAI_API_KEY: 'sk-fallback', + tokens: { access_token: makeJwt({ exp: past }), account_id: 'acct-3' }, + }, + () => resolveCodexCredential(), + ); + assert.deepEqual(cred, { mode: 'apikey', apiKey: 'sk-fallback' }); +}); + +test('resolveCodexCredential: expired token, no refresh, no key returns chatgpt cred', async () => { + const past = Math.floor(Date.now() / 1000) - 3600; + const cred = await withCodexHome( + { + auth_mode: 'chatgpt', + OPENAI_API_KEY: null, + tokens: { access_token: makeJwt({ exp: past }), account_id: 'acct-4' }, + }, + () => resolveCodexCredential(), + ); + // No refresh path and no key: proceed so the provider surfaces a clear error. + assert.equal(cred?.mode, 'chatgpt'); +}); + +test('refreshCodexToken forces 0600 on a pre-existing loose file + persists rotated tokens', async () => { + const dir = mkdtempSync(join(tmpdir(), 'duul-codex-')); + const prevHome = process.env.CODEX_HOME; + const prevFetch = globalThis.fetch; + process.env.CODEX_HOME = dir; + const path = join(dir, 'auth.json'); + const auth = { auth_mode: 'chatgpt', tokens: { access_token: 'old', refresh_token: 'r1', account_id: 'a' } }; + writeFileSync(path, JSON.stringify(auth)); + chmodSync(path, 0o644); // simulate a loosely-permissioned file + + globalThis.fetch = (async () => + new Response(JSON.stringify({ access_token: 'new-at', id_token: 'new-id', refresh_token: 'r2' }), { + status: 200, + })) as typeof fetch; + + try { + const updated = await refreshCodexToken(auth); + assert.equal(updated.tokens?.access_token, 'new-at'); + assert.equal(updated.tokens?.refresh_token, 'r2'); + assert.equal(statSync(path).mode & 0o777, 0o600); + // Rotated tokens are persisted to disk. + assert.equal(loadCodexAuth()?.tokens?.access_token, 'new-at'); + } finally { + globalThis.fetch = prevFetch; + if (prevHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevHome; + rmSync(dir, { recursive: true, force: true }); + } +}); + +test('refreshCodexToken throws on HTTP failure', async () => { + const prevFetch = globalThis.fetch; + globalThis.fetch = (async () => new Response('nope', { status: 400 })) as typeof fetch; + try { + await assert.rejects( + () => refreshCodexToken({ tokens: { refresh_token: 'r' } }), + /token refresh failed/, + ); + } finally { + globalThis.fetch = prevFetch; + } +}); + +test('refreshCodexToken throws when no refresh_token', async () => { + await assert.rejects(() => refreshCodexToken({ tokens: {} }), /no refresh_token/); +}); + test('resolveCodexCredential: chatgpt preferred even when api key present', async () => { const future = Math.floor(Date.now() / 1000) + 3600; const cred = await withCodexHome( diff --git a/src/__tests__/openai-capabilities.test.ts b/src/__tests__/openai-capabilities.test.ts new file mode 100644 index 0000000..9e8bcba --- /dev/null +++ b/src/__tests__/openai-capabilities.test.ts @@ -0,0 +1,32 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { OpenAIProvider } from '../services/providers/openai.js'; + +// Constructors do not hit the network, so these assert the capability contract +// without any API calls. + +test('openai api-key mode: native chaining, no replay', () => { + const p = new OpenAIProvider({ apiKey: 'sk-test' }); + assert.equal(p.capabilities.previousResponseId, true); + assert.equal(p.capabilities.conversationReplay, false); + assert.equal(p.capabilities.toolCalling, true); + assert.equal(p.capabilities.structuredOutputs, true); +}); + +test('openai ChatGPT-login mode: replay continuity, no native chaining', () => { + const p = new OpenAIProvider({ chatgpt: { accessToken: 'tok', accountId: 'acct' } }); + assert.equal(p.capabilities.previousResponseId, false); + assert.equal(p.capabilities.conversationReplay, true); + assert.equal(p.capabilities.toolCalling, true); + assert.equal(p.capabilities.structuredOutputs, true); +}); + +test('openai constructor without any credential throws', () => { + const prev = process.env.OPENAI_API_KEY; + delete process.env.OPENAI_API_KEY; + try { + assert.throws(() => new OpenAIProvider({}), /No OpenAI credential/); + } finally { + if (prev !== undefined) process.env.OPENAI_API_KEY = prev; + } +}); diff --git a/src/__tests__/reviewer-continuity.test.ts b/src/__tests__/reviewer-continuity.test.ts new file mode 100644 index 0000000..521e74c --- /dev/null +++ b/src/__tests__/reviewer-continuity.test.ts @@ -0,0 +1,77 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync, readFileSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + continuityPlan, + storeConversation, + getConversationHistory, + __resetConversationStoreForTest, +} from '../services/reviewer.js'; + +const caps = (previousResponseId: boolean, conversationReplay: boolean) => ({ + previousResponseId, + conversationReplay, +}); + +// --- continuityPlan (pure) --- + +test('continuityPlan: no previousReviewId → nothing to do', () => { + assert.deepEqual(continuityPlan(caps(false, false), false), { shouldLoad: false, shouldWarn: false }); + assert.deepEqual(continuityPlan(caps(true, false), false), { shouldLoad: false, shouldWarn: false }); +}); + +test('continuityPlan: native chaining (OpenAI api-key) → no load, no warn', () => { + assert.deepEqual(continuityPlan(caps(true, false), true), { shouldLoad: false, shouldWarn: false }); +}); + +test('continuityPlan: replay provider (Anthropic / ChatGPT login) → load, no warn', () => { + assert.deepEqual(continuityPlan(caps(false, true), true), { shouldLoad: true, shouldWarn: false }); +}); + +test('continuityPlan: no continuity support (Google) → warn only', () => { + assert.deepEqual(continuityPlan(caps(false, false), true), { shouldLoad: false, shouldWarn: true }); +}); + +// --- conversation store roundtrip + workspace isolation --- + +const turn = (text: string) => [{ role: 'user' as const, content: [{ type: 'input_text', text }] }]; + +test('store/load conversation roundtrip within a workspace', async () => { + __resetConversationStoreForTest(); + const ws = mkdtempSync(join(tmpdir(), 'duul-ws-')); + try { + await storeConversation('rev-1', turn('hello'), ws); + const loaded = await getConversationHistory('rev-1', ws); + assert.equal(loaded?.length, 1); + // Flushed to disk under /.duul/conversations.json + assert.ok(existsSync(join(ws, '.duul', 'conversations.json'))); + } finally { + __resetConversationStoreForTest(); + rmSync(ws, { recursive: true, force: true }); + } +}); + +test('workspace switch does not leak conversations across workspaces', async () => { + __resetConversationStoreForTest(); + const wsA = mkdtempSync(join(tmpdir(), 'duul-wsA-')); + const wsB = mkdtempSync(join(tmpdir(), 'duul-wsB-')); + try { + // Populate workspace A and read it (sets the store's active workspace to A). + await storeConversation('a-1', turn('secret-A'), wsA); + await getConversationHistory('a-1', wsA); + + // Switch to workspace B: a read must clear A's entries before touching B. + await getConversationHistory('missing', wsB); + await storeConversation('b-1', turn('data-B'), wsB); + + const bFile = JSON.parse(readFileSync(join(wsB, '.duul', 'conversations.json'), 'utf-8')); + assert.ok('b-1' in bFile, "B's own entry is present"); + assert.ok(!('a-1' in bFile), "A's entry must NOT bleed into B's file"); + } finally { + __resetConversationStoreForTest(); + rmSync(wsA, { recursive: true, force: true }); + rmSync(wsB, { recursive: true, force: true }); + } +}); diff --git a/src/services/providers/anthropic.ts b/src/services/providers/anthropic.ts index b63d29a..d699aee 100644 --- a/src/services/providers/anthropic.ts +++ b/src/services/providers/anthropic.ts @@ -189,7 +189,7 @@ export class AnthropicProvider implements ReviewerProvider { readonly capabilities: ProviderCapabilities = { structuredOutputs: false, toolCalling: true, - previousResponseId: true, // simulated via conversation history + previousResponseId: false, // no native chaining — continuity via replay conversationReplay: true, jsonSchemaStrict: false, }; diff --git a/src/services/providers/codex-auth.ts b/src/services/providers/codex-auth.ts index 76d8e8c..82fb4d7 100644 --- a/src/services/providers/codex-auth.ts +++ b/src/services/providers/codex-auth.ts @@ -15,7 +15,7 @@ * Protocol constants mirror the openai/codex `codex-rs` client so DUUL speaks * the same dialect the CLI does. */ -import { readFileSync, writeFileSync } from 'node:fs'; +import { readFileSync, writeFileSync, chmodSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; @@ -141,7 +141,12 @@ export async function refreshCodexToken(auth: CodexAuth): Promise { }; try { - writeFileSync(authPath(), JSON.stringify(updated, null, 2), { mode: 0o600 }); + const path = authPath(); + writeFileSync(path, JSON.stringify(updated, null, 2), { mode: 0o600 }); + // `mode` only applies when the file is created; force 0600 on overwrite so a + // pre-existing, loosely-permissioned auth.json can't keep the refreshed token + // world/group readable. + chmodSync(path, 0o600); } catch (error) { // Non-fatal: we can still use the refreshed token in-memory this run. console.error(`[duul] Warning: could not persist refreshed Codex token: ${error instanceof Error ? error.message : error}`); @@ -167,9 +172,23 @@ export async function resolveCodexCredential(): Promise if (preferChatgpt && chatgptCapable) { let accessToken = tokens!.access_token!; - if (isTokenExpired(accessToken) && tokens!.refresh_token) { - const refreshed = await refreshCodexToken(auth); - accessToken = refreshed.tokens?.access_token ?? accessToken; + if (isTokenExpired(accessToken)) { + // Token expired: refresh if possible, otherwise fall back to a stored API + // key rather than handing back a credential that will immediately 401. + if (tokens!.refresh_token) { + try { + const refreshed = await refreshCodexToken(auth); + accessToken = refreshed.tokens?.access_token ?? accessToken; + } catch (error) { + console.error(`[duul] Codex token refresh failed: ${error instanceof Error ? error.message : error}`); + if (auth.OPENAI_API_KEY) return { mode: 'apikey', apiKey: auth.OPENAI_API_KEY }; + throw error; + } + } else if (auth.OPENAI_API_KEY) { + return { mode: 'apikey', apiKey: auth.OPENAI_API_KEY }; + } + // else: no refresh path and no key — proceed with the expired token so the + // provider surfaces a clear auth error (better than a silent null). } return { mode: 'chatgpt', diff --git a/src/services/providers/openai.ts b/src/services/providers/openai.ts index 2b9e6ec..372fb7d 100644 --- a/src/services/providers/openai.ts +++ b/src/services/providers/openai.ts @@ -246,10 +246,9 @@ export class OpenAIProvider implements ReviewerProvider { this.capabilities = { structuredOutputs: true, toolCalling: true, - // Both modes support cross-round continuity: api-key mode natively via - // previous_response_id, ChatGPT mode by replaying conversation turns. - previousResponseId: true, - // ChatGPT backend is stateless — continuity comes from turn replay. + // Native server-side chaining is available only in api-key mode. The + // ChatGPT backend is stateless, so continuity there comes from turn replay. + previousResponseId: !this.stateless, conversationReplay: this.stateless, jsonSchemaStrict: true, }; diff --git a/src/services/providers/types.ts b/src/services/providers/types.ts index 6b2f7de..492295e 100644 --- a/src/services/providers/types.ts +++ b/src/services/providers/types.ts @@ -62,7 +62,7 @@ export interface ProviderCapabilities { structuredOutputs: boolean; /** Supports tool/function calling */ toolCalling: boolean; - /** Supports conversation continuity across rounds (native chaining or replay) */ + /** Supports NATIVE server-side conversation chaining via previous_response_id */ previousResponseId: boolean; /** * Continuity is achieved by replaying prior turns (conversationHistory) rather diff --git a/src/services/reviewer.ts b/src/services/reviewer.ts index 2b7a2bc..91ec60d 100644 --- a/src/services/reviewer.ts +++ b/src/services/reviewer.ts @@ -250,6 +250,11 @@ function conversationsPath(workspaceRoot: string): string { async function loadFromDisk(workspaceRoot: string): Promise { if (diskLoaded && lastWorkspaceRoot === workspaceRoot) return; + // Switching workspaces: drop the previous workspace's entries so they aren't + // flushed into (or replayed from) the new workspace's conversations file. + if (lastWorkspaceRoot !== null && lastWorkspaceRoot !== workspaceRoot) { + memoryCache.clear(); + } lastWorkspaceRoot = workspaceRoot; diskLoaded = true; @@ -298,7 +303,33 @@ function evictOldest(): void { } } -async function getConversationHistory(reviewId: string, workspaceRoot?: string): Promise { +/** + * Decide how to handle cross-round continuity for a provider, given whether the + * caller supplied a previousReviewId. Pure function so it can be unit-tested. + * + * - `shouldLoad`: replay-based providers need prior turns loaded and passed in. + * - `shouldWarn`: the caller asked for continuity but the provider supports + * neither native chaining nor replay, so context will be lost. + */ +export function continuityPlan( + capabilities: { previousResponseId: boolean; conversationReplay: boolean }, + hasPreviousReviewId: boolean, +): { shouldLoad: boolean; shouldWarn: boolean } { + if (!hasPreviousReviewId) return { shouldLoad: false, shouldWarn: false }; + return { + shouldLoad: capabilities.conversationReplay, + shouldWarn: !capabilities.previousResponseId && !capabilities.conversationReplay, + }; +} + +/** Reset the in-memory conversation store. Test-only. */ +export function __resetConversationStoreForTest(): void { + memoryCache.clear(); + diskLoaded = false; + lastWorkspaceRoot = null; +} + +export async function getConversationHistory(reviewId: string, workspaceRoot?: string): Promise { if (workspaceRoot) await loadFromDisk(workspaceRoot); const entry = memoryCache.get(reviewId); if (!entry) return undefined; @@ -306,7 +337,7 @@ async function getConversationHistory(reviewId: string, workspaceRoot?: string): return entry.turns; } -async function storeConversation(reviewId: string, turns: ConversationTurn[], workspaceRoot?: string): Promise { +export async function storeConversation(reviewId: string, turns: ConversationTurn[], workspaceRoot?: string): Promise { evictOldest(); memoryCache.set(reviewId, { turns, lastAccessed: Date.now() }); if (workspaceRoot) { @@ -330,7 +361,8 @@ export async function callReview( 'Reviewer will not be able to explore the workspace. Consider providing more context via relevant_code/artifact_refs.', ); } - if (!provider.capabilities.previousResponseId && !provider.capabilities.conversationReplay && options.previousReviewId) { + const plan = continuityPlan(provider.capabilities, !!options.previousReviewId); + if (plan.shouldWarn) { console.error( `[duul] Warning: ${provider.name} provider does not support conversation continuity. ` + 'Reviewer context from previous rounds will not be available.', @@ -343,8 +375,8 @@ export async function callReview( // OpenAI ChatGPT-login backend). Native-chaining providers (OpenAI api-key) // pass previousReviewId straight through and don't need replay. let conversationHistory: ConversationTurn[] | undefined; - if (options.previousReviewId && provider.capabilities.conversationReplay) { - conversationHistory = await getConversationHistory(options.previousReviewId, workspaceRoot); + if (plan.shouldLoad) { + conversationHistory = await getConversationHistory(options.previousReviewId!, workspaceRoot); if (conversationHistory) { console.error(`[duul] Loaded conversation history for ${options.previousReviewId} (${conversationHistory.length} turns)`); } else {