Skip to content

feat(integrations): add OMP client support - #1109

Draft
n3wr1ch wants to merge 3 commits into
lidge-jun:devfrom
n3wr1ch:feat/omp-integration
Draft

feat(integrations): add OMP client support#1109
n3wr1ch wants to merge 3 commits into
lidge-jun:devfrom
n3wr1ch:feat/omp-integration

Conversation

@n3wr1ch

@n3wr1ch n3wr1ch commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add OMP as a first-class file integration backed by ~/.omp/agent/models.yml
  • export the complete routed opencodex catalog in OMP's YAML provider schema, including non-GPT models and capability metadata
  • expose OMP in the GUI dashboard, management API, CLI help, localized copy, and integration documentation
  • keep OMP loopback-only and serialize only the existing non-secret opencodex-loopback placeholder

Dependency

The prerequisite Pi visibility fix has landed on dev via #1151. This branch is now rebased onto current dev; the inherited #1085 commits and the broader export-policy changes that were not part of OMP have been removed.

Overlap

#1109 was opened first and intentionally remained draft while the Pi prerequisite was unresolved. #1140 now overlaps the same OMP integration. This update keeps #1109 focused on its original OMP contract so the maintainers can review the implementations without the former stacked diff.

Security review

This adds a credential destination and requires explicit maintainer security review. No user or provider secret is serialized: OMP receives the non-secret opencodex-loopback placeholder, and integration writes remain restricted to loopback proxy binds. Upstream provider credentials stay in opencodex's own configuration.

Verification

Verified on head 6860b6c4 rebased onto dev at 1fc24f03:

  • bun run typecheck
  • bun run test — 9,633 pass, 8 skip, 0 fail
  • bun run privacy:scan
  • cd gui && bun test tests — 646 pass, 0 fail
  • cd gui && bun run lint
  • cd gui && bun run lint:i18n
  • cd gui && bun run build
  • cd docs-site && bun install --frozen-lockfile && bun run build
  • focused client integration/export suites — 152 pass, 0 fail
  • real OMP catalog load from the original implementation: omp models opencodex --json recognized routed GPT, Claude, and Grok models

Screenshot

The GUI exposes OMP as its own integration tab and reports the generated models.yml destination and restart guidance.

OMP integration tab

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; explicit maintainer security review is requested above.

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 resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • New Features
    • Added OMP as a supported integration with a dedicated GUI tab and localized labels.
    • Added OMP configuration export with YAML output, profile-aware paths, and loopback placeholders instead of real credentials.
    • Expanded client configuration API and CLI support to seven file-based integrations.
  • UI Improvements
    • Renamed “Copy JSON” actions to clearer configuration-focused wording and added copy-failure messaging.
  • Documentation
    • Updated multilingual CLI, API, and integration documentation with OMP support and credential guidance.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds OMP as a supported file integration and export client. It adds profile-aware paths, YAML serialization, loopback credentials, GUI routing, localized labels, API coverage, tests, and documentation updates.

Changes

OMP integration

Layer / File(s) Summary
OMP export paths and serialization
src/clients/config-export.ts, src/cli/*, tests/client-config-export*, tests/cli-export-command.test.ts
Adds OMP profile validation, path resolution, YAML export, loopback credentials, registry wiring, and export tests.
Integration registry and management response
src/integrations/registry.ts, tests/integrations-*, tests/management-client-config-route.test.ts
Registers OMP paths and validates OMP metadata, YAML content, model data, and placeholder credentials.
GUI routing, client lists, and localized labels
gui/src/app-routing.ts, gui/src/components/*, gui/src/pages/*, gui/src/i18n/*, gui/tests/*
Adds the OMP integration tab, file-client support, translated labels and restart semantics, and updated configuration copy terminology.
CLI, API, and integration documentation
docs-site/src/content/docs/guides/integrations.md, docs-site/src/content/docs/*/reference/cli/agents.md, docs-site/src/content/docs/*/reference/management-api.md
Documents OMP exports, destinations, environment overrides, loopback placeholders, supported clients, and generalized client-configuration responses.

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

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant CLI as ocx export
  participant Exporter as EXPORT_CLIENTS.omp
  participant Paths as OMP path resolution
  participant Config as OMP configuration
  Operator->>CLI: request OMP export
  CLI->>Exporter: select omp client
  Exporter->>Paths: resolve profile and models path
  Paths-->>Exporter: return destination
  Exporter->>Config: build YAML with loopback placeholder
  Config-->>CLI: return generated configuration
  CLI-->>Operator: write or display OMP configuration
Loading

Possibly related PRs

Suggested labels: review-ready

Suggested reviewers: ingwannu, wibias, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% 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 clearly and concisely identifies the primary change: adding first-class OMP client support across the integration, export, GUI, API, and documentation surfaces.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@n3wr1ch
n3wr1ch force-pushed the feat/omp-integration branch from ca9183d to 6860b6c Compare August 7, 2026 04:04
@github-actions
github-actions Bot marked this pull request as ready for review August 7, 2026 04:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
docs-site/src/content/docs/ja/reference/cli/agents.md (1)

136-148: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Update the localized CLI output contract.

Both pages list seven clients in the command heading but still restrict the --client flag to opencode|pi. Both pages also state that --json produces JSON, although OMP, Hermes, and Gajae produce YAML, OpenClaw produces JSON5, and Kimi produces TOML.

  • docs-site/src/content/docs/ja/reference/cli/agents.md#L136-L148: List all seven client IDs. Replace JSON-only output wording with generated configuration bytes.
  • docs-site/src/content/docs/ko/reference/cli/agents.md#L142-L154: List all seven client IDs. Replace JSON-only output wording with generated configuration bytes.

As per path instructions, translated locale pages must not contradict the English source.

🤖 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/ja/reference/cli/agents.md` around lines 136 -
148, Update the CLI documentation in
docs-site/src/content/docs/ja/reference/cli/agents.md lines 136-148 and
docs-site/src/content/docs/ko/reference/cli/agents.md lines 142-154: change the
--client option to list all seven supported client IDs, and revise --json
wording to describe generated configuration bytes rather than JSON-only output,
matching the English source and accounting for each client’s format.

Source: Path instructions

docs-site/src/content/docs/ru/reference/cli/agents.md (1)

155-193: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The localized CLI references retain the same client-list and output-format drift.

Both pages add OMP to the heading and examples but leave the client option and destination tables incomplete. Both also describe --json as JSON-only even though OMP emits YAML.

  • docs-site/src/content/docs/ru/reference/cli/agents.md#L155-L193: Update Line 167, add the four omitted client rows, and describe --json as native-format output.
  • docs-site/src/content/docs/zh-cn/reference/cli/agents.md#L129-L155: Update Line 137, add the four omitted client rows, and describe --json as native-format output.

As per path instructions, localized documentation must stay consistent with actual CLI and API behavior.

🤖 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/ru/reference/cli/agents.md` around lines 155 -
193, Update docs-site/src/content/docs/ru/reference/cli/agents.md lines 155-193
and docs-site/src/content/docs/zh-cn/reference/cli/agents.md lines 129-155
consistently: revise the client option description to include all supported
clients, add the four missing client rows to each destination table, and
describe --json as native-format output so OMP’s YAML is covered rather than
claiming JSON-only output.

Source: Path instructions

🤖 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/integrations.md`:
- Around line 19-22: Update docs-site/src/content/docs/guides/integrations.md
lines 19-22 to include PI_PROFILE and document ompAgentDir() precedence:
OMP_PROFILE overrides PI_PROFILE; with an active profile, PI_CODING_AGENT_DIR is
ignored and PI_CONFIG_DIR supplies the profile root; without a profile,
PI_CODING_AGENT_DIR takes precedence. Apply the same precedence rule in
docs-site/src/content/docs/reference/cli/agents.md line 180, replacing the flat
variable list.

In `@tests/management-client-config-route.test.ts`:
- Around line 162-176: Extend the OMP test around clientConfigApi and
PiGeneratedConfig to assert the complete routed model catalog: a/m1, a/m2, and
b/no-context. Also verify the exported models preserve their capability and
reasoning metadata, including the expected modelContextWindows values, while
retaining the existing provider and API key assertions.
- Around line 162-176: Update the OMP test around clientConfigApi and the
body.config assertions to verify that neither body.text nor the serialized
body.config contains REAL_LOOKING_KEY. Keep the existing provider model and
placeholder API-key assertions unchanged.

---

Outside diff comments:
In `@docs-site/src/content/docs/ja/reference/cli/agents.md`:
- Around line 136-148: Update the CLI documentation in
docs-site/src/content/docs/ja/reference/cli/agents.md lines 136-148 and
docs-site/src/content/docs/ko/reference/cli/agents.md lines 142-154: change the
--client option to list all seven supported client IDs, and revise --json
wording to describe generated configuration bytes rather than JSON-only output,
matching the English source and accounting for each client’s format.

In `@docs-site/src/content/docs/ru/reference/cli/agents.md`:
- Around line 155-193: Update
docs-site/src/content/docs/ru/reference/cli/agents.md lines 155-193 and
docs-site/src/content/docs/zh-cn/reference/cli/agents.md lines 129-155
consistently: revise the client option description to include all supported
clients, add the four missing client rows to each destination table, and
describe --json as native-format output so OMP’s YAML is covered rather than
claiming JSON-only output.
🪄 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: cfc8af1a-b446-49d9-b51a-426e76c08cf9

📥 Commits

Reviewing files that changed from the base of the PR and between 1fc24f0 and 6860b6c.

⛔ Files ignored due to path filters (1)
  • devlog/_plan/260806_omp_integration/evidence/omp-dashboard.png is excluded by !**/*.png
📒 Files selected for processing (35)
  • docs-site/src/content/docs/guides/integrations.md
  • docs-site/src/content/docs/ja/reference/cli/agents.md
  • docs-site/src/content/docs/ja/reference/management-api.md
  • docs-site/src/content/docs/ko/reference/cli/agents.md
  • docs-site/src/content/docs/ko/reference/management-api.md
  • docs-site/src/content/docs/reference/cli/agents.md
  • docs-site/src/content/docs/reference/management-api.md
  • docs-site/src/content/docs/ru/reference/cli/agents.md
  • docs-site/src/content/docs/ru/reference/management-api.md
  • docs-site/src/content/docs/zh-cn/reference/cli/agents.md
  • docs-site/src/content/docs/zh-cn/reference/management-api.md
  • gui/src/app-routing.ts
  • gui/src/components/apikeys-workspace/client-config-clients.ts
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Integrations.tsx
  • gui/src/pages/integrations/FileIntegrationPage.tsx
  • gui/src/pages/integrations/integration-api.ts
  • gui/src/pages/integrations/overview-clients.ts
  • gui/tests/client-config-panel.test.tsx
  • gui/tests/integrations-overview-rows.test.ts
  • src/cli/export-command.ts
  • src/cli/help.ts
  • src/clients/config-export.ts
  • src/integrations/registry.ts
  • tests/cli-export-command.test.ts
  • tests/client-config-export-new-clients.test.ts
  • tests/client-config-export.test.ts
  • tests/integrations-invariants.test.ts
  • tests/integrations-state.test.ts
  • tests/management-client-config-route.test.ts

Comment thread docs-site/src/content/docs/guides/integrations.md Outdated
Comment on lines +162 to +176
test("OMP returns the full routed catalog as models.yml YAML", async () => {
const response = await clientConfigApi(baseConfig(), "?client=omp");
expect(response.status).toBe(200);
const body = await response.json() as ClientConfigEnvelope;

expect(body.client).toBe("omp");
expect(body.filename).toBe("omp-models.yaml");
expect(body.format).toBe("yaml");
expect(body.text).toContain("providers:");
expect(body.text).toContain("a/m1");
const provider = (body.config as PiGeneratedConfig).providers[OPENCODE_PROVIDER_ID];
expect(provider.models.map(model => model.id)).toContain("a/m1");
expect(provider.apiKey).toBe(LOOPBACK_API_KEY_PLACEHOLDER);
}, 15_000);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Extend the OMP regression to cover the complete routed catalog.

baseConfig() supplies a/m1, a/m2, b/no-context, and modelContextWindows, but the test only checks a/m1. An exporter that drops non-GPT models or capability metadata can still pass. Assert all expected routed model IDs and the capability and reasoning fields that OMP must preserve.

As per path instructions, behavior changes in src/ need a focused regression test near the existing subsystem tests.

🤖 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 `@tests/management-client-config-route.test.ts` around lines 162 - 176, Extend
the OMP test around clientConfigApi and PiGeneratedConfig to assert the complete
routed model catalog: a/m1, a/m2, and b/no-context. Also verify the exported
models preserve their capability and reasoning metadata, including the expected
modelContextWindows values, while retaining the existing provider and API key
assertions.

Source: Path instructions


🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Assert that the OMP response contains no upstream credentials.

baseConfig() stores REAL_LOOKING_KEY in provider and API-key fields, but the test checks only the generated provider's apiKey. Add assertions that body.text and the serialized body.config do not contain REAL_LOOKING_KEY.

Proposed regression assertions
     expect(provider.apiKey).toBe(LOOPBACK_API_KEY_PLACEHOLDER);
+    expect(body.text).not.toContain(REAL_LOOKING_KEY);
+    expect(JSON.stringify(body.config)).not.toContain(REAL_LOOKING_KEY);

Based on PR objectives, upstream credentials must remain in opencodex configuration and must not be serialized into management responses.

📝 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
test("OMP returns the full routed catalog as models.yml YAML", async () => {
const response = await clientConfigApi(baseConfig(), "?client=omp");
expect(response.status).toBe(200);
const body = await response.json() as ClientConfigEnvelope;
expect(body.client).toBe("omp");
expect(body.filename).toBe("omp-models.yaml");
expect(body.format).toBe("yaml");
expect(body.text).toContain("providers:");
expect(body.text).toContain("a/m1");
const provider = (body.config as PiGeneratedConfig).providers[OPENCODE_PROVIDER_ID];
expect(provider.models.map(model => model.id)).toContain("a/m1");
expect(provider.apiKey).toBe(LOOPBACK_API_KEY_PLACEHOLDER);
}, 15_000);
test("OMP returns the full routed catalog as models.yml YAML", async () => {
const response = await clientConfigApi(baseConfig(), "?client=omp");
expect(response.status).toBe(200);
const body = await response.json() as ClientConfigEnvelope;
expect(body.client).toBe("omp");
expect(body.filename).toBe("omp-models.yaml");
expect(body.format).toBe("yaml");
expect(body.text).toContain("providers:");
expect(body.text).toContain("a/m1");
const provider = (body.config as PiGeneratedConfig).providers[OPENCODE_PROVIDER_ID];
expect(provider.models.map(model => model.id)).toContain("a/m1");
expect(provider.apiKey).toBe(LOOPBACK_API_KEY_PLACEHOLDER);
expect(body.text).not.toContain(REAL_LOOKING_KEY);
expect(JSON.stringify(body.config)).not.toContain(REAL_LOOKING_KEY);
}, 15_000);
🤖 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 `@tests/management-client-config-route.test.ts` around lines 162 - 176, Update
the OMP test around clientConfigApi and the body.config assertions to verify
that neither body.text nor the serialized body.config contains REAL_LOOKING_KEY.
Keep the existing provider model and placeholder API-key assertions unchanged.

@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: 6860b6c4ac

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

Comment thread src/clients/config-export.ts Outdated
const OMP_WINDOWS_RESERVED_PROFILE_RE = /^(?:CON|PRN|AUX|NUL|COM[0-9]|LPT[0-9])(?:\..*)?$/i;

function ompProfileName(env: OpencodeLaunchEnv): string | undefined {
const raw = env.OMP_PROFILE?.trim() || env.PI_PROFILE?.trim();

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 Honor empty OMP_PROFILE before falling back

The upstream OMP environment docs state that OMP_PROFILE wins over PI_PROFILE even when explicitly empty, and PI_PROFILE is used only when OMP_PROFILE is undefined (https://github.com/can1357/oh-my-pi/blob/main/docs/environment-variables.md#6-storage-and-config-root-paths). This || fallback treats OMP_PROFILE="" or whitespace as absent, so anyone clearing OMP_PROFILE to force the default profile while a legacy PI_PROFILE=work remains set will have opencodex write ~/.omp/profiles/work/agent/models.yml while OMP reads the default profile. Check property presence instead of truthiness before falling back.

Useful? React with 👍 / 👎.

| `--client <opencode\|pi>` | Required. Selects the client dialect: opencode's keyed `provider` object or Pi's `providers` array. |
| `--json` | Print only the config JSON on stdout, so a redirect captures byte-exact output. Every diagnostic, including the `--out` write note, goes to stderr. |
| `--client <opencode\|pi\|omp\|hermes\|openclaw\|kimi\|gajae>` | Required. Selects the client config dialect. |
| `--json` | Print only the generated config bytes on stdout, so a redirect captures byte-exact output. Every diagnostic, including the `--out` write note, goes to stderr. |

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 Keep --json docs aligned with actual stdout

This now documents --json as safe native config bytes, but handleExportCommand still sends the document object through printData when wantsJson is true, which JSON-stringifies it instead of emitting built.text. Users following this for non-JSON clients such as Kimi Code or OpenClaw will redirect JSON into a .toml or .json5 config file rather than the client's native serialization; either document the existing JSON behavior or change the CLI before advertising redirects this way.

Useful? React with 👍 / 👎.

Comment on lines +1015 to +1016
// OMP's provider block has no dedicated admission-header field.
loopbackOnly: true,

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 Stop treating OMP as headerless

For users who bind opencodex to a non-loopback host, this marks OMP as impossible even though OMP's models.yml provider schema supports a headers map and command-resolved header secrets (https://github.com/can1357/oh-my-pi/blob/main/docs/models.md#provider-level-fields). The writer therefore refuses every OMP apply with non_loopback instead of emitting the required x-opencodex-api-key header in a non-secret form, so remote/LAN or wildcard-bound setups cannot use the new integration even though the client can carry the required admission header.

Useful? React with 👍 / 👎.

@n3wr1ch n3wr1ch Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verified against OMP’s current models.yml contract: provider-level headers are supported, and header values are resolved through the same environment/command config-value path. The current comment and loopbackOnly classification therefore rest on an inaccurate schema assumption.

There are two reasonable scopes:

  1. Keep this PR deliberately loopback-only, correct the explanation to describe that as an opencodex policy limitation, and handle remote OMP support separately.
  2. Add an OMP-specific x-opencodex-api-key environment reference in this PR and remove loopbackOnly; this expands the credential-destination/security-review surface.

I have fixed the independent profile-precedence and --json documentation defects at e35bc9b8, but left this security-sensitive choice unchanged pending maintainer direction. Which scope do you prefer?

@github-actions
github-actions Bot marked this pull request as draft August 7, 2026 04:31

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@docs-site/src/content/docs/ja/reference/cli/agents.md`:
- Line 154: Synchronize the OMP destination metadata rows in
docs-site/src/content/docs/ja/reference/cli/agents.md:154-154,
docs-site/src/content/docs/ko/reference/cli/agents.md:160-160,
docs-site/src/content/docs/ru/reference/cli/agents.md:187-187, and
docs-site/src/content/docs/zh-cn/reference/cli/agents.md:155-155: mark
~/.omp/agent/models.yml as the default path, document OMP_PROFILE/PI_PROFILE
precedence, and replace the generic placeholder with the literal
opencodex-loopback value in each localized table.
- Around line 141-144: Update the localized export-output descriptions to
reflect that non-JSON output is generated client configuration or the selected
native format, rather than always JSON. In
docs-site/src/content/docs/ja/reference/cli/agents.md lines 141-144, revise the
adjacent Line 148 wording; apply the equivalent correction in
docs-site/src/content/docs/ko/reference/cli/agents.md lines 147-150 for Line
154, docs-site/src/content/docs/ru/reference/cli/agents.md lines 172-175 for
Lines 179-181, and docs-site/src/content/docs/zh-cn/reference/cli/agents.md
lines 142-145 for Line 149.

In `@tests/client-config-export.test.ts`:
- Around line 479-488: Make the test around EXPORT_CLIENTS.omp.destination
deterministic by isolating homedir() or mocking the filesystem check used by
ompModelsConfigPath, so the real test account cannot affect the selected
filename. Preserve separate assertions covering both canonical models.yml and
legacy models.yaml resolution.
🪄 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: cc38f932-0454-4ec6-a12e-105f1d85597c

📥 Commits

Reviewing files that changed from the base of the PR and between 6860b6c and e35bc9b.

📒 Files selected for processing (10)
  • docs-site/src/content/docs/guides/integrations.md
  • docs-site/src/content/docs/ja/reference/cli/agents.md
  • docs-site/src/content/docs/ko/reference/cli/agents.md
  • docs-site/src/content/docs/reference/cli/agents.md
  • docs-site/src/content/docs/ru/reference/cli/agents.md
  • docs-site/src/content/docs/zh-cn/reference/cli/agents.md
  • src/cli/export-command.ts
  • src/cli/help.ts
  • src/clients/config-export.ts
  • tests/client-config-export.test.ts

Comment on lines 141 to +144
```bash
ocx export --client opencode # config plus destination, merge warning, and counts
ocx export --client pi --json > pi-models.json # byte-exact JSON for a pipe or a diff
ocx export --client pi --json > pi-models.json # JSON document for a pipe or a diff
ocx export --client omp --out ./omp-models.yml # native OMP YAML

Copy link
Copy Markdown
Contributor

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

Correct the localized non---json output description.

handleExportCommand passes built.text to printData when --json is absent, so OMP emits YAML. The localized pages still say that JSON appears first.

  • docs-site/src/content/docs/ja/reference/cli/agents.md#L141-L144: Update the adjacent Line 148 wording to describe the generated client config or selected native format.
  • docs-site/src/content/docs/ko/reference/cli/agents.md#L147-L150: Update the adjacent Line 154 wording to describe the generated client config or selected native format.
  • docs-site/src/content/docs/ru/reference/cli/agents.md#L172-L175: Update Lines 179-181 to describe the generated client config or selected native format.
  • docs-site/src/content/docs/zh-cn/reference/cli/agents.md#L142-L145: Update Line 149 to describe the generated client config or selected native format.

As per path instructions, localized documentation must match the actual CLI output format.

📍 Affects 4 files
  • docs-site/src/content/docs/ja/reference/cli/agents.md#L141-L144 (this comment)
  • docs-site/src/content/docs/ko/reference/cli/agents.md#L147-L150
  • docs-site/src/content/docs/ru/reference/cli/agents.md#L172-L175
  • docs-site/src/content/docs/zh-cn/reference/cli/agents.md#L142-L145
🤖 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/ja/reference/cli/agents.md` around lines 141 -
144, Update the localized export-output descriptions to reflect that non-JSON
output is generated client configuration or the selected native format, rather
than always JSON. In docs-site/src/content/docs/ja/reference/cli/agents.md lines
141-144, revise the adjacent Line 148 wording; apply the equivalent correction
in docs-site/src/content/docs/ko/reference/cli/agents.md lines 147-150 for Line
154, docs-site/src/content/docs/ru/reference/cli/agents.md lines 172-175 for
Lines 179-181, and docs-site/src/content/docs/zh-cn/reference/cli/agents.md
lines 142-145 for Line 149.

Source: Path instructions

| `opencode` | `~/.config/opencode/opencode.json` (設定すると `XDG_CONFIG_HOME` が勝ち) | `opencode.json` | `OPENCODEX_OPENCODE_API_KEY` |
| `pi` | `~/.pi/agent/models.json` | `pi-models.json` | `OPENCODEX_API_KEY` |
| `pi` | `~/.pi/agent/models.json` | `pi-models.json` | なし - ブロックにリテラル `opencodex-loopback` が入ります |
| `omp` | `~/.omp/agent/models.yml` | `omp-models.yaml` | なし - loopback placeholder |

Copy link
Copy Markdown
Contributor

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

Synchronize localized OMP destination metadata with runtime behavior.

OMP paths are profile-aware, and the exporter uses the literal non-secret opencodex-loopback placeholder. Each localized row currently shows only the default path and a generic placeholder.

  • docs-site/src/content/docs/ja/reference/cli/agents.md#L154-L154: Mark ~/.omp/agent/models.yml as the default, document OMP_PROFILE/PI_PROFILE precedence, and name opencodex-loopback.
  • docs-site/src/content/docs/ko/reference/cli/agents.md#L160-L160: Mark ~/.omp/agent/models.yml as the default, document OMP_PROFILE/PI_PROFILE precedence, and name opencodex-loopback.
  • docs-site/src/content/docs/ru/reference/cli/agents.md#L187-L187: Mark ~/.omp/agent/models.yml as the default, document OMP_PROFILE/PI_PROFILE precedence, and name opencodex-loopback.
  • docs-site/src/content/docs/zh-cn/reference/cli/agents.md#L155-L155: Mark ~/.omp/agent/models.yml as the default, document OMP_PROFILE/PI_PROFILE precedence, and name opencodex-loopback.

As per path instructions, localized documentation must stay synchronized with actual OMP path and credential behavior.

📍 Affects 4 files
  • docs-site/src/content/docs/ja/reference/cli/agents.md#L154-L154 (this comment)
  • docs-site/src/content/docs/ko/reference/cli/agents.md#L160-L160
  • docs-site/src/content/docs/ru/reference/cli/agents.md#L187-L187
  • docs-site/src/content/docs/zh-cn/reference/cli/agents.md#L155-L155
🤖 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/ja/reference/cli/agents.md` at line 154,
Synchronize the OMP destination metadata rows in
docs-site/src/content/docs/ja/reference/cli/agents.md:154-154,
docs-site/src/content/docs/ko/reference/cli/agents.md:160-160,
docs-site/src/content/docs/ru/reference/cli/agents.md:187-187, and
docs-site/src/content/docs/zh-cn/reference/cli/agents.md:155-155: mark
~/.omp/agent/models.yml as the default path, document OMP_PROFILE/PI_PROFILE
precedence, and replace the generic placeholder with the literal
opencodex-loopback value in each localized table.

Source: Path instructions

Comment on lines +479 to +488
// OMP_PROFILE wins by presence, so an explicit blank selects the default
// profile instead of inheriting a legacy PI_PROFILE.
expect(EXPORT_CLIENTS.omp.destination({
OMP_PROFILE: " ",
PI_PROFILE: "legacy",
} as NodeJS.ProcessEnv)).toBe(
join(homedir(), ".omp", "agent", "models.yml"),
);
});

Copy link
Copy Markdown
Contributor

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

Make this destination-path test independent of the real home directory.

EXPORT_CLIENTS.omp.destination(...) uses homedir(), while ompModelsConfigPath can select an existing legacy models.yaml. If that file exists under the test account, this assertion expects models.yml but the implementation returns models.yaml. Use an isolated temporary home or mock the existence check. Keep separate coverage for the canonical and legacy paths.

As per path instructions, this focused regression test must remain reliable across test environments.

🤖 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 `@tests/client-config-export.test.ts` around lines 479 - 488, Make the test
around EXPORT_CLIENTS.omp.destination deterministic by isolating homedir() or
mocking the filesystem check used by ompModelsConfigPath, so the real test
account cannot affect the selected filename. Preserve separate assertions
covering both canonical models.yml and legacy models.yaml resolution.

Source: Path instructions

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.

1 participant