Skip to content

feat(cli): add ocx opencode launcher#461

Draft
mihneaptu wants to merge 1 commit into
lidge-jun:devfrom
mihneaptu:feat/ocx-opencode
Draft

feat(cli): add ocx opencode launcher#461
mihneaptu wants to merge 1 commit into
lidge-jun:devfrom
mihneaptu:feat/ocx-opencode

Conversation

@mihneaptu

@mihneaptu mihneaptu commented Jul 25, 2026

Copy link
Copy Markdown

Summary

Adds ocx opencode [opencode args...], a third client surface alongside ocx claude and the native Codex injection.

opencode reads providers from a JSON config rather than env slots, so the ocx claude env-injection pattern does not transfer. This launcher instead:

  1. ensures the proxy is running (same findLiveProxy + detached-start path as ocx claude),
  2. generates an opencodex provider block from the proxy's visible catalog (fetchAllModelsfilterCatalogVisibleModels + visibleNativeSlugs, the same pair ocx claude desktop uses),
  3. points OPENCODE_CONFIG at the generated file and execs opencode with stdio inherited.

The user's own opencode.json is never written to. Their effective config is read (an 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 — 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 the OPENCODE_CONFIG layer or replaces it, and leaves plain opencode completely 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 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.

Other behavior worth a reviewer's eye:

  • 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 (32_000, matching REASONING_MAX_TOKENS_CEILING) rides along, clamped to the context window so a small-context model is never emitted with output > context.
  • JSONC. opencode documents opencode.json as JSONC. Strict JSON.parse runs 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.
  • Project layer. opencode loads a project-level opencode.json after the OPENCODE_CONFIG layer, so a project-level provider.opencodex outranks 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.
  • Pre-launch messages go to stderr, so ocx opencode run "…" > out.txt keeps stdout clean for the child.

Docs: new guides/opencode.md plus its sidebar entry (English only; the label is a proper noun in every locale).

Verification

  • bun run typecheck — clean
  • bun run test4252 pass, 4 skip, 2 fail
  • bun run lint:gui, bun run privacy:scan — clean
  • bun 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:

(fail) oauth refresh hardening > fresh Kiro CLI SQLite token is imported before refresh endpoint
(fail) oauth refresh hardening > failed refresh recovers from a now-fresh Kiro CLI SQLite token

I confirmed this by stashing the branch and running the full suite on a clean upstream/dev checkout — the same two tests fail there (4216 pass, 2 fail). They also pass when tests/oauth-refresh.test.ts is run on its own. tests/oauth-refresh.test.ts and tests/kiro-oauth.test.ts both mutate the process-global process.env.HOME in beforeEach/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 launched
  • opencode models opencodex → all 76 listed (this is what caught the limit.output schema requirement — unit tests alone would not have)
  • 47 models carry authoritative limits, with 0 output > context violations
  • generated apiKey is {env:OPENCODEX_OPENCODE_API_KEY}; no literal secret in the file
  • opencode run --model opencodex/kiro/claude-haiku-4.5 "…" → completion returned end to end
  • existing ~/.config/opencode/opencode.json byte-for-byte untouched across runs; its hand-written providers and default model reappear intact in the generated copy
  • ocx help opencode renders

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

On the last point: the generated config contains no credential — only an {env:…} reference — and is still written 0o600 with the config dir created 0o700. No token is logged; the launcher prints only the port, the model count and the two file paths.

@github-actions github-actions Bot added the enhancement New feature or request label Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds ocx opencode, which starts or reuses the proxy, generates an opencodex provider configuration from visible models, preserves existing OpenCode settings, launches opencode, and documents the command.

Changes

OpenCode launcher

Layer / File(s) Summary
Provider and config generation
src/cli/opencode.ts, tests/opencode-cli.test.ts
Defines OpenCode configuration types and helpers, builds model entries and limits, merges only the opencodex provider, and tests these behaviors.
Proxy, config, and process orchestration
src/cli/opencode.ts, tests/opencode-cli.test.ts
Discovers or starts the proxy, reads and validates configuration sources, writes a restricted generated config, launches opencode, and handles exit and not-found cases.
CLI wiring and user documentation
src/cli/index.ts, src/cli/help.ts, docs-site/src/content/docs/guides/opencode.md, docs-site/astro.config.mjs
Registers the command and help text, and adds the localized OpenCode guide and sidebar entry.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant cmdOpencode
  participant Proxy
  participant Catalog
  participant OpenCode
  User->>cmdOpencode: ocx opencode arguments
  cmdOpencode->>Proxy: check or start proxy
  cmdOpencode->>Catalog: fetch visible models
  cmdOpencode->>OpenCode: set OPENCODE_CONFIG and spawn process
  OpenCode-->>User: exit status
Loading

Suggested reviewers: lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding the ocx opencode CLI launcher.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@src/cli/opencode.ts`:
- Around line 216-220: Update ensureProxyForOpencode to accept the
already-loaded config object as a parameter and derive cfgPort from it instead
of calling loadConfig(). Update cmdOpencode’s call site to pass its existing
config, preserving the current port fallback behavior.
- Around line 216-235: Update ensureProxyForOpencode so the child returned by
spawn has an error listener attached before unref, preventing launch failures
from becoming unhandled events and allowing the existing timeout/null path to
handle them. Keep the current detached spawn options and health-check loop
unchanged.
🪄 Autofix (Beta)

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: f796a736-e4fd-4e95-b842-3aa22218f902

📥 Commits

Reviewing files that changed from the base of the PR and between 233f3f8 and 01a74b9.

📒 Files selected for processing (6)
  • docs-site/astro.config.mjs
  • docs-site/src/content/docs/guides/opencode.md
  • src/cli/help.ts
  • src/cli/index.ts
  • src/cli/opencode.ts
  • tests/opencode-cli.test.ts

Comment thread src/cli/opencode.ts Outdated
Comment thread src/cli/opencode.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01a74b9f93

ℹ️ 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".

Comment thread src/cli/opencode.ts Outdated
console.error(" Fix or move that file — refusing to launch, because generating a config would drop its settings.");
return 1;
}
const merged = mergeOpencodeConfig(base.config, buildOpencodeConfig(port, nativeSlugs, routed, opencodeApiKey(config)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the admission key out of the generated config

When config.apiKeys is nonempty, this copies the first reusable admission secret into the provider block, which is then persisted in plaintext at ~/.opencodex/opencode-config.json and remains there after the child exits; this unnecessarily duplicates the credential even for loopback proxies where authentication is disabled. Pass the credential only through the child environment or an opencode-supported environment reference instead of serializing it into the generated file.

AGENTS.md reference: AGENTS.md:L65-L71

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The generated block now carries opencode's documented {env:VAR} reference and the real value travels only in the child environment:

"options": { "baseURL": "", "apiKey": "{env:OPENCODEX_OPENCODE_API_KEY}" }

You are right that 0o600 was not sufficient justification — the file outlives the child, so the secret had no reason to be on disk at all. Verified end to end against a live proxy (the substitution resolves at load time and routing still works), and there is a test asserting the serialized block never contains a literal key.

Comment thread src/cli/opencode.ts Outdated
Comment on lines +212 to +213
export function opencodeApiKey(config: OcxConfig): string {
return config.apiKeys?.[0]?.key ?? "ocx";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Honor the environment admission token

When the proxy is bound to a non-loopback hostname with only the required OPENCODEX_API_AUTH_TOKEN configured, this returns the placeholder ocx because config.apiKeys is empty. The server validates requests against the environment token, so every opencode request receives 401; resolve the environment or service-file token before falling back to a configured API key.

AGENTS.md reference: AGENTS.md:L65-L69

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — this was a real bug. opencodeApiKey now resolves OPENCODEX_API_AUTH_TOKEN before config.apiKeys[0], matching the precedence fetchClaudeContextWindows already uses in src/cli/claude.ts:125. I had copied the config-only half and missed the env half, which would have 401'd every request on a non-loopback bind. Covered by a test asserting the env token wins over a configured key.

Comment thread src/cli/opencode.ts Outdated
return {
npm: OPENCODE_PROVIDER_NPM,
name: "OpenCodex",
options: { baseURL: `http://127.0.0.1:${port}/v1`, apiKey },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the live proxy hostname

For a proxy bound to a concrete interface such as 192.168.1.10 or an IPv6-only loopback address, findLiveProxy() can successfully discover it using its recorded hostname, but ensureProxyForOpencode() discards that hostname and this URL always targets IPv4 loopback. The generated client then connects to an address where the proxy is not listening; retain the discovered hostname and construct the base URL with the same probe-host normalization used by liveness.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid, but deliberately not changed here — I would like a maintainer call first.

ocx claude has exactly the same assumption: buildClaudeEnv hardcodes ANTHROPIC_BASE_URL to http://127.0.0.1:${port} (src/cli/claude.ts:35) and discards the hostname findLiveProxy() discovered. So this is a pre-existing pattern shared by both launchers, not something new in this PR.

My concern with fixing it only here is divergence: the two launchers would then disagree about how a non-loopback or IPv6 bind is addressed, which seems worse than being consistently wrong until both are fixed together with the probe-host normalization you describe. If you would prefer it fixed in this PR anyway I will do it, and can follow up with the matching ocx claude change.

Comment thread src/cli/opencode.ts
Comment on lines +125 to +127
const candidates: OpencodeRoutedModel[] = [
...nativeSlugs.map(id => ({ provider: "native", id })),
...routedModels,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Carry native context windows into model entries

Every native slug is converted without a contextWindow, so the limit-emission branch can never run for native models even though the repository provides authoritative values through nativeOpenAiContextWindow (including the 1M window for gpt-5.4). Under the custom opencodex provider this leaves opencode using fallback limits and can trigger compaction or token budgeting at the wrong boundary; attach the native metadata when constructing these candidates.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — I did not know nativeOpenAiContextWindow existed, thanks for the pointer. buildOpencodeProviderBlock now takes a nativeContextWindow resolver (defaulted for test purity) and cmdOpencode passes the catalog's. Against a live catalog this took models carrying authoritative limits from 42 to 47, with the native slugs now correctly reporting e.g. gpt-5.6-sol: 372000 instead of falling back.

Comment thread src/cli/opencode.ts Outdated
const candidate = explicit && explicit !== generatedPath ? explicit : opencodeGlobalConfigPath(env);
if (!existsSync(candidate)) return { config: null, sourcePath: null };
try {
const parsed = JSON.parse(readFileSync(candidate, "utf8")) as unknown;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse documented JSONC configuration files

The opencode configuration documentation allows opencode.json to use JSONC, but a valid user config containing comments or trailing commas fails this JSON.parse call and makes ocx opencode refuse to launch as though the file were malformed. Read the base through a JSONC-compatible parser so the launcher accepts the same configuration syntax as the client it wraps.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. This was worse than it looked, because the launcher deliberately aborts on an unparseable base config rather than silently dropping settings — so a perfectly valid commented opencode.json turned into a hard refusal to launch.

Rather than add a dependency (the project keeps 4, and AGENTS.md flags dependency installation for security review) I added a small string-aware stripper. Strict JSON.parse still runs first and untouched; the tolerant path is only attempted when that throws, so well-formed configs never reach the stripper. Tests cover line/block comments, trailing commas, and — importantly — comment-like and comma-like text inside string values plus escaped quotes, so a "http://…" URL or "a // b" note is not corrupted.

Comment thread src/cli/opencode.ts Outdated
Comment on lines +135 to +136
if (typeof contextWindow === "number" && Number.isFinite(contextWindow) && contextWindow > 0) {
entry.limit = { context: Math.floor(contextWindow), output: SCHEMA_REQUIRED_OUTPUT_BUDGET };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound the output limit by the context window

For a routed custom or local model whose authoritative contextWindow is below 32,000 tokens, this emits an impossible model definition with limit.output greater than limit.context. That can leave opencode with no usable input budget or cause an otherwise valid small-context model to be rejected before reaching the proxy; cap the placeholder output value at the emitted context limit, or omit the limit pair when a safe output value is unavailable.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — clear bug, thank you. limit.output is now Math.min(SCHEMA_REQUIRED_OUTPUT_BUDGET, context), so a sub-32k model can never be emitted with output > context. I chose clamping over omitting the pair so the authoritative context window is still published. Test added with an 8192-context model asserting { context: 8192, output: 8192 }, and I verified 0 violations across all 76 models generated from a live catalog.

Comment thread src/cli/opencode.ts Outdated
Comment on lines +207 to +208
export function buildOpencodeEnv(configPath: string, base: OpencodeLaunchEnv): OpencodeLaunchEnv {
return { ...base, OPENCODE_CONFIG: configPath };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the provider override at the highest precedence

When the current project has an opencode.json that already defines provider.opencodex, opencode loads that project layer after the custom OPENCODE_CONFIG layer, so it overrides this generated block's base URL, API key, or models. The command still reports that it is wired to the detected proxy while the child can use the stale project definition; inject the owned provider block through a higher-precedence layer or otherwise reapply it after project configuration.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct diagnosis, but I do not think it is fully fixable as stated — so I have made it visible instead.

opencode loads the project layer last by design. The only ways to outrank it are to write into the user's project opencode.json, or to point OPENCODE_CONFIG at something that suppresses the project layer — both of which break the property this command is built around (never modify the user's files, plain opencode unchanged afterwards). Silently winning that fight would be worse than losing it.

So projectConfigOverridesProvider() now detects a project-level opencode.json defining provider.opencodex and prints a warning naming the file, rather than reporting a successful wiring that the child will not honour. It is also documented under "Project configs still win" in the guide. Tests cover detect / ignore-other-providers / no-file.

If you know of a precedence lever I have missed, I will happily take it.

Comment thread src/cli/opencode.ts
Comment on lines +98 to +99
export function generatedOpencodeConfigPath(): string {
return join(getConfigDir(), "opencode-config.json");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Give concurrent launches separate generated configs

Every invocation writes and passes the same global file, so two ocx opencode launches using different explicit base configs can race: one child may read the other launch's settings, or read the file while it is being truncated and rewritten. This is especially visible when starting sessions for different projects at the same time; generate a per-launch file and remove it after the child exits rather than sharing one mutable path.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially addressed, and I would like a maintainer steer on the rest.

Fixed: the write is now atomic (writeFileSync to <path>.<pid>.tmp then renameSync, with the temp file cleaned up on failure). That removes the torn-read half — a child can no longer observe a half-written file.

Not done — per-launch files. It would fix the remaining cross-launch case, but it trades one problem for another: the path is handed to a detached child we do not wait on, so "remove it after the child exits" has no reliable hook here, and orphaned per-launch configs would accumulate in the config dir on every crash or hard kill. A single stable path is also what makes the command debuggable — you can always look at one file to see what the child was given.

The residual race needs two concurrent launches with different explicit base configs, which is narrow. If you would rather have per-launch files with a best-effort cleanup on exit, I am happy to switch — it is a small change, I just did not want to make that durability trade unilaterally.

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.
@mihneaptu
mihneaptu force-pushed the feat/ocx-opencode branch from 01a74b9 to 5ef84f4 Compare July 25, 2026 21:19
@mihneaptu

Copy link
Copy Markdown
Author

Thanks both — this was a genuinely useful review, and four of these were real bugs rather than style.

Force-pushed an amended commit (01a74b95ef84f4) and replied to each thread individually. Summary:

Fixed (7)

  • P1 admission key no longer serialized — the block carries {env:OPENCODEX_OPENCODE_API_KEY} and the value travels in the child env
  • P1 OPENCODEX_API_AUTH_TOKEN now resolved before config.apiKeys (non-loopback binds would have 401'd on every request)
  • P2 limit.output clamped to limit.context (sub-32k models were being emitted with output > context)
  • P2 opencode.json parsed as JSONC — a valid commented config was causing a hard refusal to launch
  • P2 native slugs now carry authoritative windows via nativeOpenAiContextWindow (42 → 47 models with limits)
  • P2 generated config written atomically (temp + rename), removing the torn-read race
  • CodeRabbit: error listener on the detached spawn, and the redundant loadConfig() threaded through

Not changed, reasoning in-thread (2, both awaiting a maintainer call)

  • Hardcoded 127.0.0.1 base URL — buildClaudeEnv (src/cli/claude.ts:35) makes the identical assumption, so fixing only this launcher would make the two disagree about non-loopback binds. Happy to fix both together.
  • Per-launch generated files — the atomic write covers torn reads; per-launch paths would orphan files on crash since the child is detached and unwaited. Will switch if you prefer.

Partially addressed (1)

  • Project-layer precedence is real but not winnable without writing to a user file, so it is now detected and warned about, plus documented.

Tests went 21 → 36. Full suite 4252 pass with the same 2 pre-existing oauth-refresh.test.ts failures I documented in the description (verified identical on clean dev).

One note: the heavy CI matrix has not run yet — I believe it needs a maintainer to approve workflows for a first-time contributor.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 `@docs-site/src/content/docs/guides/opencode.md`:
- Around line 22-25: Add a text or console language identifier to the fenced
code block containing the opencodex model slugs in the opencode documentation,
while preserving its contents unchanged.
- Around line 29-42: The documentation incorrectly promises byte-for-byte
preservation of the base configuration. In the launcher behavior section,
replace “preserved verbatim” with wording that promises semantic preservation of
providers and top-level fields, and clarify that JSONC comments and trailing
commas are accepted when reading input but are not retained in the generated
file.

In `@src/cli/opencode.ts`:
- Around line 400-409: Wrap the fetchAllModels(config) call in the same
try/catch error-handling pattern used by the disk I/O blocks in cmdOpencode. On
failure, print a friendly actionable console.error and return 1, while
preserving the existing model filtering flow for successful fetches.
- Around line 302-318: Update projectConfigOverridesProvider to search for
opencode.json in cwd and each ancestor directory up to the project or filesystem
root, matching OpenCode’s resolution order. Inspect each existing config for the
provider key and return the applicable config path so nested invocations still
trigger the warning; return null only when no ancestor config overrides the
provider.
🪄 Autofix (Beta)

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: db3c8946-c95d-4dfd-a503-600ae1045301

📥 Commits

Reviewing files that changed from the base of the PR and between 01a74b9 and 5ef84f4.

📒 Files selected for processing (6)
  • docs-site/astro.config.mjs
  • docs-site/src/content/docs/guides/opencode.md
  • src/cli/help.ts
  • src/cli/index.ts
  • src/cli/opencode.ts
  • tests/opencode-cli.test.ts

Comment on lines +22 to +25
```
opencodex/kiro/glm-5
opencodex/gpt-5.6-sol # native slugs stay unprefixed
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add a language identifier to this fenced block.

Markdownlint reports MD040 for the unlabeled fence at Line 22. Use text or console after the opening backticks.

🧰 Tools
🪛 markdownlint-cli2 (0.23.0)

[warning] 22-22: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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 `@docs-site/src/content/docs/guides/opencode.md` around lines 22 - 25, Add a
text or console language identifier to the fenced code block containing the
opencodex model slugs in the opencode documentation, while preserving its
contents unchanged.

Sources: Path instructions, Linters/SAST tools

Comment on lines +29 to +42
The launcher does not write to `~/.config/opencode/opencode.json`. Instead it reads your
effective config, merges it forward into the generated copy, and overwrites only the
`opencodex` provider key. Every other provider, plus unrelated top-level fields (`model`,
`agents`, `keybinds`, `mcp`, …), is preserved verbatim.

| Source | Behavior |
| --- | --- |
| `OPENCODE_CONFIG` already exported | Used as the base config, then superseded by the generated path |
| `~/.config/opencode/opencode.json` (or `$XDG_CONFIG_HOME`) | Used as the base config when no explicit export exists |
| Neither exists | A standalone config containing only the `opencodex` provider |
| Base config is malformed | The launch is refused rather than silently dropping your settings |

Comments and trailing commas are fine — opencode documents `opencode.json` as JSONC, and the
launcher parses the same syntax the client accepts.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not promise byte-for-byte preservation of the base config.

src/cli/opencode.ts parses the base JSONC and rewrites the merged object with JSON.stringify, so settings are preserved semantically, but comments and formatting are not carried into the generated file. Please replace “preserved verbatim” and clarify that JSONC comments/trailing commas are accepted on input only.

🤖 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 `@docs-site/src/content/docs/guides/opencode.md` around lines 29 - 42, The
documentation incorrectly promises byte-for-byte preservation of the base
configuration. In the launcher behavior section, replace “preserved verbatim”
with wording that promises semantic preservation of providers and top-level
fields, and clarify that JSONC comments and trailing commas are accepted when
reading input but are not retained in the generated file.

Source: Path instructions

Comment thread src/cli/opencode.ts
Comment on lines +302 to +318
/**
* Detect a project-level opencode.json that defines our provider key. opencode loads the
* project layer AFTER the OPENCODE_CONFIG layer, so such a block wins over the generated
* one and the child may talk to a stale base URL. The launcher cannot outrank that layer
* without writing to a user file, so it warns instead.
*/
export function projectConfigOverridesProvider(cwd: string): string | null {
const candidate = join(cwd, "opencode.json");
if (!existsSync(candidate)) return null;
try {
const parsed = parseJsonc(readFileSync(candidate, "utf8"));
if (!isRecord(parsed) || !isRecord(parsed.provider)) return null;
return OPENCODE_PROVIDER_ID in parsed.provider ? candidate : null;
} catch {
return null; // opencode will report its own parse failure; not this command's business
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does opencode CLI merge opencode.json config layers deeply per provider key, or replace the whole provider object on a top-level key collision?

💡 Result:

The OpenCode CLI performs a deep merge of configuration layers, rather than replacing the entire provider object on a key collision [1][2][3]. When multiple configuration files are loaded, the CLI uses a deep merge strategy to combine settings from different locations (such as remote, global, and project-specific files) [1][4][3]. Specifically, the implementation utilizes a mergeDeep function—often wrapped by a custom utility like mergeConfigConcatArrays—to ensure that nested structures are combined [5][6][3]. As a result, if you have a provider defined in a global configuration file and add or override specific fields (like options or models) for that same provider in a project-specific opencode.json file, the CLI will merge those settings at the appropriate nested level instead of discarding the entire provider entry from the lower-precedence configuration [1][2][3]. Non-conflicting settings are preserved, and conflicting keys are updated according to the established precedence order [1][4][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- src/cli/opencode.ts (outline) ---'
ast-grep outline src/cli/opencode.ts --view expanded || true

echo
echo '--- usages of projectConfigOverridesProvider ---'
rg -n "projectConfigOverridesProvider|OPENCODE_PROVIDER_ID|opencode.json" src -S

echo
echo '--- relevant slice of src/cli/opencode.ts ---'
sed -n '250,360p' src/cli/opencode.ts

Repository: lidge-jun/opencodex

Length of output: 9367


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- locate config docs / resolver references ---'
rg -n "deep merge|mergeDeep|mergeConfig|config resolution|project config|opencode.json" . -S

Repository: lidge-jun/opencodex

Length of output: 3932


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- src/cli/opencode.ts (outline) ---'
ast-grep outline src/cli/opencode.ts --view expanded || true

echo
echo '--- usages of projectConfigOverridesProvider ---'
rg -n "projectConfigOverridesProvider|OPENCODE_PROVIDER_ID|opencode.json" src -S

echo
echo '--- relevant slice of src/cli/opencode.ts ---'
sed -n '250,360p' src/cli/opencode.ts

echo
echo '--- locate config docs / resolver references ---'
rg -n "deep merge|mergeDeep|mergeConfig|config resolution|project config|opencode.json" . -S

Repository: lidge-jun/opencodex

Length of output: 13143


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- call sites of projectConfigOverridesProvider ---'
rg -n "projectConfigOverridesProvider\(" src -S

echo
echo '--- config resolution references in repo docs/code ---'
rg -n "search.*upward|walk.*upward|project root|opencode.json" src docs . -S

Repository: lidge-jun/opencodex

Length of output: 3805


🌐 Web query:

OpenCode project config resolution search parent directories opencode.json upward project root docs

💡 Result:

OpenCode uses a hierarchical configuration resolution system that traverses upwards from the current working directory to find and merge opencode.json (or opencode.jsonc) files [1][2][3]. ### Configuration Resolution Mechanism When OpenCode starts, it performs a search for configuration files using the following logic: * Upward Traversal: It searches for opencode.json or opencode.jsonc files starting from the current directory, traversing upward through parent directories until it reaches the nearest Git repository root or the filesystem root if no Git repository is detected [1][2][4]. * Merging Strategy: Found configuration files are merged, typically with closer (lower-level) configurations overriding settings found in higher-level (ancestor) directories [5][2]. * Directory-Based Configs: Beyond the primary opencode.json file, OpenCode also searches for .opencode directories along the same path. These directories are used to store modular configurations for agents, commands, and plugins [6][1][7]. ### Precedence Order The system follows a specific precedence order, where sources loaded later override earlier ones [1][3]: 1. Remote configuration (e.g., from .well-known/opencode) [1][7]. 2. Global configuration (e.g., ~/.config/opencode/opencode.json) [1][7]. 3. Custom environment variables (e.g., OPENCODE_CONFIG) [1][7]. 4. Project configuration (traversed opencode.json / opencode.jsonc files) [1][7]. 5. .opencode directories [1][7]. 6. Inline configuration (e.g., OPENCODE_CONFIG_CONTENT environment variable) [1][7]. 7. Managed/System-level configuration (highest precedence, non-overridable by users) [6][1][3]. ### Controlling Inheritance To manage or disable the default upward traversal behavior, users can employ specific controls: * Environment Variable: The OPENCODE_NO_PARENT_CONFIG environment variable can be set to true to restrict OpenCode to only load configuration from the current directory, effectively disabling parent directory inheritance [8]. * Custom Path: The OPENCODE_CONFIG_DIR environment variable allows users to specify an explicit directory for agent, command, and plugin configurations, which is then processed as part of the resolution chain [1][7][4].

Citations:


Search ancestor directories for opencode.json. src/cli/opencode.ts:308-315 only checks cwd/opencode.json, but OpenCode walks project config upward to the nearest Git/root directory. A nested package-level config can still override provider.opencodex and skip this warning. Walk parent directories (or reuse OpenCode’s resolver) before returning null.

🤖 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/cli/opencode.ts` around lines 302 - 318, Update
projectConfigOverridesProvider to search for opencode.json in cwd and each
ancestor directory up to the project or filesystem root, matching OpenCode’s
resolution order. Inspect each existing config for the provider key and return
the applicable config path so nested invocations still trigger the warning;
return null only when no ancestor config overrides the provider.

Comment thread src/cli/opencode.ts
Comment on lines +400 to +409
const { fetchAllModels } = await import("../server/management-api");
const { filterCatalogVisibleModels, nativeOpenAiContextWindow, visibleNativeSlugs } = await import("../codex/catalog");
const allModels = await fetchAllModels(config);
const routed = filterCatalogVisibleModels(allModels, config).map(m => ({
provider: m.provider,
id: m.id,
contextWindow: m.contextWindow,
displayName: m.displayName,
}));
const nativeSlugs = [...visibleNativeSlugs(config)];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

fetchAllModels(config) isn't guarded by try/catch, unlike every other I/O step in this function.

Lines 413-417 and 422-426 both wrap their disk I/O in try/catch and print a friendly console.error before returning 1. fetchAllModels(config) at Line 402 — a network call to every configured upstream provider — has no such guard. If it throws (provider misconfiguration, DNS/TLS failure, an upstream 5xx during catalog probing, etc.), cmdOpencode throws an unhandled rejection and the user gets a raw stack trace instead of the clean, actionable error messages this function otherwise consistently provides.

🛠️ Proposed fix
   const { fetchAllModels } = await import("../server/management-api");
   const { filterCatalogVisibleModels, nativeOpenAiContextWindow, visibleNativeSlugs } = await import("../codex/catalog");
-  const allModels = await fetchAllModels(config);
+  let allModels: Awaited<ReturnType<typeof fetchAllModels>>;
+  try {
+    allModels = await fetchAllModels(config);
+  } catch (error) {
+    const reason = error instanceof Error ? error.message : String(error);
+    console.error(`❌ Could not fetch the model catalog from the proxy: ${reason}`);
+    return 1;
+  }
   const routed = filterCatalogVisibleModels(allModels, config).map(m => ({
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { fetchAllModels } = await import("../server/management-api");
const { filterCatalogVisibleModels, nativeOpenAiContextWindow, visibleNativeSlugs } = await import("../codex/catalog");
const allModels = await fetchAllModels(config);
const routed = filterCatalogVisibleModels(allModels, config).map(m => ({
provider: m.provider,
id: m.id,
contextWindow: m.contextWindow,
displayName: m.displayName,
}));
const nativeSlugs = [...visibleNativeSlugs(config)];
const { fetchAllModels } = await import("../server/management-api");
const { filterCatalogVisibleModels, nativeOpenAiContextWindow, visibleNativeSlugs } = await import("../codex/catalog");
let allModels: Awaited<ReturnType<typeof fetchAllModels>>;
try {
allModels = await fetchAllModels(config);
} catch (error) {
const reason = error instanceof Error ? error.message : String(error);
console.error(`❌ Could not fetch the model catalog from the proxy: ${reason}`);
return 1;
}
const routed = filterCatalogVisibleModels(allModels, config).map(m => ({
provider: m.provider,
id: m.id,
contextWindow: m.contextWindow,
displayName: m.displayName,
}));
const nativeSlugs = [...visibleNativeSlugs(config)];
🤖 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/cli/opencode.ts` around lines 400 - 409, Wrap the fetchAllModels(config)
call in the same try/catch error-handling pattern used by the disk I/O blocks in
cmdOpencode. On failure, print a friendly actionable console.error and return 1,
while preserving the existing model filtering flow for successful fetches.

@lidge-jun

Copy link
Copy Markdown
Owner

🔒 Under maintainer review — detailed feedback incoming

@lidge-jun (maintainer) has this PR in an active review pass. Please do not merge, rebase, or
close it
until the detailed review lands; a full comment with specific file:line findings,
failure modes, and suggested fixes is being prepared.

This is a claim marker so two maintainers do not review or land the same PR at once. If you are a
maintainer and need this PR urgently, please reply here first instead of merging.

Baseline for this pass: dev is at bef0dfbe (yesterday's integration batch ebc62d1f plus
#458, #456, #391, #392, #463, #465). If this PR touches the same files, the upcoming review will
name the exact overlapping paths so you only have to rebase once.

No action needed from you until then. Thanks for the contribution and for your patience.

Review tracker: devlog/_plan/260726_pr_review_pass · marker posted by the maintainer review pass

@lidge-jun

Copy link
Copy Markdown
Owner

Thanks for the ocx opencode launcher — wiring OpenCode through the local proxy is a good addition, and the process-spawn side of this is done carefully. Two credential-lifecycle problems block merge.

1. High — the generated file duplicates arbitrary user secrets. src/cli/opencode.ts:284 reads the entire user configuration, :267 merges every field into the generated object, and :418 persists the result to ~/.opencodex/opencode-config.json. Any literal provider API keys, tokens, or MCP credentials in the source config are copied into a second persistent file and stay there after the user removes or rotates them in the original. Mode 0600 limits who can read it but does not fix the duplicated-secret lifecycle. Please keep OpenCode's existing config layers intact and inject only the generated provider.opencodex block through OPENCODE_CONFIG_CONTENT — OpenCode documents merged config layers with the inline runtime layer taking higher precedence, so nothing else needs to be materialized. See https://opencode.ai/docs/config

2. High — copying the config silently changes what relative {file:...} references resolve to. src/cli/opencode.ts:293 parses the source config but :421 writes it under a different directory, and OpenCode resolves relative file references against the containing config file. A {file:./secret} or relative instruction path that worked under ~/.config/opencode/ now resolves under ~/.opencodex/, so the launcher can fail authentication, read the wrong file, or silently drop instructions. The same fix applies: don't relocate the base config, add only the generated runtime override.

3. Medium — project-override detection doesn't match OpenCode's lookup rules. src/cli/opencode.ts:308 checks only ${cwd}/opencode.json, but OpenCode also supports JSONC and walks upward toward the nearest Git directory. A parent opencode.json, or any opencode.jsonc, can override provider.opencodex without triggering your warning while the launcher still tells the user it's wired to the local proxy. Either mirror the upward JSON/JSONC search or rely on the inline runtime layer, which outranks project config.

What I checked and found clean: no argument-injection defect — src/cli/opencode.ts:439 routes through the existing commandInvocation() helper, which preserves argument boundaries and handles Windows .cmd/.bat shims via escaped cmd.exe. PATH failure handling covers POSIX ENOENT and Windows exit 9009. The Node compatibility imports match existing Bun-native usage in this repo and introduce no compile-step assumption.

Overlap with dev: 50 commits behind the bef0dfbe baseline. A synthetic merge finds no textual conflict, but please rebase — GitHub currently reports UNSTABLE and only enforce-target and label have run on this head. The required macOS/Ubuntu/Windows and npm-global matrix has not executed at all yet.

Test gap: tests/opencode-cli.test.ts exercises pure helpers but never invokes cmdOpencode(). There is no command-level regression for proxy startup failure, config write failure, exact child argv and environment, ENOENT, signal exit behavior, or cleanup after a failed spawn. A dependency-injected launcher test plus cross-platform smoke coverage would be the right addition once the config-copy behavior is removed.


Reviewed as part of a maintainer review pass against dev at bef0dfbe, snapshot 2026-07-26T08:35+09:00, at head 5ef84f48.

@Wibias
Wibias marked this pull request as draft July 26, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants