feat: add Command Code OAuth provider - #1066
Conversation
Review readiness checklistThis PR is kept in draft until every requirement below is fulfilled. The tickable checklist has been added to your PR description — tick all four boxes there.
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
|
This pull request mentions @hanbinnoh Please add a screenshot of the UI change to the description — drag and drop the image into the description editor, or paste a markdown image such as ⏳ Review readiness checklist This pull request stays in draft until all four boxes of the readiness checklist in the description are ticked (currently 0/4). @hanbinnoh Tick the boxes once your local CI is green, your branch is on the latest This pull request is being kept as a draft automatically. Once every issue above is resolved, it will be marked ready for review again. |
github-actions
Bot
added
enhancement
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesCommand Code is now a first-class OAuth provider. The change adds local CLI credential import, browser OAuth, live model discovery, authenticated agent-mode streaming, adapter routing, provider icons, and comprehensive tests. Command Code provider
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OAuthController
participant CommandCodeOAuth
participant CommandCodeCallbackServer
OAuthController->>CommandCodeOAuth: start loginCommandCode()
CommandCodeOAuth->>CommandCodeCallbackServer: start callback server
CommandCodeCallbackServer->>CommandCodeOAuth: return validated callback
CommandCodeOAuth->>OAuthController: return OAuthCredentials
sequenceDiagram
participant ProviderAdapter
participant CommandCodeAgentModeEndpoint
participant NDJSONParser
ProviderAdapter->>CommandCodeAgentModeEndpoint: send authenticated streaming request
CommandCodeAgentModeEndpoint->>NDJSONParser: stream NDJSON events
NDJSONParser->>ProviderAdapter: emit normalized adapter events
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/provider-icons.ts`:
- Around line 66-67: Move the “Command Code” display text out of
PROVIDER_DISPLAY_NAMES by adding one shared key to PROVIDER_DISPLAY_NAME_KEYS,
then define that key’s localized value in each locale file and reference it
wherever both command-code provider identifiers are displayed.
In `@src/adapters/command-code.ts`:
- Around line 61-75: Update createCommandCodeAdapter and commandCodeConfig to
resolve process.cwd() once inside a guarded flow with a safe fallback, then
reuse that value everywhere request metadata is built. The issue is that
commandCodeConfig and the x-project-slug path both call process.cwd() outside
the existing readdirSync protection, so a removed or renamed working directory
can still throw ENOENT. Fix it by centralizing the cwd lookup in the adapter,
omitting directory-derived fields such as workingDir and x-project-slug when the
cwd is unavailable, and keeping the rest of the request construction unchanged.
In `@src/oauth/command-code.ts`:
- Around line 112-131: Update loginCommandCode so it checks ctrl.signal?.aborted
before any local credential import or createCallbackServer work, and immediately
rejects or throws using the signal’s existing reason instead of proceeding to
the timeout path. Keep the current abort listener for in-flight cancellations,
but add a regression test that invokes loginCommandCode with an already-aborted
signal and verifies it fails right away without leaving the callback server
waiting for LOGIN_TIMEOUT_MS.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d55d8757-991c-40b0-a675-cf19264bf213
📒 Files selected for processing (9)
gui/src/provider-icons.tssrc/adapters/command-code.tssrc/oauth/command-code.tssrc/oauth/index.tssrc/providers/registry.tssrc/server/adapter-resolve.tstests/command-code-provider.test.tstests/provider-registry-parity.test.tstests/provider-workspace-data.test.ts
| "command-code": "Command Code", | ||
| commandcode: "Command Code", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Move the display names into the locale files.
Lines 66-67 add user-visible strings to PROVIDER_DISPLAY_NAMES. Add one shared key to PROVIDER_DISPLAY_NAME_KEYS and define its value in the locale files instead.
As per path instructions, “user-visible strings go through the i18n locale files rather than hardcoded text.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/provider-icons.ts` around lines 66 - 67, Move the “Command Code”
display text out of PROVIDER_DISPLAY_NAMES by adding one shared key to
PROVIDER_DISPLAY_NAME_KEYS, then define that key’s localized value in each
locale file and reference it wherever both command-code provider identifiers are
displayed.
Source: Path instructions
| function commandCodeConfig(): Record<string, unknown> { | ||
| let structure: string[] = []; | ||
| try { structure = readdirSync(process.cwd()).filter(name => !name.startsWith(".")); } catch { /* cwd may disappear */ } | ||
| return { | ||
| workingDir: process.cwd(), | ||
| date: new Date().toISOString().slice(0, 10), | ||
| environment: process.platform, | ||
| structure, | ||
| isGitRepo: false, | ||
| currentBranch: "", | ||
| mainBranch: "", | ||
| gitStatus: "", | ||
| recentCommits: [], | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -F 'src/adapters/command-code.ts' || true
echo "== file outline =="
ast-grep outline src/adapters/command-code.ts --view expanded || true
echo "== relevant source =="
sed -n '1,220p' src/adapters/command-code.ts
echo "== usages of commandCodeConfig/cwd =="
rg -n "commandCodeConfig|process\.cwd\(|readdirSync" src/adapters/command-code.ts src || trueRepository: lidge-jun/opencodex
Length of output: 16493
🌐 Web query:
Bun Node.js process.cwd behavior when current working directory is deleted renamed fs.ErrNoSuchFile
💡 Result:
When the current working directory (CWD) of a Node.js or Bun process is deleted, renamed, or otherwise becomes inaccessible, calling process.cwd will typically throw an ENOENT error [1][2][3]. In both Node.js and Bun, the behavior is rooted in the fact that process.cwd relies on the underlying operating system's getcwd system call [4][5][2]. When the directory is removed, this system call fails because the path to the current location is no longer valid, resulting in an ENOENT (no such file or directory) error [4][1][2]. Key Behavioral Details: 1. Bun's Behavior: Historically, Bun had issues where deleting the CWD caused crashes or inconsistent behavior in certain scenarios, such as when using Bun Shell [6] or compiled binaries [4]. Recent updates have improved this, ensuring that Bun behaves more predictably—typically by throwing a proper ENOENT error rather than crashing [4][5]. Bun has also been updated to ensure that process.cwd accurately re-queries the kernel after a process.chdir call, allowing it to correctly observe if the new CWD has been removed or renamed [5]. 2. Node.js Behavior: Node.js has consistently maintained that throwing an error is the correct, secure behavior when the CWD disappears, as it avoids presenting potentially misleading information [3]. Recent versions of Node.js have focused on improving the clarity of the error message to explicitly inform users that the failure is likely due to the CWD being removed while the process was still running [2][7][8]. If you encounter this error in your application, it usually means your process is currently executing in a directory that no longer exists on the filesystem. This is common in automated environments or CI/CD pipelines where directories are dynamically created and cleaned up. The standard resolution is to ensure the process changes its working directory to an existing path (e.g., via process.chdir('/')) before attempting operations that rely on the CWD [9].
Citations:
- 1:
process.chdirthrowsENOENTgetcwdafter current working directory is deleted oven-sh/bun#32409 - 2: process.cwd() fails in a not usefully descriptive way nodejs/node#57045
- 3: process.cwd throws error if cwd is deleted nodejs/node-v0.x-archive#1806
- 4: Fix crash when starting a compiled executable in a deleted cwd oven-sh/bun#31496
- 5: process: re-query the kernel for cwd() after chdir; ENAMETOOLONG for overlong chdir targets oven-sh/bun#36158
- 6: Bun Shell throws if current working directory is deleted oven-sh/bun#23589
- 7: process: improve error message for process.cwd() when directory is deleted nodejs/node#57053
- 8: nodejs/node@ca5c4c9752
- 9: Exception on startup if $PWD is deleted nodejs/node#1184
Handle a removed working directory before building Command Code request metadata.
process.cwd() in src/adapters/command-code.ts:65 and src/adapters/command-code.ts:150 is outside the readdirSync catch block. If the directory is removed or renamed concurrently with createCommandCodeAdapter, both calls can throw ENOENT and fail request construction before the upstream fetch. Resolve the working directory once with a safe fallback; omit directory-derived metadata like workingDir or x-project-slug when it is unavailable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/command-code.ts` around lines 61 - 75, Update
createCommandCodeAdapter and commandCodeConfig to resolve process.cwd() once
inside a guarded flow with a safe fallback, then reuse that value everywhere
request metadata is built. The issue is that commandCodeConfig and the
x-project-slug path both call process.cwd() outside the existing readdirSync
protection, so a removed or renamed working directory can still throw ENOENT.
Fix it by centralizing the cwd lookup in the adapter, omitting directory-derived
fields such as workingDir and x-project-slug when the cwd is unavailable, and
keeping the rest of the request construction unchanged.
| export async function loginCommandCode(ctrl: OAuthController, options: CommandCodeLoginOptions = {}): Promise<OAuthCredentials> { | ||
| if (shouldImportLocalCommandCodeAuth(options)) { | ||
| const local = await importLocalCommandCodeAuth(); | ||
| if (local) { | ||
| ctrl.onProgress?.("Imported existing Command Code CLI authentication."); | ||
| return local; | ||
| } | ||
| } | ||
| const state = randomState(); | ||
| const { server, callback } = createCallbackServer(state); | ||
| const callbackUrl = `http://localhost:${server.port}/callback`; | ||
| const authUrl = `${COMMAND_CODE_STUDIO_URL}/studio/auth/cli?callback=${encodeURIComponent(callbackUrl)}&state=${encodeURIComponent(state)}`; | ||
| ctrl.onAuth?.({ url: authUrl, instructions: "Sign in with Command Code in the browser." }); | ||
| ctrl.onProgress?.("Waiting for Command Code authentication..."); | ||
| let timeoutId: ReturnType<typeof setTimeout> | undefined; | ||
| try { | ||
| const timeout = new Promise<never>((_, reject) => { | ||
| timeoutId = setTimeout(() => reject(new Error("Command Code OAuth callback timed out")), LOGIN_TIMEOUT_MS); | ||
| ctrl.signal?.addEventListener("abort", () => { if (timeoutId) clearTimeout(timeoutId); reject(ctrl.signal?.reason); }, { once: true }); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reject an already-aborted login immediately.
At Line 130, addEventListener("abort", ...) does not run when ctrl.signal was already aborted. The login then keeps the callback server open until the 120-second timeout.
Check ctrl.signal?.aborted before local credential I/O and before starting the callback server. Add a regression test that passes an already-aborted signal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/oauth/command-code.ts` around lines 112 - 131, Update loginCommandCode so
it checks ctrl.signal?.aborted before any local credential import or
createCallbackServer work, and immediately rejects or throws using the signal’s
existing reason instead of proceeding to the timeout path. Keep the current
abort listener for in-flight cancellations, but add a regression test that
invokes loginCommandCode with an already-aborted signal and verifies it fails
right away without leaving the callback server waiting for LOGIN_TIMEOUT_MS.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49bc0b657d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| params: { | ||
| model: COMMAND_CODE_MODEL_ALIASES[parsed.modelId] ?? parsed.modelId, | ||
| messages: wireMessages(parsed.context.messages), | ||
| tools: wireTools(parsed.context.tools), |
There was a problem hiding this comment.
When a request sets tool_choice: "none" or an allowed_tools subset, this still serializes every tool into params.tools and sends no equivalent choice constraint. In tool-disabled or forced-final turns, Command Code can therefore return a tool call the client explicitly disallowed, and the bridge may surface/execute it. Filter/drop the advertised tools and forward the provider's choice equivalent before building the request.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| body: JSON.stringify(body), | ||
| }; | ||
| }, | ||
| async *parseStream(response: Response, _budget: TranslatorBudget): AsyncGenerator<AdapterEvent> { |
There was a problem hiding this comment.
This parser accepts a TranslatorBudget but never uses it, while ndjson() accumulates buffer until a newline and tool-call inputs are stringified into emitted deltas. If Command Code or a proxy sends a very large/unterminated NDJSON line or huge tool arguments, this path can allocate unbounded memory instead of producing the bounded translation failure that the other adapters provide. Please charge/release the budget around stream buffering and retained events.
AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.
| noVisionModels: ["grok-build-0.1", "grok-composer-2.5-fast"], | ||
| }, | ||
| { | ||
| id: "command-code", |
There was a problem hiding this comment.
This adds a user-facing command-code OAuth account provider, but the provider guide still documents only the API-key Provider plan and says CLI auth bridging for Go/Pro subscriptions is not available. Users following docs-site/src/content/docs/guides/providers.md and the translated copies will miss ocx login command-code or choose the wrong setup path, so update the English source and locales with the new account flow.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| max_tokens: parsed.options.maxOutputTokens ?? provider.defaultMaxOutputTokens ?? 64_000, | ||
| stream: true, | ||
| ...(parsed.options.temperature !== undefined ? { temperature: parsed.options.temperature } : {}), | ||
| ...(parsed.options.reasoning && parsed.options.reasoning !== "none" ? { reasoning_effort: parsed.options.reasoning } : {}), |
There was a problem hiding this comment.
The registry intentionally exposes reasoningEfforts: [] for this provider, but this still forwards any requested parsed.options.reasoning except none. For normal Codex turns that carry a default effort, models whose Command Code catalog does not support an effort knob can receive an unsupported reasoning_effort even though the picker hid effort control; use the shared effort mapper/config checks and omit the field when the configured ladder is empty.
Useful? React with 👍 / 👎.
| out.push({ role: "tool", content: [{ | ||
| type: "tool-result", | ||
| toolCallId: message.toolCallId, | ||
| toolName: namespacedToolName(message.toolNamespace, message.toolName), |
There was a problem hiding this comment.
When a tool result contains image parts, this conversion calls textContent() and drops every non-text part before sending the result upstream. A turn that uses view_image or any image-returning MCP tool will therefore continue with only the text portion of the tool result, leaving the model unable to inspect the image output; mirror the existing adapter pattern that carries tool-result images forward instead of filtering them out.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| } | ||
| const state = randomState(); | ||
| const { server, callback } = createCallbackServer(state); | ||
| const callbackUrl = `http://localhost:${server.port}/callback`; |
There was a problem hiding this comment.
This custom callback flow binds only IPv4 127.0.0.1 but advertises localhost, and it never races ctrl.onManualCodeInput; on Windows where localhost resolves to ::1 first, or in remote/headless GUI sessions where the browser cannot reach the proxy loopback, Command Code login will wait until timeout instead of using the existing IPv6/manual paste fallback. Please route this through the shared OAuth callback flow or add the same fallback behavior here.
Useful? React with 👍 / 👎.
| name: "command-code", | ||
| buildRequest(parsed: OcxParsedRequest): AdapterRequest { | ||
| if (!provider.apiKey) throw new Error("Command Code credential missing — run ocx login command-code"); | ||
| const system = parsed.context.systemPrompt?.join("\n\n") ?? ""; |
There was a problem hiding this comment.
This sends Codex's system prompt to Command Code unchanged, but that prompt includes the native GPT/OpenAI identity text that the other routed adapters rewrite before calling non-OpenAI models. When users pick Command Code-hosted DeepSeek, Kimi, GLM, or other non-OpenAI models, the model can be instructed to present itself as GPT/OpenAI; run the system text through the same routed-model identity rewrite before serializing it.
Useful? React with 👍 / 👎.
| // Command Code documents effort support as model-dependent. Do not synthesize a ladder. | ||
| reasoningEfforts: [], | ||
| defaultMaxOutputTokens: 64_000, | ||
| parallelToolCalls: true, |
There was a problem hiding this comment.
The registry advertises Command Code as supporting parallel tool calls, so Codex may send parallel_tool_calls:false on turns that require serialized tool use, but the adapter never reads parsed.options.parallelToolCalls or sends a disable flag. In those turns Command Code can still emit multiple simultaneous tool calls despite the caller's constraint; either forward the provider's equivalent flag or avoid advertising parallel support.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
49bc0b6 to
4b95fdb
Compare
[GD] Verdict: gatedTLDR
Full verdictSemantic propagation
Linked: none UsefulnessUseful. This is a real provider integration: OAuth login with optional local CLI import, proprietary generate adapter, live model discovery without a static fallback catalog, reasoning-effort profile facts, GUI display name keys, and focused tests. Claimed live local validation is author-side only and not reproducible here. Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
Simplification (for the PR owner)Foreign PR — nothing edited or pushed. Bounded candidates only:
Gatedraft | incomplete readiness checklist | Bottom lineShip only after @hanbinnoh (owner) addresses tool-result images, OAuth callback robustness, docs/OAuth product copy, GUI screenshot, and maintainer security sponsorship. This review did not push to the fork head. Re-request full review on the next tip. |
2 participants
Summary
Validation
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I fixed all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit