feat(cli): add ocx opencode launcher - #461
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
opencode reads providers from a JSON config rather than env slots, so the
`ocx claude` env-injection pattern does not transfer. This adds a launcher that
generates a provider block from the proxy's visible catalog and points
OPENCODE_CONFIG at it.
The user's own opencode.json is never written to. Their effective config is read
(explicit OPENCODE_CONFIG first, then the XDG global path), merged forward into
a generated copy under the opencodex config dir, and only the `opencodex`
provider key is overwritten. Carrying the base config forward keeps the command
correct whether opencode merges the OPENCODE_CONFIG layer or replaces it, and
leaves plain `opencode` completely unchanged.
Credential handling: the generated file is written to disk and outlives the
child, so it carries opencode's documented `{env:VAR}` reference instead of the
admission key, and the real value is passed only through the child environment.
The key resolves OPENCODEX_API_AUTH_TOKEN before config.apiKeys, matching
fetchClaudeContextWindows — a non-loopback bind requires the env token and may
have no apiKeys at all, where a placeholder would 401 every request.
Model limits: limit.context is emitted only from an authoritative context
window, including native slugs via nativeOpenAiContextWindow. opencode's schema
rejects a limit block carrying context without output and CatalogModel has no
authoritative output field, so a documented budget rides along, clamped to the
context window so a small-context model is never emitted with output > context.
Robustness: opencode.json is parsed as JSONC (strict JSON first, tolerant only
on failure) because opencode documents that syntax and a commented config would
otherwise be rejected as malformed; the generated file is written atomically so
a concurrent launch cannot read a torn file; the detached proxy-start child gets
an error listener so a failed spawn reports through the health poll instead of
throwing; and a project-level opencode.json defining provider.opencodex is
detected and warned about, since opencode loads that layer last and it outranks
the generated block.
Verified against a live proxy: 76 models registered, 47 carrying authoritative
limits with no output > context violations, `opencode models opencodex` lists
all of them, and an end-to-end run through opencodex/kiro/claude-haiku-4.5
returns a completion.
01a74b9 to
5ef84f4
Compare
|
Thanks both — this was a genuinely useful review, and four of these were real bugs rather than style. Force-pushed an amended commit ( Fixed (7)
Not changed, reasoning in-thread (2, both awaiting a maintainer call)
Partially addressed (1)
Tests went 21 → 36. Full suite 4252 pass with the same 2 pre-existing One note: the heavy CI matrix has not run yet — I believe it needs a maintainer to approve workflows for a first-time contributor. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
🔒 Under maintainer review — detailed feedback incoming@lidge-jun (maintainer) has this PR in an active review pass. Please do not merge, rebase, or This is a claim marker so two maintainers do not review or land the same PR at once. If you are a Baseline for this pass: No action needed from you until then. Thanks for the contribution and for your patience. Review tracker: |
|
Thanks for the 1. High — the generated file duplicates arbitrary user secrets. 2. High — copying the config silently changes what relative 3. Medium — project-override detection doesn't match OpenCode's lookup rules. What I checked and found clean: no argument-injection defect — Overlap with Test gap: Reviewed as part of a maintainer review pass against |
|
Maintainer takeover update for PR #461: Thanks @mihneaptu for the Could not force-update the contributor head (
Please either enable “Allow edits from maintainers” on this PR so we can repoint the head, or reset Review fixes landed
Still open (non-blocking for the review blockers above)
Local verification on
Fresh Cross-platform CI must run on the rebased head before merge; the green matrix on Docs updated in |
|
Closing in favor of maintainer takeover #568, which rebases your work onto current \dev\ and applies the credential-lifecycle fixes from the maintainer review (runtime \OPENCODE_CONFIG_CONTENT\ injection instead of copying user config to disk). Thank you @mihneaptu — the launcher design and spawn/proxy wiring were solid, and the original review thread helped catch real issues. Your authorship is called out in #568; sorry we couldn't update the fork head directly (\maintainerCanModify\ was off). Please follow #568 for merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef84f4853
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * fetchClaudeContextWindows in src/cli/claude.ts. | ||
| */ | ||
| export function opencodeApiKey(config: OcxConfig, env: OpencodeLaunchEnv = process.env): string { | ||
| return env.OPENCODEX_API_AUTH_TOKEN || config.apiKeys?.[0]?.key || "ocx"; |
There was a problem hiding this comment.
When a non-loopback proxy runs as an installed service and ocx opencode is invoked from a fresh shell, the admission token exists only in ~/.opencodex/service-api-token; this resolver checks only the shell environment and config.apiKeys, so it passes ocx and every request receives 401. Fresh evidence after the earlier fix is that loadServiceTokenFromFile is still called only by handleStart in src/cli/index.ts, not by this launcher. Resolve the persisted service token through the shared service-secret path before falling back to a configured key.
AGENTS.md reference: AGENTS.md:L65-L71
Useful? React with 👍 / 👎.
| contextWindow: m.contextWindow, | ||
| displayName: m.displayName, | ||
| })); | ||
| const nativeSlugs = [...visibleNativeSlugs(config)]; |
There was a problem hiding this comment.
When providers.openai.codexAccountMode is direct, every native slug advertised here is unusable from opencode: the generated provider supplies only the proxy admission key (or ocx) as Authorization, while handleChatCompletions preserves that header for Direct routes and handleResponses treats it as the caller's ChatGPT bearer, yielding either the admission-credential 401 or an upstream authentication failure. Omit native entries in Direct mode, or provide a separate valid Direct bearer and admission channel, and cover this supported mode with a focused regression test.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
| }; | ||
| if (typeof contextWindow === "number" && Number.isFinite(contextWindow) && contextWindow > 0) { | ||
| const context = Math.floor(contextWindow); | ||
| entry.limit = { context, output: Math.min(SCHEMA_REQUIRED_OUTPUT_BUDGET, context) }; |
There was a problem hiding this comment.
For any model whose context window is at most 32,000 tokens, this sets limit.output equal to limit.context, leaving opencode no usable input budget once it reserves the advertised maximum output. Fresh evidence after the prior clamp fix is the equality produced by Math.min(..., context), and the new 8,192-token test now codifies that broken value. Clamp below the context window using a safe input reserve, or omit the pair when no defensible output limit is available, and update the regression accordingly.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
| * native OpenAI entries (which the proxy accepts unprefixed). | ||
| */ | ||
| export function opencodeModelKey(provider: string, id: string): string { | ||
| return provider === "native" ? id : `${provider}/${id}`; |
There was a problem hiding this comment.
native provider as the sentinel
Provider names are user-defined and repository validation permits native, but this helper treats every routed model from such a provider as a built-in native model and removes its namespace. For example, native/gpt-5-custom becomes bare gpt-5-custom, which the router sends to the canonical OpenAI provider rather than the configured native provider. Represent built-in candidates with a separate discriminator instead of overloading a valid provider name, and add a focused collision test.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
| const generated = buildOpencodeConfig(port, nativeSlugs, routed, nativeOpenAiContextWindow); | ||
| const merged = mergeOpencodeConfig(base.config, generated); | ||
| try { | ||
| writeGeneratedConfig(generatedPath, JSON.stringify(merged, null, 2) + "\n"); |
There was a problem hiding this comment.
When the user's base opencode config contains literal credentials such as provider.<id>.options.apiKey, the full merge is serialized into the persistent generated file, duplicating arbitrary third-party secrets even though the proxy admission key itself was moved to an environment reference. Removing or rotating the credential in the original config still leaves the stale copy here until another launch overwrites it; mode 0600 does not eliminate that expanded credential footprint. Preserve secret-bearing values through environment references, or rely on opencode's configuration layers instead of serializing the entire base config.
AGENTS.md reference: AGENTS.md:L65-L71
Useful? React with 👍 / 👎.
| const routed = filterCatalogVisibleModels(allModels, config).map(m => ({ | ||
| provider: m.provider, | ||
| id: m.id, | ||
| contextWindow: m.contextWindow, | ||
| displayName: m.displayName, |
There was a problem hiding this comment.
When a configured combo has an alias, fetchAllModels returns a CatalogModel containing { provider: "combo", id, alias }, but this projection discards alias, so the generated picker exposes combo/<id> instead of the configured public model ID. This also bypasses the catalog's established rule that a combo alias shadows a provider model with the same public slug. Carry the alias through and key the entry with the catalog's public-slug helper, with a focused alias regression test.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
3 participants
Summary
Adds
ocx opencode [opencode args...], a third client surface alongsideocx claudeand the native Codex injection.opencode reads providers from a JSON config rather than env slots, so the
ocx claudeenv-injection pattern does not transfer. This launcher instead:findLiveProxy+ detached-start path asocx claude),opencodexprovider block from the proxy's visible catalog (fetchAllModels→filterCatalogVisibleModels+visibleNativeSlugs, the same pairocx claude desktopuses),OPENCODE_CONFIGat the generated file and execsopencodewith stdio inherited.The user's own
opencode.jsonis never written to. Their effective config is read (an explicitOPENCODE_CONFIGfirst, then the XDG global path), merged forward into a generated copy under the opencodex config dir, and only theopencodexprovider key is overwritten — other providers and unrelated top-level fields (model,agents,keybinds,mcp, …) survive verbatim. Carrying the base config forward keeps the command correct whether opencode merges theOPENCODE_CONFIGlayer or replaces it, and leaves plainopencodecompletely unchanged.The admission key is never serialized to disk. The generated block carries opencode's documented
{env:VAR}reference and the real value is passed only through the child environment. The key resolvesOPENCODEX_API_AUTH_TOKENbeforeconfig.apiKeys, matchingfetchClaudeContextWindows— a non-loopback bind requires the env token and may have noapiKeysat all, where a placeholder would 401 every request.Other behavior worth a reviewer's eye:
limit.contextis emitted only from an authoritative context window, including native slugs vianativeOpenAiContextWindow. opencode's schema rejects alimitblock carryingcontextwithoutoutputandCatalogModelhas no authoritative output field, so a documented budget (32_000, matchingREASONING_MAX_TOKENS_CEILING) rides along, clamped to the context window so a small-context model is never emitted withoutput > context.opencode.jsonas JSONC. StrictJSON.parseruns first and untouched; a small string-aware stripper is attempted only on failure, so a commented config launches instead of being rejected as malformed. No new dependency.opencode.jsonafter theOPENCODE_CONFIGlayer, so a project-levelprovider.opencodexoutranks the generated block. The launcher cannot win that without writing to a user file, so it detects and warns rather than reporting a wiring the child will not honour.ocx opencode run "…" > out.txtkeeps stdout clean for the child.Docs: new
guides/opencode.mdplus its sidebar entry (English only; the label is a proper noun in every locale).Verification
bun run typecheck— cleanbun run test— 4252 pass, 4 skip, 2 failbun run lint:gui,bun run privacy:scan— cleanbun test tests/opencode-cli.test.ts— 36 pass (new)The 2 failures are pre-existing and unrelated to this change. Both are in
tests/oauth-refresh.test.ts:I confirmed this by stashing the branch and running the full suite on a clean
upstream/devcheckout — the same two tests fail there (4216 pass, 2 fail). They also pass whentests/oauth-refresh.test.tsis run on its own.tests/oauth-refresh.test.tsandtests/kiro-oauth.test.tsboth mutate the process-globalprocess.env.HOMEinbeforeEach/afterEach, which looks like the cross-file race, but I have left it alone as out of scope. Happy to open a separate issue.Manual verification against a live proxy on
127.0.0.1:10100(Windows 11, opencode 1.18.5):ocx opencode --version→ proxy detected, 76 models registered, generated config written, child launchedopencode models opencodex→ all 76 listed (this is what caught thelimit.outputschema requirement — unit tests alone would not have)output > contextviolationsapiKeyis{env:OPENCODEX_OPENCODE_API_KEY}; no literal secret in the fileopencode run --model opencodex/kiro/claude-haiku-4.5 "…"→ completion returned end to end~/.config/opencode/opencode.jsonbyte-for-byte untouched across runs; its hand-written providers and defaultmodelreappear intact in the generated copyocx help opencoderendersChecklist
On the last point: the generated config contains no credential — only an
{env:…}reference — and is still written0o600with the config dir created0o700. No token is logged; the launcher prints only the port, the model count and the two file paths.