feat(integrations): add Oh My Pi (omp) client integration - #1140
feat(integrations): add Oh My Pi (omp) client integration#1140hanbinnoh wants to merge 8 commits into
Conversation
omp 17.x reads custom providers from ~/.omp/agent/models.yml, so the existing pi integration (legacy ~/.pi/agent/models.json) cannot reach it. Add an 'omp' export/integration client that writes the opencodex provider block as models.yml, honoring omp's profile and agent-dir overrides, and wire it through the CLI, management API, GUI and docs.
📝 WalkthroughWalkthroughAdds Oh My Pi as a seventh export client. The change generates OMP provider YAML, registers its paths and integration state, exposes GUI integration controls and translations, updates CLI help, and adds documentation and tests. ChangesOh My Pi integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as ocx export
participant Export as config-export
participant Registry as integrations registry
participant GUI as Integrations GUI
participant OMP as OMP models.yml
CLI->>Export: select --client omp
Export->>Registry: resolve OMP path and metadata
Export->>OMP: write provider YAML
GUI->>Registry: load OMP integration state
Registry-->>GUI: return configuration path and client metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a53748d27
ℹ️ 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".
| const profile = env.OMP_PROFILE?.trim() ?? env.PI_PROFILE?.trim(); | ||
| const agentOverride = env.PI_CODING_AGENT_DIR?.trim(); | ||
| if (profile && profile !== "default") return join(root, "profiles", profile, "agent"); | ||
| if (agentOverride && agentOverride.length > 0) return agentOverride; |
There was a problem hiding this comment.
When PI_CODING_AGENT_DIR is relative, this returns a relative config path, so the dashboard integration reads and writes it relative to opencodex's working directory while separately launched omp resolves the same variable relative to its own working directory. The switch can therefore report the integration as current after modifying a file omp never reads, and later disable/restore operations can target another file if the proxy's working directory changes. Expand only stable absolute or ~ paths, or refuse the mutation as the existing OpenClaw path handling does.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/oh-my-pi.md`:
- Line 3: Update the frontmatter description for the Oh My Pi guide to
accurately state that `ocx export --client omp` prints the custom provider YAML,
destination, and merge instructions rather than writing
`~/.omp/agent/models.yml` directly; alternatively, explicitly attribute the
write operation to the dashboard.
- Around line 28-29: Clarify the Oh My Pi configuration example around the
exported YAML by stating that openai-completions is the OMP adapter and
OPENCODEX_OMP_API_KEY is only a local OMP provider key, not an opencodex OAuth
or access-token credential. Add this explanation near the YAML exports without
changing the generated example content.
In `@gui/src/components/apikeys-workspace/client-config-clients.ts`:
- Around line 11-17: Update the OMP entry in CLIENT_LABEL_KEYS and its related
client-config UI copy to avoid JSON-specific wording, using format-neutral
“config” keys or selecting YAML-specific i18n keys based on the client format
declared in config-export. Add or update all user-visible text through the i18n
locale files, including copy buttons, ARIA labels, and announcements.
In `@src/clients/config-export.ts`:
- Around line 368-372: Update the profile selection in ompAgentDir to treat a
trimmed, blank OMP_PROFILE as unset before falling back to PI_PROFILE, while
preserving the existing non-default profile and agentOverride behavior. Add a
regression case covering blank OMP_PROFILE with a configured PI_PROFILE and
assert the PI_PROFILE agent directory is returned.
🪄 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: e51bac55-a2e7-4f23-96a5-e4685e8e2a78
⛔ Files ignored due to path filters (1)
gui-screenshot.pngis excluded by!**/*.png
📒 Files selected for processing (23)
docs-site/astro.config.mjsdocs-site/src/content/docs/guides/integrations.mddocs-site/src/content/docs/guides/oh-my-pi.mdgui/src/components/apikeys-workspace/client-config-clients.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/Integrations.tsxgui/src/pages/integrations/FileIntegrationPage.tsxgui/src/pages/integrations/integration-api.tsgui/src/pages/integrations/overview-clients.tssrc/cli/help.tssrc/clients/config-export.tssrc/integrations/registry.tstests/cli-export-command.test.tstests/client-config-export-new-clients.test.tstests/client-config-export.test.tstests/integrations-invariants.test.tstests/integrations-state.test.tstests/management-client-config-route.test.ts
- Reject a relative PI_CODING_AGENT_DIR instead of silently anchoring it to opencodex's cwd, so the toggle cannot report current against a file omp never reads (Codex P2). - Treat a blank OMP_PROFILE as unset so the legacy PI_PROFILE falls through, matching omp's own env precedence (CodeRabbit). - Register integrations/omp in the app router so the Oh My Pi tab does not bounce back to Overview (the user-visible tab bug). - Make client-config UI copy format-neutral (config instead of JSON) in all six locales and their tests. - Clarify the oh-my-pi guide: ocx export prints, the dashboard writes; openai-completions is the OMP adapter and OPENCODEX_OMP_API_KEY is a local provider key, not an opencodex credential.
The omp integration dropped every model's reasoning metadata because ExportModel never carried reasoningEfforts/defaultReasoningEffort and toExportModel did not pass them through. omp only exposes a reasoning effort control when the model entry declares a thinking block, so without this the omp picker showed no effort selector at all. - ExportModel gains reasoningEfforts/defaultReasoningEffort and model-rows passes them through from the live catalog. - The omp builder emits a thinking block (mode effort) per model with efforts, dropping out-of-vocabulary values so one bad entry cannot fail the whole provider config. - Add moonshotai/Kimi-K3 to the Command Code official effort table (high/max, verified on commandcode.ai/models/kimi-k3) so OAuth-linked Kimi K3 also gets an effort picker. - Tests cover the omp thinking emission and the registry entry.
omp's resolveModelThinking returns undefined unless the model entry sets reasoning: true, so the thinking block alone was silently dropped and the omp picker showed no effort selector for command-code models. Emit reasoning: true alongside the thinking block; verified with a scratch omp agent dir that command-code/deepseek and Kimi-K3 now list high,max.
The official Command Code CLI (v1.14.0) marks Kimi K3 reasoning:true but does not give it a reasoningEfforts array — there is no effort selector in the original client. Adding high/max advertised an effort tier the upstream does not offer. Remove the entry; the existing test already pinned Kimi-K3 as absent from the official effort table.
omp dials POST /v1/chat/completions with the namespaced selector (command-code/deepseek-deepseek-v4-flash, inner '/' encoded as '-'), which the effort table lookup did not recognize, so reasoning_effort was dropped on the floor and the upstream call went out without it. Strip the provider prefix and decode the encoded id against the official effort table (plus compat aliases) before the lookup. Verified end-to-end: omp's reasoning_effort: high now reaches /alpha/generate as params.reasoning_effort.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97eb31048d
ℹ️ 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".
| */ | ||
| export function ompHomeDir(env: OpencodeLaunchEnv = process.env, home: string = homedir()): string { | ||
| const override = env.PI_CONFIG_DIR?.trim(); | ||
| return override && override.length > 0 ? override : join(home, ".omp"); |
There was a problem hiding this comment.
When PI_CONFIG_DIR is relative, ompHomeDir returns it unchanged, so the integration reads and writes models.yml relative to opencodex's working directory while a separately launched omp resolves the same variable from its own working directory. The agent-directory override now avoids this mismatch with absoluteClientPath, but the config-root override can still report a successful/current integration after modifying a file omp never reads; validate or expand PI_CONFIG_DIR through the same helper.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| /** Reasoning levels the proxy will accept for this model (wire vocabulary). */ | ||
| reasoningEfforts?: string[]; | ||
| /** Default reasoning level, when the model carries one. */ | ||
| defaultReasoningEffort?: string; |
There was a problem hiding this comment.
ExportModel now carries the reasoning fields needed to emit omp's thinking block, but exportModelsFromProxyRows in src/cli/export-command.ts still copies only identity, display, context, and modalities from /api/models. Consequently, ocx export --client omp drops every model's reasoningEfforts and defaultReasoningEffort, unlike the dashboard and integration APIs, so its generated config exposes no effort selector; extend that CLI projection to copy both fields and add focused CLI coverage.
AGENTS.md reference: src/AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
| * legacy `~/.pi/agent/models.json` the old Pi exporter targets. | ||
| */ | ||
| export function ompConfigPath(env: OpencodeLaunchEnv = process.env, home: string = homedir()): string { | ||
| return join(ompAgentDir(env, home), "models.yml"); |
There was a problem hiding this comment.
When an omp installation already uses the supported models.yaml filename, this unconditionally targets the sibling models.yml instead of the active file. Applying the integration therefore neither merges nor backs up the user's existing provider document, and creating the preferred .yml candidate can make omp stop loading the existing .yaml providers; select an existing models.yml/models.yaml according to omp's lookup order and default to .yml only when neither exists.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| ...(row.reasoningEfforts ? { reasoningEfforts: row.reasoningEfforts } : {}), | ||
| ...(row.defaultReasoningEffort ? { defaultReasoningEffort: row.defaultReasoningEffort } : {}), |
There was a problem hiding this comment.
toExportModel can forward the new reasoning fields only when they exist on the management row, but the native-row branch in listManagementModelRows constructs each GPT row with identity and context metadata only. The canonical nativeReasoningEfforts and nativeDefaultReasoningEffort helpers already contain these models' ladders and defaults, yet every omp export consequently omits their thinking blocks and never exposes an effort selector for native GPT models; populate the native rows from those helpers before this projection.
AGENTS.md reference: src/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
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/oh-my-pi.md`:
- Around line 39-42: Update the documentation around api: openai-completions to
use the documented OpenAI Chat Completions terminology: describe it as selecting
OMP’s proxy wire dialect, and identify the generated OpenCodex provider as using
the openai-chat Chat Completions-compatible wire format. Preserve the existing
clarification about OPENCODEX_OMP_API_KEY.
In `@src/adapters/command-code.ts`:
- Around line 322-333: Extract the model-ID decoding logic around
`decodeRoutedModelId` into a shared resolver that returns the canonical model
ID, then use that result for both `supportedCommandCodeEffort` and the
`params.model` assignment in the Command Code request. Ensure namespaced OMP
selectors resolve to canonical IDs such as `deepseek/deepseek-v4-flash`, and add
coverage in `tests/command-code-provider.test.ts` asserting the serialized
`params.model` value.
In `@src/clients/config-export.ts`:
- Around line 850-860: Update the reasoning-effort emission in the model export
flow around ompAcceptsEffort so each emitted effort is normalized before
populating efforts, and derive defaultLevel from that normalized list. Only emit
defaultLevel when the normalized model.defaultReasoningEffort is present in
efforts, preserving the advertised-subset clamp.
In `@tests/management-client-config-route.test.ts`:
- Around line 186-198: Strengthen the test around modelRows and
buildClientConfig by selecting a native model fixture with an OMP-supported
defaultReasoningEffort, then assert exported.thinking?.defaultLevel equals that
expected value alongside the existing reasoning assertions. Keep the regression
test focused on preserving the mapper’s default reasoning level.
🪄 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: 3149dff6-ccfa-4880-a8a9-1056116322e9
📒 Files selected for processing (18)
docs-site/src/content/docs/guides/oh-my-pi.mdgui/src/app-routing.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/tests/client-config-panel.test.tsxgui/tests/integrations-routing.test.tssrc/adapters/command-code.tssrc/cli/export-command.tssrc/clients/config-export.tssrc/server/management/model-rows.tstests/cli-export-command.test.tstests/client-config-export-new-clients.test.tstests/command-code-provider.test.tstests/management-client-config-route.test.ts
| `api: openai-completions` selects omp's OpenAI-compatible adapter, which talks | ||
| to the proxy's `/v1` surface. `$OPENCODEX_OMP_API_KEY` is only omp's local | ||
| provider key for the `opencodex` entry — it is not an opencodex OAuth or | ||
| access-token credential, and opencodex never sees its value. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'openai-chat|openai-completions|Chat Completions' \
src/clients/config-export.ts \
docs-site/src/content/docs/reference/adapters.md \
docs-site/src/content/docs/guides/providers.md \
docs-site/src/content/docs/guides/oh-my-pi.mdRepository: lidge-jun/opencodex
Length of output: 15601
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- adapter reference relevant section ---'
sed -n '1,80p' docs-site/src/content/docs/reference/adapters.md | cat -n
printf '%s\n' '--- config-export OMP dialect/use sites ---'
sed -n '130,190p' src/clients/config-export.ts | cat -n
sed -n '900,995p' src/clients/config-export.ts | cat -n
printf '%s\n' '--- semantic check for wire format mentions in OMP provider docs ---'
python3 - <<'PY'
from pathlib import Path
paths = [
Path("src/clients/config-export.ts"),
Path("docs-site/src/content/docs/reference/adapters.md"),
Path("docs-site/src/content/docs/guides/providers.md"),
Path("docs-site/src/content/docs/guides/oh-my-pi.md"),
]
for p in paths:
text = p.read_text()
print(p, {
"api:openai-completions": 'api: "openai-completions"' in text or "api: openai-completions" in text,
"openai-chat": text.count("`openai-chat`"),
"Chat Completions": text.count("Chat Completions"),
"COMPLETIONS": text.count("/chat/completions"),
})
PYRepository: lidge-jun/opencodex
Length of output: 13597
Use the documented OpenAI Chat Completions terminology.
In docs-site/src/content/docs/guides/oh-my-pi.md:39-40, replace the vague “OpenAI-compatible adapter” description with the documented wire format. OMP’s api: openai-completions selects the proxy wire dialect, while openai-chat is the Chat Completions adapter used by the generated OpenCodex provider; identify the generated provider as targeting OpenAI Chat Completions through the openai-chat-compatible wire format.
Proposed wording
-`api: openai-completions` selects omp's OpenAI-compatible adapter, which talks
-to the proxy's `/v1` surface.
+`api: openai-completions` configures omp to target OpenAI Chat Completions
+through the `openai-chat`-compatible wire format at the proxy's `/v1` surface.🤖 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/oh-my-pi.md` around lines 39 - 42, Update
the documentation around api: openai-completions to use the documented OpenAI
Chat Completions terminology: describe it as selecting OMP’s proxy wire dialect,
and identify the generated OpenCodex provider as using the openai-chat Chat
Completions-compatible wire format. Preserve the existing clarification about
OPENCODEX_OMP_API_KEY.
Source: Path instructions
| // omp and other OpenAI-compatible clients dial with the namespaced selector | ||
| // (command-code/deepseek-deepseek-v4-flash, inner "/" encoded as "-"), so | ||
| // decode it back against the official effort table before the lookup. | ||
| const aliased = COMMAND_CODE_MODEL_ALIASES[modelId] ?? modelId; | ||
| const knownIds = [...new Set([ | ||
| ...Object.keys(COMMAND_CODE_MODEL_REASONING_EFFORTS), | ||
| ...Object.values(COMMAND_CODE_MODEL_ALIASES), | ||
| ])]; | ||
| // Strip the provider prefix first: decodeRoutedModelId expects the encoded | ||
| // id portion (deepseek-deepseek-v4-flash), not the full command-code/ slug. | ||
| const encodedId = aliased.startsWith("command-code/") ? aliased.slice("command-code/".length) : aliased; | ||
| const canonicalId = decodeRoutedModelId(encodedId, knownIds); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Send the canonical model ID in the Command Code request.
Lines 325-333 decode the OMP selector only for the reasoning-effort lookup. Line 362 still writes command-code/deepseek-deepseek-v4-flash to params.model.
OMP exports this namespaced selector. Command Code requires the canonical model ID, deepseek/deepseek-v4-flash. The request can therefore carry a valid reasoning_effort but fail model resolution upstream.
Extract this decoding into a shared resolver. Use its result in both supportedCommandCodeEffort and params.model. Extend tests/command-code-provider.test.ts to assert the serialized params.model value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/command-code.ts` around lines 322 - 333, Extract the model-ID
decoding logic around `decodeRoutedModelId` into a shared resolver that returns
the canonical model ID, then use that result for both
`supportedCommandCodeEffort` and the `params.model` assignment in the Command
Code request. Ensure namespaced OMP selectors resolve to canonical IDs such as
`deepseek/deepseek-v4-flash`, and add coverage in
`tests/command-code-provider.test.ts` asserting the serialized `params.model`
value.
| // omp only exposes reasoning-effort controls when the model declares a | ||
| // thinking block; without it omp sends no reasoning_effort at all. | ||
| const efforts = model.reasoningEfforts?.filter(effort => ompAcceptsEffort(effort)) ?? []; | ||
| if (efforts.length > 0) { | ||
| entry.reasoning = true; | ||
| entry.thinking = { | ||
| mode: "effort", | ||
| efforts, | ||
| ...(model.defaultReasoningEffort && ompAcceptsEffort(model.defaultReasoningEffort) | ||
| ? { defaultLevel: model.defaultReasoningEffort } | ||
| : {}), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep defaultLevel within normalized emitted efforts.
Line 852 validates a trimmed lowercase value but emits the original value. For example, " HIGH " passes validation and is written as " HIGH ", which is outside the OMP schema vocabulary. Line 858 can also emit "max" as defaultLevel when the model advertises only ["high"].
Normalize each effort before emission. Emit defaultLevel only when the normalized value exists in efforts.
Proposed fix
- const efforts = model.reasoningEfforts?.filter(effort => ompAcceptsEffort(effort)) ?? [];
+ const efforts = [...new Set(model.reasoningEfforts?.flatMap(effort => {
+ const normalized = effort.trim().toLowerCase();
+ return ompAcceptsEffort(normalized) ? [normalized] : [];
+ }) ?? [])];
+ const defaultLevel = model.defaultReasoningEffort?.trim().toLowerCase();
if (efforts.length > 0) {
entry.reasoning = true;
entry.thinking = {
mode: "effort",
efforts,
- ...(model.defaultReasoningEffort && ompAcceptsEffort(model.defaultReasoningEffort)
- ? { defaultLevel: model.defaultReasoningEffort }
+ ...(defaultLevel && efforts.includes(defaultLevel)
+ ? { defaultLevel }
: {}),
};
}As per path instructions, “Reasoning effort is clamped to the model’s advertised subset.”
📝 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.
| // omp only exposes reasoning-effort controls when the model declares a | |
| // thinking block; without it omp sends no reasoning_effort at all. | |
| const efforts = model.reasoningEfforts?.filter(effort => ompAcceptsEffort(effort)) ?? []; | |
| if (efforts.length > 0) { | |
| entry.reasoning = true; | |
| entry.thinking = { | |
| mode: "effort", | |
| efforts, | |
| ...(model.defaultReasoningEffort && ompAcceptsEffort(model.defaultReasoningEffort) | |
| ? { defaultLevel: model.defaultReasoningEffort } | |
| : {}), | |
| // omp only exposes reasoning-effort controls when the model declares a | |
| // thinking block; without it omp sends no reasoning_effort at all. | |
| const efforts = [...new Set(model.reasoningEfforts?.flatMap(effort => { | |
| const normalized = effort.trim().toLowerCase(); | |
| return ompAcceptsEffort(normalized) ? [normalized] : []; | |
| }) ?? [])]; | |
| const defaultLevel = model.defaultReasoningEffort?.trim().toLowerCase(); | |
| if (efforts.length > 0) { | |
| entry.reasoning = true; | |
| entry.thinking = { | |
| mode: "effort", | |
| efforts, | |
| ...(defaultLevel && efforts.includes(defaultLevel) | |
| ? { defaultLevel } | |
| : {}), |
🤖 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/clients/config-export.ts` around lines 850 - 860, Update the
reasoning-effort emission in the model export flow around ompAcceptsEffort so
each emitted effort is normalized before populating efforts, and derive
defaultLevel from that normalized list. Only emit defaultLevel when the
normalized model.defaultReasoningEffort is present in efforts, preserving the
advertised-subset clamp.
Source: Path instructions
| test("native model rows carry reasoning metadata into omp exports", async () => { | ||
| const rows = await modelRows(baseConfig()); | ||
| const native = rows.find(row => row.native && (row.reasoningEfforts?.length ?? 0) > 0); | ||
| expect(native).toBeDefined(); | ||
| const doc = buildClientConfig("omp", { | ||
| baseUrl: "http://127.0.0.1:10100/v1", | ||
| models: [toExportModel(native!)], | ||
| config: baseConfig(), | ||
| }) as OmpGeneratedConfig; | ||
| const exported = doc.providers[OPENCODE_PROVIDER_ID]!.models[0]!; | ||
| expect(exported.reasoning).toBe(true); | ||
| expect(exported.thinking?.efforts.length).toBeGreaterThan(0); | ||
| }, 15_000); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Assert the default reasoning level.
The mapper now copies defaultReasoningEffort, but this test selects a row only by reasoningEfforts and checks only reasoning and thinking.efforts. A regression that drops thinking.defaultLevel will still pass. Select a fixture with an OMP-supported default and assert that exported.thinking?.defaultLevel equals the expected value.
As per path instructions, a behavior change in src/ should come with a focused regression test near the existing tests for that subsystem.
🤖 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 186 - 198,
Strengthen the test around modelRows and buildClientConfig by selecting a native
model fixture with an OMP-supported defaultReasoningEffort, then assert
exported.thinking?.defaultLevel equals that expected value alongside the
existing reasoning assertions. Keep the regression test focused on preserving
the mapper’s default reasoning level.
Source: Path instructions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4537997dde
ℹ️ 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".
| omp: { | ||
| id: "omp", | ||
| configPath: (env = process.env, home = homedir()) => ompConfigPath(env, home), | ||
| detectDir: (env = process.env, home = homedir()) => ompHomeDir(env, home), |
There was a problem hiding this comment.
When PI_CODING_AGENT_DIR points to an existing agent directory but the default/configured omp root does not exist, configPath correctly resolves into the override while detectDir still checks ompHomeDir. Consequently, readIntegrationState reports installed: false, the dashboard locks the switch, and applyIntegration returns not_installed even though the explicitly selected omp directory exists; make detection follow ompAgentDir when this override is active.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
1 participant
Summary
Adds Oh My Pi (omp) as a first-class client integration. omp 17.x reads custom providers from
~/.omp/agent/models.yml, so the existingpiintegration (which targets the legacy~/.pi/agent/models.json) cannot reach it.The new
ompclient:opencodexprovider block into~/.omp/agent/models.yml(YAML,api: openai-completions,$OPENCODEX_OMP_API_KEYenv reference — never a serialized key).PI_CONFIG_DIR,PI_CODING_AGENT_DIR, andOMP_PROFILE/PI_PROFILE(profile wins over the agent-dir override, matching omp'sDirResolver).ocx export --client omp, the management API, and the CLI help.x-opencodex-api-keyheader a non-loopback bind requires, so a remote bind is refused rather than handed a config that 401s.The generated document was validated against omp 17.2.10's own
ModelsConfigSchemaandvalidateProviderConfiguration.Verification
bun run typecheck— cleanbun run test— 9460 pass, 0 failbun run lint:gui— cleanbun run privacy:scan— passedbun run build:gui— buildsPI_CONFIG_DIR/PI_CODING_AGENT_DIR/ profiles), and the full integration lifecycle (apply/disable round trip)models.ymlvalidated against the installed@oh-my-pi/pi-coding-agent@17.2.10schemaScreenshot
Oh My Pi tab in the Integrations dashboard:
Checklist
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
ocx export --client ompto generate provider configuration, including credentials and model reasoning settings.Documentation