fix(pi): keep loopback models visible - #1085
Conversation
|
✅ Deterministic PR hygiene checks passed. |
Review readiness checklistThis PR is kept in draft until every requirement below is fulfilled. The tickable checklist has been added to your PR description — tick all four boxes there.
✅ 4/4 boxes ticked. ✅ All four boxes are ticked. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPi and Kimi exports now use the literal ChangesLoopback credential handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ExportCommand
participant ConfigExport
participant ModelRows
participant Pi
User->>ExportCommand: request Pi configuration export
ExportCommand->>ConfigExport: build provider configuration
ConfigExport->>ModelRows: load exportable models
ModelRows-->>ConfigExport: return filtered model rows
ConfigExport->>ConfigExport: embed opencodex-loopback
ConfigExport-->>ExportCommand: omit API-key environment export
ExportCommand-->>Pi: write models.json configuration
Pi->>Pi: resolve apiKey for model visibility
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
382f424 to
ad837c9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@docs-site/src/content/docs/ja/reference/cli/agents.md`:
- Around line 152-160: Update the localized output summaries to describe
client-specific launch guidance rather than implying every client uses
environment exports: in docs-site/src/content/docs/ja/reference/cli/agents.md
lines 152-160, docs-site/src/content/docs/ru/reference/cli/agents.md lines
185-190, and docs-site/src/content/docs/zh-cn/reference/cli/agents.md lines
153-161, revise the surrounding summary using consistent wording that reflects
Pi’s prose “Before launching:” hint while preserving environment-variable
guidance for clients that support it.
In `@docs-site/src/content/docs/ru/reference/cli/agents.md`:
- Around line 185-190: Update the credential paragraph in the localized agents
documentation to distinguish the clients: state that opencode stores an
environment-variable reference, while Pi stores the literal `opencodex-loopback`
placeholder. Remove wording that implies the loopback reference is unused for
both clients, matching the distinction in the English source.
🪄 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: 3deb26aa-f662-4b71-8af7-922671d20dfa
⛔ Files ignored due to path filters (1)
devlog/_fin/260806_pi_loopback_models/evidence/pi-config-preview.pngis excluded by!**/*.png
📒 Files selected for processing (19)
docs-site/src/content/docs/guides/integrations.mddocs-site/src/content/docs/guides/pi.mddocs-site/src/content/docs/ja/guides/pi.mddocs-site/src/content/docs/ja/reference/cli/agents.mddocs-site/src/content/docs/ko/guides/pi.mddocs-site/src/content/docs/ko/reference/cli/agents.mddocs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/ru/guides/pi.mddocs-site/src/content/docs/ru/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/guides/pi.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mdgui/src/components/apikeys-workspace/ClientConfigDialog.tsxgui/tests/client-config-panel.test.tsxsrc/clients/config-export.tstests/cli-export-command.test.tstests/client-config-export-new-clients.test.tstests/client-config-export.test.tstests/client-config-new-clients.test.tstests/management-client-config-route.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs-site/src/content/docs/ja/reference/cli/agents.md (1)
154-154: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winScope the no-environment-variable claim to the generated Pi export.
The localized paragraphs say that Pi never uses environment variables, then describe Pi resolving
apiKeyenvironment references. State that the generated Pi export uses the literalopencodex-loopbackplaceholder and does not require an environment variable. Keep the separate behavior for existing Pi configurations that contain environment references.
docs-site/src/content/docs/ja/reference/cli/agents.md#L154-L154: revise the absolute Pi claim.docs-site/src/content/docs/ru/reference/cli/agents.md#L187-L190: revise the absolute Pi claim.docs-site/src/content/docs/zh-cn/reference/cli/agents.md#L155-L155: revise the absolute Pi claim.🤖 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, Revise the absolute Pi environment-variable claim in docs-site/src/content/docs/ja/reference/cli/agents.md lines 154-154, docs-site/src/content/docs/ru/reference/cli/agents.md lines 187-190, and docs-site/src/content/docs/zh-cn/reference/cli/agents.md lines 155-155: clarify that the generated Pi export uses the literal opencodex-loopback placeholder and requires no environment variable, while preserving the separate behavior for existing Pi configurations containing environment references.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.
Outside diff comments:
In `@docs-site/src/content/docs/ja/reference/cli/agents.md`:
- Line 154: Revise the absolute Pi environment-variable claim in
docs-site/src/content/docs/ja/reference/cli/agents.md lines 154-154,
docs-site/src/content/docs/ru/reference/cli/agents.md lines 187-190, and
docs-site/src/content/docs/zh-cn/reference/cli/agents.md lines 155-155: clarify
that the generated Pi export uses the literal opencodex-loopback placeholder and
requires no environment variable, while preserving the separate behavior for
existing Pi configurations containing environment references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 032d7207-3014-47bd-b0bc-39f1826bbc9b
📒 Files selected for processing (3)
docs-site/src/content/docs/ja/reference/cli/agents.mddocs-site/src/content/docs/ru/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.md
|
Addressed the remaining CodeRabbit outside-diff finding in 58216c4: all five locales now scope the no-env claim to the generated Pi export while preserving Pi’s behavior for existing unset env references. Docs build and privacy scan pass. |
![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: 58216c4f4b
ℹ️ 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".
|
Maintainer triage (code-level, against
Since this touches credential-destination behavior in |
58216c4 to
1397890
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/pi.md`:
- Around line 72-78: Update the Pi guide sections in
docs-site/src/content/docs/guides/pi.md:72-78,
docs-site/src/content/docs/ja/guides/pi.md:63-65,
docs-site/src/content/docs/ko/guides/pi.md:73-81,
docs-site/src/content/docs/ru/guides/pi.md:74-80, and
docs-site/src/content/docs/zh-cn/guides/pi.md:63-65 to consistently describe
opencodex-loopback as a non-secret literal apiKey placeholder in Pi’s normal
request format, not an admission credential or upstream provider key. State that
Pi normally calls /chat/completions with Bearer authentication, while the
loopback proxy ignores this value and requires no credential.
🪄 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: 7cbdc7e5-3404-41b8-9f97-4a4c927df2ea
⛔ Files ignored due to path filters (1)
devlog/_fin/260806_pi_loopback_models/evidence/pi-config-preview.pngis excluded by!**/*.png
📒 Files selected for processing (20)
docs-site/src/content/docs/guides/integrations.mddocs-site/src/content/docs/guides/pi.mddocs-site/src/content/docs/ja/guides/pi.mddocs-site/src/content/docs/ja/reference/cli/agents.mddocs-site/src/content/docs/ko/guides/pi.mddocs-site/src/content/docs/ko/reference/cli/agents.mddocs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/ru/guides/pi.mddocs-site/src/content/docs/ru/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/guides/pi.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mdgui/src/components/apikeys-workspace/ClientConfigDialog.tsxgui/tests/client-config-panel.test.tsxsrc/clients/config-export.tssrc/server/management/model-rows.tstests/cli-export-command.test.tstests/client-config-export-new-clients.test.tstests/client-config-export.test.tstests/client-config-new-clients.test.tstests/management-client-config-route.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ru/guides/pi.md`:
- Around line 74-75: В русском тексте вокруг `Bearer` уберите перенос строки
после дефиса, чтобы в отрендеренном предложении не появлялось `Bearer-
авторизации`; используйте формулировку `авторизации Bearer` или разместите
составную фразу на одной строке.
🪄 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: 706b0d8d-f410-45e9-ada9-64119bbef8e6
📒 Files selected for processing (5)
docs-site/src/content/docs/guides/pi.mddocs-site/src/content/docs/ja/guides/pi.mddocs-site/src/content/docs/ko/guides/pi.mddocs-site/src/content/docs/ru/guides/pi.mddocs-site/src/content/docs/zh-cn/guides/pi.md
|
Verdict from triage: READY pending a credential-destination security pass, since the change affects which loopback destinations models stay visible for without an env export. No code defects found; the branch is current against |
2 participants
Summary
opencodexprovider visible without requiring an unusedOPENCODEX_API_KEYenvironment variable on loopbackRoot cause
Pi resolves
apiKeywhile building its model list and hides the entire provider when the value is an unset environment-variable reference. The integration previously emitted$OPENCODEX_API_KEY, even though Pi is loopback-only and opencodex does not require admission authentication on loopback.The generated Pi block now carries the existing non-secret
opencodex-loopbackplaceholder shared by loopback-only clients. Non-loopback integration writes remain refused.Security review
This changes credential-destination behavior and needs explicit maintainer security review. The change does not serialize a user or provider secret: the literal placeholder is accepted only for the loopback-only Pi integration, where the data-plane admission key is not checked. Upstream provider credentials remain in opencodex's own configuration.
Verification
bun run typecheckbun run test— 9,015 pass, 8 skip, 0 failbun run privacy:scancd gui && bun test tests— 601 pass, 0 failcd gui && bun run lintcd gui && bun run lint:i18ncd gui && bun run buildcd docs-site && bun install --frozen-lockfile && bun run build— 216 pagespi --list-modelschanged from 0 to 25opencodexmodels with noOPENCODEX_API_KEY; an actualxai/grok-4.5request returnedPI_OKScreenshot
The Pi config preview shows the literal loopback placeholder and the routed model array; the empty environment-variable warning is no longer rendered.
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 fixed all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
opencodex-loopbackplaceholder without requiring an API-key environment variable.Bug Fixes
Documentation