feat: support OpenAI Codex OAuth subscription flow#73
Conversation
- add openai-codex provider with token resolution from ~/.codex/auth.json\n- add 'aceclaw models auth login --provider openai-codex' command\n- improve 401 message for missing api.responses.write scope\n- update docs/dev script and add provider tests\n\nRefs #72
When ACECLAW_PROVIDER is explicitly set and no matching profile exists, do not apply defaultProfile. This prevents unrelated model/apiKey from leaking into provider-specific runs (e.g. openai-codex via dev.sh).\n\nRefs #72
- switch openai-codex default base URL to https://chatgpt.com/backend-api/codex\n- add config fallback to load token from ~/.codex/auth.json for provider=openai-codex\n- update provider docs with new default endpoint\n\nRefs #72
- skip temperature and max_output_tokens for provider=openai-codex\n- keep non-codex providers behavior unchanged\n- add mapper tests for codex vs non-codex request options\n\nRefs #72
- document codex backend request constraints\n- note openai-codex ignores temperature and max tokens\n\nRefs #72
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis PR adds an openai-codex provider: OAuth token discovery (including ~/.codex/auth.json), CLI login flows, provider-aware client wiring/routing, Responses API request semantics for Codex (omit temperature/max_output_tokens, include store=false), and improved 401 handling for missing responses.write scope. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as AceClawMain (CLI)
participant AuthFile as ~/.codex/auth.json (FS)
participant Env as Environment (OPENAI_API_KEY)
participant Config as AceClawConfig (Daemon)
participant TokenProv as OpenAiCodexTokenProvider
User->>CLI: aceclaw models auth login --provider openai-codex
CLI->>TokenProv: hasCodexAccessToken() / check files/env
alt token present
CLI->>User: report token present
else
CLI->>User: run external `codex auth login`
User->>AuthFile: external CLI writes token
CLI->>AuthFile: verify token exists
CLI->>User: confirm login
end
User->>Config: start daemon with provider=openai-codex
Config->>TokenProv: resolve token (configured -> auth.json -> env)
TokenProv->>AuthFile: read/parse auth.json
alt tokens.access_token present
TokenProv->>Config: return access_token
else alt OPENAI_API_KEY present
TokenProv->>Config: return OPENAI_API_KEY
else
TokenProv->>Env: check env var
TokenProv->>Config: return env value or "not-configured"
end
Config->>Config: apply profile and set apiKey
sequenceDiagram
participant Client as LLM Client
participant Factory as LlmClientFactory
participant Router as OpenAIRoutingClient
participant Responses as OpenAIResponsesClient
participant Mapper as OpenAIResponsesMapper
participant OpenAI as OpenAI Backend
Client->>Factory: create(provider=openai-codex, apiKey,...)
Factory->>Router: instantiate with providerName=openai-codex
Client->>Router: send(request for codex model)
Router->>Responses: route to Responses API
Responses->>Mapper: toRequestJson(..., includeMaxOutputTokens=false, includeTemperature=false, includeStoreFalse=true)
Mapper->>Responses: return JSON (no temperature/max_output_tokens, store=false)
Responses->>OpenAI: POST /backend-api/codex with token
OpenAI->>Responses: response
Responses->>Router: return result
Router->>Client: deliver response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryAdds
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CLI: aceclaw models auth login] -->|--provider openai-codex| B[codex auth login]
B --> C[~/.codex/auth.json]
D[AceClawConfig.load] -->|provider=openai-codex| E{apiKey set?}
E -->|No| F[loadCodexAuthToken]
F --> C
E -->|Yes| G[Use apiKey]
H[LlmClientFactory.create] -->|openai-codex| I[OpenAiCodexTokenProvider]
I -->|1| J[configuredToken]
I -->|2| C
I -->|3| K[OPENAI_API_KEY env]
I -->|4| L[not-configured]
H --> M[OpenAIRoutingClient]
M -->|codex model| N[OpenAIResponsesClient]
M -->|non-codex model| O[OpenAICompatClient]
N --> P[OpenAIResponsesMapper]
P -->|openai-codex| Q[omit temperature, max_output_tokens, add store=false]
P -->|openai/copilot| R[include temperature, max_output_tokens, store=false ⚠️]
Last reviewed commit: 8a0c35e |
| // Codex backend requires store=false for authenticated subscription flow. | ||
| root.put("store", false); |
There was a problem hiding this comment.
store=false applied to all Responses API providers, not just Codex
This store=false is added unconditionally inside toRequestJson, which is used by all three providers that go through the Responses API: openai, copilot, and openai-codex. Before this PR, no store field was sent for the openai or copilot providers (which defaults to true on the OpenAI side).
The comment says "Codex backend requires store=false" but the implementation applies it globally. For the standard openai provider, this silently opts out of response storage (used for fine-tuning, usage dashboards, etc.). For copilot, the Copilot API may or may not honor this field, but it's still a behavioral change.
Consider making store conditional, similar to how temperature and max_output_tokens are handled — either pass a boolean includeStore parameter to the mapper, or gate it on the provider name in OpenAIResponsesClient.
| // Codex backend requires store=false for authenticated subscription flow. | |
| root.put("store", false); | |
| // Codex backend requires store=false for authenticated subscription flow. | |
| // Note: only set store=false when explicitly needed; standard OpenAI defaults store=true. | |
| if (!includeMaxOutputTokens) { | |
| // openai-codex omits max_output_tokens AND requires store=false | |
| root.put("store", false); | |
| } |
Context Used: Context from dashboard - CLAUDE.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesMapper.java
Line: 87-88
Comment:
**`store=false` applied to all Responses API providers, not just Codex**
This `store=false` is added unconditionally inside `toRequestJson`, which is used by all three providers that go through the Responses API: `openai`, `copilot`, and `openai-codex`. Before this PR, no `store` field was sent for the `openai` or `copilot` providers (which defaults to `true` on the OpenAI side).
The comment says "Codex backend requires store=false" but the implementation applies it globally. For the standard `openai` provider, this silently opts out of response storage (used for fine-tuning, usage dashboards, etc.). For `copilot`, the Copilot API may or may not honor this field, but it's still a behavioral change.
Consider making `store` conditional, similar to how `temperature` and `max_output_tokens` are handled — either pass a `boolean includeStore` parameter to the mapper, or gate it on the provider name in `OpenAIResponsesClient`.
```suggestion
// Codex backend requires store=false for authenticated subscription flow.
// Note: only set store=false when explicitly needed; standard OpenAI defaults store=true.
if (!includeMaxOutputTokens) {
// openai-codex omits max_output_tokens AND requires store=false
root.put("store", false);
}
```
**Context Used:** Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=7788bd8f-ef8a-482c-b00b-a1259a687856))
How can I resolve this? If you propose a fix, please make it concise.| /** | ||
| * Converts an {@link LlmRequest} to Responses API JSON body with provider-specific options. | ||
| * | ||
| * @param includeMaxOutputTokens whether to include {@code max_output_tokens} | ||
| */ |
There was a problem hiding this comment.
Incomplete Javadoc: missing @param includeTemperature
The Javadoc documents includeMaxOutputTokens but omits includeTemperature.
| /** | |
| * Converts an {@link LlmRequest} to Responses API JSON body with provider-specific options. | |
| * | |
| * @param includeMaxOutputTokens whether to include {@code max_output_tokens} | |
| */ | |
| /** | |
| * Converts an {@link LlmRequest} to Responses API JSON body with provider-specific options. | |
| * | |
| * @param includeMaxOutputTokens whether to include {@code max_output_tokens} | |
| * @param includeTemperature whether to include {@code temperature} | |
| */ |
Prompt To Fix With AI
This is a comment left during a code review.
Path: aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesMapper.java
Line: 47-51
Comment:
**Incomplete Javadoc: missing `@param includeTemperature`**
The Javadoc documents `includeMaxOutputTokens` but omits `includeTemperature`.
```suggestion
/**
* Converts an {@link LlmRequest} to Responses API JSON body with provider-specific options.
*
* @param includeMaxOutputTokens whether to include {@code max_output_tokens}
* @param includeTemperature whether to include {@code temperature}
*/
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
aceclaw-daemon/src/main/java/dev/aceclaw/daemon/StreamingAgentHandler.java (1)
492-512:⚠️ Potential issue | 🟡 MinorAvoid user-facing
"null"in error text.
messagecan be null, so the 400 and fallback branches may emit"Bad request ... null"or"LLM error: null". Consider a null-safe fallback for better UX.🔧 Suggested tweak
- String message = e.getMessage(); + String message = e.getMessage(); + String safeMessage = (message == null || message.isBlank()) + ? "(no additional details)" : message; if (status == 401 && message != null && message.contains("api.responses.write")) { return "Authentication succeeded but token lacks required scope 'api.responses.write'. " + "Use a full OpenAI API key or provider=openai-codex with a valid Codex OAuth token."; } @@ - } else if (status == 400) { - return "Bad request to LLM API: " + message; + } else if (status == 400) { + return "Bad request to LLM API: " + safeMessage; } else if (message != null && message.contains("not-configured")) { return "API key not configured. Set ANTHROPIC_API_KEY (or OPENAI_API_KEY) or add apiKey to ~/.aceclaw/config.json."; } else { - return "LLM error: " + message; + return "LLM error: " + safeMessage; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/StreamingAgentHandler.java` around lines 492 - 512, The error handling in StreamingAgentHandler uses e.getMessage() (message) which can be null, causing user-facing strings like "null"; update the logic to use a null-safe message (e.g., compute a defaultMessage variable from e.getMessage() or Optional.ofNullable(e.getMessage()).orElse("unknown error") ) and use that defaultMessage in the 400 branch and the final fallback branch so no "null" appears in returned messages; keep the special-case checks (status 401, 429, 529, 5xx, "not-configured") unchanged and only replace usages of message with the null-safe variable.
🧹 Nitpick comments (2)
aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAiCodexTokenProvider.java (1)
65-66: Consider throwing or returningnullinstead of "not-configured".Returning the literal string
"not-configured"as a token will result in a 401 from the API with a generic "Invalid API key" message. This loses the opportunity to provide a clear, actionable error message to the user.Consider returning
nulland handling it upstream to display a helpful message like "No Codex token found. Runaceclaw models auth loginor set OPENAI_API_KEY."♻️ Proposed alternative
- return "not-configured"; + return null;Then handle
nullin the calling code to provide actionable guidance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAiCodexTokenProvider.java` around lines 65 - 66, The method in OpenAiCodexTokenProvider currently returns the literal string "not-configured" as a token; change it to return null (or throw a specific unchecked exception) to avoid passing an invalid token to the API and to allow upstream code to present a clear action message; update the OpenAiCodexTokenProvider method that currently returns "not-configured" so callers can check for null (or catch the new exception) and render: "No Codex token found. Run `aceclaw models auth login` or set OPENAI_API_KEY."aceclaw-cli/src/main/java/dev/aceclaw/cli/AceClawMain.java (1)
149-165: Consider reusingOpenAiCodexTokenProviderto avoid duplication.The
hasCodexAccessToken()logic duplicates the token detection fromOpenAiCodexTokenProvider.loadFromCodexAuthFile(). Changes to the auth.json structure would need updates in both places.♻️ Proposed consolidation
+import dev.aceclaw.llm.openai.OpenAiCodexTokenProvider; + private static boolean hasCodexAccessToken() { - try { - if (!Files.exists(CODEX_AUTH_FILE)) { - return false; - } - var mapper = new ObjectMapper(); - JsonNode root = mapper.readTree(Files.readString(CODEX_AUTH_FILE)); - String accessToken = root.path("tokens").path("access_token").asText(""); - if (!accessToken.isBlank()) { - return true; - } - String legacy = root.path("OPENAI_API_KEY").asText(""); - return !legacy.isBlank(); - } catch (Exception e) { - return false; - } + var provider = new OpenAiCodexTokenProvider(null); + String token = provider.get(); + return token != null && !"not-configured".equals(token); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-cli/src/main/java/dev/aceclaw/cli/AceClawMain.java` around lines 149 - 165, hasCodexAccessToken duplicates token-detection logic; replace it by calling OpenAiCodexTokenProvider.loadFromCodexAuthFile() (or the equivalent public API on OpenAiCodexTokenProvider) and determine token presence from that result instead of re-parsing the file yourself; handle the provider's returned object or Optional and any exceptions it throws, returning true when it indicates an access token or legacy key exists and false on absence or error, thus centralizing auth.json parsing in OpenAiCodexTokenProvider.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/AceClawConfig.java`:
- Around line 456-478: In loadCodexAuthToken(), after calling
mapper.readTree(CODEX_AUTH_FILE.toFile()), check whether the returned JsonNode
(tree) is null or tree.isNull() and return early (or log a debug message) if so
to avoid NPE on the subsequent tree.path(...) calls; keep the rest of the token
extraction (tokens/access_token and OPENAI_API_KEY fallback) unchanged and only
proceed to set this.apiKey and log when a non-blank token is found.
In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/LlmClientFactory.java`:
- Around line 158-171: The createOpenAiCodexClient function is using
"/v1/chat/completions" and "/v1/responses" which are incorrect for the ChatGPT
backend-api Codex base URL; update the endpoint paths passed to
OpenAICompatClient and OpenAIResponsesClient to remove the "/v1/" prefix (use
"/chat/completions" and "/responses") so the calls constructed against the
resolvedBaseUrl (e.g., DEFAULT_BASE_URLS.get("openai-codex") like
https://chatgpt.com/backend-api/codex) match the backend-api structure used by
OpenAIRoutingClient/OpenAICompatClient/OpenAIResponsesClient.
In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesMapper.java`:
- Around line 87-88: OpenAIResponsesMapper currently always sets "store" to
false (root.put("store", false)); change the mapper to accept a configurable
store flag (e.g., a boolean parameter named store or includeStore) and only add
root.put("store", store) when the flag is provided/true so callers can opt into
storage; update the mapper's public factory/mapping method signature (the method
that constructs the root JSON in OpenAIResponsesMapper) to accept this new
parameter and ensure any callers are updated to pass the desired store behavior
(defaulting to true if not supplied).
---
Outside diff comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/StreamingAgentHandler.java`:
- Around line 492-512: The error handling in StreamingAgentHandler uses
e.getMessage() (message) which can be null, causing user-facing strings like
"null"; update the logic to use a null-safe message (e.g., compute a
defaultMessage variable from e.getMessage() or
Optional.ofNullable(e.getMessage()).orElse("unknown error") ) and use that
defaultMessage in the 400 branch and the final fallback branch so no "null"
appears in returned messages; keep the special-case checks (status 401, 429,
529, 5xx, "not-configured") unchanged and only replace usages of message with
the null-safe variable.
---
Nitpick comments:
In `@aceclaw-cli/src/main/java/dev/aceclaw/cli/AceClawMain.java`:
- Around line 149-165: hasCodexAccessToken duplicates token-detection logic;
replace it by calling OpenAiCodexTokenProvider.loadFromCodexAuthFile() (or the
equivalent public API on OpenAiCodexTokenProvider) and determine token presence
from that result instead of re-parsing the file yourself; handle the provider's
returned object or Optional and any exceptions it throws, returning true when it
indicates an access token or legacy key exists and false on absence or error,
thus centralizing auth.json parsing in OpenAiCodexTokenProvider.
In
`@aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAiCodexTokenProvider.java`:
- Around line 65-66: The method in OpenAiCodexTokenProvider currently returns
the literal string "not-configured" as a token; change it to return null (or
throw a specific unchecked exception) to avoid passing an invalid token to the
API and to allow upstream code to present a clear action message; update the
OpenAiCodexTokenProvider method that currently returns "not-configured" so
callers can check for null (or catch the new exception) and render: "No Codex
token found. Run `aceclaw models auth login` or set OPENAI_API_KEY."
| private void loadCodexAuthToken() { | ||
| if (!Files.isRegularFile(CODEX_AUTH_FILE)) { | ||
| return; | ||
| } | ||
| try { | ||
| var mapper = new ObjectMapper(); | ||
| var tree = mapper.readTree(CODEX_AUTH_FILE.toFile()); | ||
|
|
||
| String token = null; | ||
| var tokens = tree.path("tokens"); | ||
| if (tokens.has("access_token")) { | ||
| token = tokens.get("access_token").asText(null); | ||
| } | ||
| if ((token == null || token.isBlank()) && tree.has("OPENAI_API_KEY")) { | ||
| token = tree.get("OPENAI_API_KEY").asText(null); | ||
| } | ||
| if (token != null && !token.isBlank()) { | ||
| this.apiKey = token; | ||
| log.info("Loaded OpenAI Codex access token from {}", CODEX_AUTH_FILE); | ||
| } | ||
| } catch (IOException e) { | ||
| log.debug("Could not read Codex auth file {}: {}", CODEX_AUTH_FILE, e.getMessage()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n aceclaw-daemon/src/main/java/dev/aceclaw/daemon/AceClawConfig.java | sed -n '450,485p'Repository: xinhuagu/AceClaw
Length of output: 1604
🏁 Script executed:
# Search for other uses of ObjectMapper and readTree in the codebase
rg "readTree" --type java -B 2 -A 2Repository: xinhuagu/AceClaw
Length of output: 50372
🌐 Web query:
Jackson ObjectMapper readTree null return behavior JsonNode
💡 Result:
ObjectMapper.readTree(...) (and ObjectCodec#readTree(JsonParser)) returns Java null only when there is no JSON content to read—i.e., the parser is at end-of-input / no more tokens. It does not return Java null for a JSON literal null; JSON null is represented as a non-null JsonNode where node.isNull() is true (a NullNode). [1][2]
This commonly shows up in custom deserializers when you do:
defaultDeserializer.deserialize(p, ctxt); // consumes all tokens for the value
JsonNode node = p.getCodec().readTree(p); // now at end => returns nullAfter the default deserializer has consumed the JSON, the parser has “no more content accessible”, so readTree(p) returns null. [3][4]
How to handle it (typical fixes):
- Read the tree first, then deserialize from that tree by creating a new parser (so you don’t read the same
JsonParsertwice), e.g. viaTreeTraversingParser. [4] - Or buffer the token stream with a
TokenBufferand create two parsers from it (one for default deserialization, one for tree access). [4]
Rule of thumb: if you need both “default POJO deserialization” and “tree access”, you must buffer (tree or token buffer) because a JsonParser is forward-only. [4]
Sources:
[1] Jackson ObjectMapper#readTree Javadoc (return null on no content; JSON null => JsonNode.isNull())
[2] ObjectMapper.readValue() returns null only for JSON literal null (Stack Overflow)
[3] ObjectMapper.readTree(JsonParser) implementation notes: returns null if no more content accessible
[4] “Use JsonParser again after deserialization” solution: read tree first / use TreeTraversingParser or TokenBuffer (Stack Overflow)
Add null check after readTree() to prevent NPE on empty or malformed auth.json.
If the auth file is empty, mapper.readTree() returns null, causing an uncaught NullPointerException at tree.path(). Additionally, check tree.isNull() to handle JSON null content gracefully.
Proposed fix
try {
var mapper = new ObjectMapper();
var tree = mapper.readTree(CODEX_AUTH_FILE.toFile());
+ if (tree == null || tree.isNull()) {
+ log.warn("Codex auth file {} is empty or null JSON", CODEX_AUTH_FILE);
+ return;
+ }
String token = null;
var tokens = tree.path("tokens");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/AceClawConfig.java` around
lines 456 - 478, In loadCodexAuthToken(), after calling
mapper.readTree(CODEX_AUTH_FILE.toFile()), check whether the returned JsonNode
(tree) is null or tree.isNull() and return early (or log a debug message) if so
to avoid NPE on the subsequent tree.path(...) calls; keep the rest of the token
extraction (tokens/access_token and OPENAI_API_KEY fallback) unchanged and only
proceed to set this.apiKey and log when a non-blank token is found.
| private static LlmClient createOpenAiCodexClient(String apiKey, String baseUrl, String model) { | ||
| var tokenProvider = new OpenAiCodexTokenProvider(apiKey); | ||
| String resolvedBaseUrl = baseUrl != null ? baseUrl : DEFAULT_BASE_URLS.get("openai-codex"); | ||
| String resolvedModel = model != null ? model : DEFAULT_MODELS.getOrDefault("openai-codex", "gpt-5-codex"); | ||
|
|
||
| var chatClient = new OpenAICompatClient( | ||
| tokenProvider, resolvedBaseUrl, "/v1/chat/completions", | ||
| "openai-codex", resolvedModel, ProviderCapabilities.OPENAI, Map.of()); | ||
| var responsesClient = new OpenAIResponsesClient( | ||
| tokenProvider, resolvedBaseUrl, "/v1/responses", | ||
| "openai-codex", resolvedModel, ProviderCapabilities.CODEX, Map.of()); | ||
|
|
||
| return new OpenAIRoutingClient(chatClient, responsesClient, resolvedModel, "openai-codex"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for backend-api/codex references and related endpoint usage
rg -n "backend-api/codex" --type java -C 3
rg -n "codex" -i --type java | grep -E "endpoint|path|url|/v1" | head -20Repository: xinhuagu/AceClaw
Length of output: 945
🌐 Web query:
OpenAI Codex backend API endpoint paths documentation
💡 Result:
Codex doesn’t have a separate “Codex API” with unique paths; Codex clients (e.g., Codex CLI) call the Responses API. The relevant documented endpoint paths are:
# OpenAI API (API-key auth)
POST https://api.openai.com/v1/responses
This is the standard endpoint Codex CLI uses when authenticated with an API key. [1]
# ChatGPT login (Codex CLI / ChatGPT-backed)
POST https://chatgpt.com/backend-api/codex/responses
This is the ChatGPT “backend-api” path used by Codex CLI when you log in with ChatGPT (it’s referenced in OpenAI’s Codex agent loop write-up, not as a general public API reference). [1]
Additional related API path docs (model pages list supported endpoints) include:
/v1/chat/completions
/v1/realtime
/v1/assistants
/v1/batch
(plus /v1/responses above). [2]
Sources: OpenAI “Unrolling the Codex agent loop” (Jan 23, 2026) [1]; OpenAI model docs showing supported endpoints for Codex-capable models [2].
Fix incorrect endpoint paths for ChatGPT backend-api Codex client.
The base URL https://chatgpt.com/backend-api/codex does not use /v1/ prefixed paths. According to OpenAI documentation, the ChatGPT backend-api Codex endpoint is POST https://chatgpt.com/backend-api/codex/responses, not /v1/responses. Similarly, the chat completions path should be adjusted to match the backend-api structure (likely /chat/completions without the /v1/ prefix). Update the endpoint paths in createOpenAiCodexClient() to remove the /v1/ prefix when appending to the backend-api base URL.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/LlmClientFactory.java` around lines
158 - 171, The createOpenAiCodexClient function is using "/v1/chat/completions"
and "/v1/responses" which are incorrect for the ChatGPT backend-api Codex base
URL; update the endpoint paths passed to OpenAICompatClient and
OpenAIResponsesClient to remove the "/v1/" prefix (use "/chat/completions" and
"/responses") so the calls constructed against the resolvedBaseUrl (e.g.,
DEFAULT_BASE_URLS.get("openai-codex") like
https://chatgpt.com/backend-api/codex) match the backend-api structure used by
OpenAIRoutingClient/OpenAICompatClient/OpenAIResponsesClient.
- make Responses mapper include store=false only when requested\n- pass store=false only for provider=openai-codex\n- add mapper regression assertions for codex/non-codex store behavior\n\nRefs #72
- add null/blank-safe fallback for LlmException messages\n- use safe message in HTTP 400 and generic fallback branches\n\nRefs #72
|
Addressed CodeRabbit feedback in commit
For the remaining two nitpicks, keeping current behavior for now:
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesClient.java (1)
208-218: Consider extracting the repeated provider name to a constant.The string
"openai-codex"is repeated in three helper methods. Extracting it to aprivate static final Stringconstant would improve maintainability and reduce typo risk.♻️ Suggested refactor
+ private static final String OPENAI_CODEX_PROVIDER = "openai-codex"; + private boolean shouldIncludeMaxOutputTokens() { - return !"openai-codex".equals(providerName); + return !OPENAI_CODEX_PROVIDER.equals(providerName); } private boolean shouldIncludeTemperature() { - return !"openai-codex".equals(providerName); + return !OPENAI_CODEX_PROVIDER.equals(providerName); } private boolean shouldIncludeStoreFalse() { - return "openai-codex".equals(providerName); + return OPENAI_CODEX_PROVIDER.equals(providerName); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesClient.java` around lines 208 - 218, The three helper methods in OpenAIResponsesClient (shouldIncludeMaxOutputTokens, shouldIncludeTemperature, shouldIncludeStoreFalse) repeat the literal "openai-codex"; extract that literal into a private static final String constant (e.g., OPENAI_CODEX_PROVIDER) and replace the inline string comparisons with the constant to improve maintainability and avoid typos, leaving the method logic unchanged.aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesMapper.java (1)
47-55: Incomplete Javadoc forincludeTemperatureparameter.The Javadoc at line 50-51 only documents
includeMaxOutputTokensbut omitsincludeTemperature. Same issue in the 5-param overload (lines 60-61).📝 Suggested Javadoc fix
/** * Converts an {`@link` LlmRequest} to Responses API JSON body with provider-specific options. * * `@param` includeMaxOutputTokens whether to include {`@code` max_output_tokens} + * `@param` includeTemperature whether to include {`@code` temperature} */ String toRequestJson(LlmRequest request, boolean stream, boolean includeMaxOutputTokens, boolean includeTemperature) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesMapper.java` around lines 47 - 55, The Javadoc for the toRequestJson methods omits documentation for the includeTemperature parameter; update the Javadoc for both overloads of toRequestJson(LlmRequest request, boolean stream, boolean includeMaxOutputTokens, boolean includeTemperature) and toRequestJson(..., boolean includeTemperature, boolean includeNop) to add a `@param` entry describing includeTemperature (e.g., whether temperature is included in the generated JSON and what effect true/false has), and ensure the param descriptions match the existing style and align with includeMaxOutputTokens wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesClient.java`:
- Around line 208-218: The three helper methods in OpenAIResponsesClient
(shouldIncludeMaxOutputTokens, shouldIncludeTemperature,
shouldIncludeStoreFalse) repeat the literal "openai-codex"; extract that literal
into a private static final String constant (e.g., OPENAI_CODEX_PROVIDER) and
replace the inline string comparisons with the constant to improve
maintainability and avoid typos, leaving the method logic unchanged.
In `@aceclaw-llm/src/main/java/dev/aceclaw/llm/openai/OpenAIResponsesMapper.java`:
- Around line 47-55: The Javadoc for the toRequestJson methods omits
documentation for the includeTemperature parameter; update the Javadoc for both
overloads of toRequestJson(LlmRequest request, boolean stream, boolean
includeMaxOutputTokens, boolean includeTemperature) and toRequestJson(...,
boolean includeTemperature, boolean includeNop) to add a `@param` entry describing
includeTemperature (e.g., whether temperature is included in the generated JSON
and what effect true/false has), and ensure the param descriptions match the
existing style and align with includeMaxOutputTokens wording.
Summary
openai-codexprovider mode that reuses Codex OAuth token from~/.codex/auth.jsonhttps://chatgpt.com/backend-api/codex)aceclaw models auth login --provider openai-codexFixes in this branch
ACECLAW_PROVIDERis setopenai-codex, omit unsupportedmax_output_tokensandtemperatureCloses #72
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests