Skip to content

feat: Codex support#8118

Open
thewulf7 wants to merge 6 commits into
janhq:mainfrom
thewulf7:thewulf7/codex
Open

feat: Codex support#8118
thewulf7 wants to merge 6 commits into
janhq:mainfrom
thewulf7:thewulf7/codex

Conversation

@thewulf7

@thewulf7 thewulf7 commented May 5, 2026

Copy link
Copy Markdown
Contributor

…dling functions

Describe Your Changes

image image

thewulf7 and others added 4 commits May 5, 2026 09:44
…eserve bare string shorthand

Co-authored-by: Copilot <copilot@github.com>
…tings

- Implemented JanCodeRecommendation component for model selection and download management.
- Created SettingsIntegrationPage for better organization of settings UI.
- Added Codex integration route and settings management with local storage persistence.
- Enhanced SettingsMenu to include Codex option.
- Developed useCodexSettings hook for managing Codex model settings.
- Updated Claude Code integration to utilize new components and improve UI.
- Added tests for useCodexSettings to ensure functionality.

Co-authored-by: Copilot <copilot@github.com>
…ma to handle bare string shorthand

Co-authored-by: Copilot <copilot@github.com>
@thewulf7 thewulf7 changed the title feat(proxy): adding support for openai v1/responses api feat: Codex support May 5, 2026
Co-authored-by: thewulf7 <1582808+thewulf7@users.noreply.github.com>
@tokamak-pm

tokamak-pm Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review: feat: Codex support

Summary

This is a large PR (17 files, ~2200 additions, ~500 deletions) that adds OpenAI Codex CLI integration to Jan. The changes span three major areas:

  1. Rust backend (proxy.rs, commands.rs): Adds /responses endpoint support to the proxy server, including normalization of Codex-specific input items (local_shell_call, custom_tool_call, reasoning, etc.), and Tauri commands to write/clear Codex config.toml.
  2. Frontend (codex.tsx, hooks, containers): New settings page for Codex integration with model selection, shared UI components (IntegrationModelSelector, JanCodeRecommendation, SettingsIntegrationPage) extracted from the Claude Code page.
  3. Refactoring: Extracts the model selector and Jan-Code recommendation components from the Claude Code page into reusable containers.

Strengths

  • Thorough proxy normalization: The pipeline handles a wide variety of Codex-specific input item types (local_shell_call, custom_tool_call, tool_search_call, reasoning with encrypted content, web_search_call, compaction, etc.) and maps them to standard function_call/function_call_output types that llama.cpp can consume.
  • Good test coverage for proxy logic: The Rust tests cover schema normalization, bare string wrapping, input patching, reasoning fallback, unsupported item filtering, model fallback, and header forwarding.
  • Smart model fallback: The and functions handle the case where Codex requests a model that is not locally available, falling back to whatever model is currently loaded.
  • Clean refactoring: Extracting IntegrationModelSelector, JanCodeRecommendation, and SettingsIntegrationPage from the Claude Code page reduces duplication and makes both integration pages more maintainable.
  • Config management: The write_codex_config/clear_codex_config commands properly handle TOML parsing, preserve existing unrelated config, and clean up only Jan-owned entries.

Issues and Concerns

  1. Schema normalization is duplicated (proxy.rs): and have nearly identical logic but are implemented as separate functions. The main difference is that the tool parameters version uses tracking while the responses version does not. Consider unifying them to avoid divergence bugs.

  2. Content-Length not forwarded (proxy.rs): The new function filters out in addition to the previously filtered and . This is generally correct since the body may be rewritten (changing its length), but there is no corresponding logic to add the correct Content-Length back. The HTTP client (reqwest) likely handles this automatically, but this should be verified.

  3. System message collapsing may lose ordering context (proxy.rs, ): The function collects all system/developer messages and instructions into a single leading system message, joined by "\n\n". This could alter the semantic meaning if system messages were intentionally placed between user/assistant turns. For standard Codex usage this is likely fine, but it is a lossy transformation.

  4. No error handling for missing model on /responses (proxy.rs): If returns , the response is 503 with a plain text body. Other error paths in the proxy return JSON-formatted errors. This inconsistency could confuse Codex clients.

  5. Codex config path security (commands.rs): reads from the environment. If this points to a symlinked or unexpected location, writing config there could be a concern. The and calls do not validate the path. This is low risk since this runs as the user's own process.

  6. Hardcoded context window values (commands.rs): and are hardcoded. Different local models have very different context windows (e.g., a 7B model might only handle 8K). These values should ideally be derived from the selected model's actual context length.

  7. IntegrationModelSelector type casting (IntegrationModelSelector.tsx): The component uses in several places when calling . This suggests a type mismatch that should be properly resolved rather than suppressed.

  8. Missing newline at end of file in several new files (IntegrationModelSelector.tsx, JanCodeRecommendation.tsx, SettingsIntegrationPage.tsx, useCodexSettings.ts, codex.tsx).

  9. No Codex-specific proxy tests for the full pipeline: While individual normalization functions are well-tested, there are no integration tests for the full proxy flow (receiving a Codex-style request, normalizing it, forwarding to llama.cpp, and returning the response).

  10. PR description is minimal: For a 2200-line PR touching Rust backend, frontend, and config management, the description only contains screenshots. A more detailed description of the architecture decisions, Codex wire protocol handling, and testing approach would help reviewers.

Architecture Notes

The approach of normalizing Codex /responses API requests into something llama.cpp can handle at the proxy level is sound. The alternative would be to implement /responses support in the downstream server, which is much more work. The proxy transformation layer is a pragmatic choice.

Merge Readiness

This is a substantial feature PR with good test coverage for the critical proxy normalization logic. The issues identified are mostly about code quality and edge cases rather than correctness. The hardcoded context window values and the duplicated schema normalization are the most important items to address before merge.

Recommendation: improve needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants