fix: support split web/api domains in usage-doc examples#982
fix: support split web/api domains in usage-doc examples#982xuesongjun wants to merge 1 commit intoding113:devfrom
Conversation
📝 WalkthroughWalkthrough更新 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
| const t = useTranslations("usage"); | ||
| const resolvedOrigin = origin || t("ui.currentSiteAddress"); | ||
| // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 | ||
| const apiOrigin = process.env.NEXT_PUBLIC_API_BASE_URL?.trim() || resolvedOrigin; |
There was a problem hiding this comment.
Trailing slash not sanitized from env-var value
String.prototype.trim() only removes whitespace. If an operator sets NEXT_PUBLIC_API_BASE_URL=https://api.example.com/ (with a trailing slash), every URL that appends a path segment will produce a double slash — e.g. https://api.example.com//v1/messages.
Consider stripping a trailing slash as well:
| const apiOrigin = process.env.NEXT_PUBLIC_API_BASE_URL?.trim() || resolvedOrigin; | |
| const apiOrigin = (process.env.NEXT_PUBLIC_API_BASE_URL?.trim() ?? "").replace(/\/$/, "") || resolvedOrigin; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/usage-doc/page.tsx
Line: 143
Comment:
**Trailing slash not sanitized from env-var value**
`String.prototype.trim()` only removes whitespace. If an operator sets `NEXT_PUBLIC_API_BASE_URL=https://api.example.com/` (with a trailing slash), every URL that appends a path segment will produce a double slash — e.g. `https://api.example.com//v1/messages`.
Consider stripping a trailing slash as well:
```suggestion
const apiOrigin = (process.env.NEXT_PUBLIC_API_BASE_URL?.trim() ?? "").replace(/\/$/, "") || resolvedOrigin;
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Code Review
This pull request introduces apiOrigin to support split-domain deployments, allowing the API base URL to be configured independently of the web origin via the NEXT_PUBLIC_API_BASE_URL environment variable. Documentation snippets for Claude, Gemini, and various model providers have been updated to use this new variable. Feedback indicates that the migration is incomplete in several sections of the documentation and that the Windows network connectivity test should be updated for consistency with the Unix version.
| const t = useTranslations("usage"); | ||
| const resolvedOrigin = origin || t("ui.currentSiteAddress"); | ||
| // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 | ||
| const apiOrigin = process.env.NEXT_PUBLIC_API_BASE_URL?.trim() || resolvedOrigin; |
There was a problem hiding this comment.
While apiOrigin is correctly introduced to support split-domain deployments, its application is incomplete. Several documentation examples still use resolvedOrigin for API endpoints, which will generate incorrect URLs for users in split-domain deployments.
The following lines need to be updated to use apiOrigin instead of resolvedOrigin:
- Line 1169
- Line 1269
- Line 1762
|
|
||
| ${t("snippets.comments.testNetworkConnection")} | ||
| curl -I ${resolvedOrigin}`} | ||
| curl -i ${apiOrigin}/v1/messages`} |
There was a problem hiding this comment.
This update to the Unix network test is a great improvement. However, the corresponding test for Windows (on line 1510) was not updated. It still uses resolvedOrigin and a basic TCP check (Test-NetConnection).
For consistency and to provide an equivalent test, the Windows command should be updated to:
- Use
apiOrigin. - Use
Invoke-WebRequestto test the/v1/messagesendpoint, similar to thiscurlcommand.
Consider replacing the Test-NetConnection command with Invoke-WebRequest:
Invoke-WebRequest -Uri "${apiOrigin}/v1/messages" -UseBasicParsingThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0102dc0eb
ℹ️ 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".
| const t = useTranslations("usage"); | ||
| const resolvedOrigin = origin || t("ui.currentSiteAddress"); | ||
| // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 | ||
| const apiOrigin = process.env.NEXT_PUBLIC_API_BASE_URL?.trim() || resolvedOrigin; |
There was a problem hiding this comment.
Strip trailing slash from configured API origin
When NEXT_PUBLIC_API_BASE_URL is set with a trailing slash (a common format like https://api.example.com/), apiOrigin is used verbatim and later concatenated with suffixes such as /v1 and /v1beta, producing //v1 URLs in generated examples. Those double-slash paths can be treated differently by some gateways/SDKs (redirects or 404s), so users who copy these snippets may get connectivity failures even with a correct domain. Normalize trailing slashes before composing endpoint paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[locale]/usage-doc/page.tsx (1)
1504-1519:⚠️ Potential issue | 🟠 MajorWindows 的网络自检仍然在测 Web 域名。
Line 1510 继续从
resolvedOrigin取主机,而且端口写死为443。在 Web/UI 与 API 分域的部署里,Windows 用户会去探测前端域名而不是 API 域名;如果 API 用了非默认端口,这个示例也会给出错误结论。建议修复
const lang = os === "windows" ? "powershell" : "bash"; + const apiConnectionTarget = (() => { + try { + const url = new URL(apiOrigin); + return { + host: url.hostname, + port: url.port || (url.protocol === "http:" ? "80" : "443"), + }; + } catch { + return { + host: apiOrigin.replace(/^https?:\/\//, ""), + port: "443", + }; + } + })(); ... - Test-NetConnection -ComputerName ${resolvedOrigin.replace("https://", "").replace("http://", "")} -Port 443 + Test-NetConnection -ComputerName ${apiConnectionTarget.host} -Port ${apiConnectionTarget.port}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/usage-doc/page.tsx around lines 1504 - 1519, The Windows Test-NetConnection snippet incorrectly uses resolvedOrigin and hardcodes port 443; change it to derive host and port from apiOrigin instead. In the component rendering the PowerShell CodeBlock (referencing resolvedOrigin, apiOrigin, envKeyName, t), parse apiOrigin with the URL API (new URL(apiOrigin)) to get hostname and port, use the hostname for -ComputerName, and use the parsed port if present otherwise fall back to 443 for -Port, then interpolate those variables into the Test-NetConnection line in the CodeBlock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[locale]/usage-doc/page.tsx:
- Around line 142-143: Normalize NEXT_PUBLIC_API_BASE_URL into an origin before
assigning to apiOrigin: read the raw value
(process.env.NEXT_PUBLIC_API_BASE_URL), trim it, then try to construct new
URL(raw) and use url.origin; if URL construction throws, fall back to stripping
any trailing slashes from the raw value (raw.replace(/\/+$/,'') ) and use that
only if non-empty, otherwise use resolvedOrigin; update the assignment for
apiOrigin (the variable in this file) accordingly so values like
"https://api.example.com/", "https://api.example.com/v1" or malformed values
won't produce path fragments in downstream examples.
---
Outside diff comments:
In `@src/app/`[locale]/usage-doc/page.tsx:
- Around line 1504-1519: The Windows Test-NetConnection snippet incorrectly uses
resolvedOrigin and hardcodes port 443; change it to derive host and port from
apiOrigin instead. In the component rendering the PowerShell CodeBlock
(referencing resolvedOrigin, apiOrigin, envKeyName, t), parse apiOrigin with the
URL API (new URL(apiOrigin)) to get hostname and port, use the hostname for
-ComputerName, and use the parsed port if present otherwise fall back to 443 for
-Port, then interpolate those variables into the Test-NetConnection line in the
CodeBlock.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 407178e4-c13b-4743-a252-f7581195d257
📒 Files selected for processing (1)
src/app/[locale]/usage-doc/page.tsx
| // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 | ||
| const apiOrigin = process.env.NEXT_PUBLIC_API_BASE_URL?.trim() || resolvedOrigin; |
There was a problem hiding this comment.
先把 NEXT_PUBLIC_API_BASE_URL 归一化成 origin。
Line 143 现在只做了 trim()。如果部署把这个变量写成 https://api.example.com/ 或 https://api.example.com/v1,下面这些新示例会分别变成 //v1 或 /v1/v1,本 PR 刚改到的 Claude/Codex/Gemini/OpenCode/Droid 配置都会一起误导用户。
建议修复
const resolvedOrigin = origin || t("ui.currentSiteAddress");
- const apiOrigin = process.env.NEXT_PUBLIC_API_BASE_URL?.trim() || resolvedOrigin;
+ const normalizeApiOrigin = (value: string) => {
+ const trimmed = value.trim();
+ try {
+ return new URL(trimmed).origin;
+ } catch {
+ return trimmed.replace(/\/+$/, "");
+ }
+ };
+ const apiOrigin = normalizeApiOrigin(
+ process.env.NEXT_PUBLIC_API_BASE_URL || resolvedOrigin,
+ );📝 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.
| // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 | |
| const apiOrigin = process.env.NEXT_PUBLIC_API_BASE_URL?.trim() || resolvedOrigin; | |
| const resolvedOrigin = origin || t("ui.currentSiteAddress"); | |
| // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 | |
| const normalizeApiOrigin = (value: string) => { | |
| const trimmed = value.trim(); | |
| try { | |
| return new URL(trimmed).origin; | |
| } catch { | |
| return trimmed.replace(/\/+$/, ""); | |
| } | |
| }; | |
| const apiOrigin = normalizeApiOrigin( | |
| process.env.NEXT_PUBLIC_API_BASE_URL || resolvedOrigin, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/usage-doc/page.tsx around lines 142 - 143, Normalize
NEXT_PUBLIC_API_BASE_URL into an origin before assigning to apiOrigin: read the
raw value (process.env.NEXT_PUBLIC_API_BASE_URL), trim it, then try to construct
new URL(raw) and use url.origin; if URL construction throws, fall back to
stripping any trailing slashes from the raw value (raw.replace(/\/+$/,'') ) and
use that only if non-empty, otherwise use resolvedOrigin; update the assignment
for apiOrigin (the variable in this file) accordingly so values like
"https://api.example.com/", "https://api.example.com/v1" or malformed values
won't produce path fragments in downstream examples.
Summary
This PR fixes generated client configuration examples on the
usage-docpage for deployments where the Web UI domain and API domain are separated.Previously, the page used the current site origin (
window.location.origin) for all client examples. This works for single-domain deployments, but fails forsplit-domain setups such as:
https://example.comhttps://api.example.comIn that case, users open the docs page on the Web domain, but the generated CLI/SDK examples incorrectly show the Web domain instead of the actual API domain.
Changes
NEXT_PUBLIC_API_BASE_URLenvironment variable/v1/messagesinstead of the root URL, providing a more accurate connectivity test for API-only subdomains (returns401 Unauthorizedwhen reachable but unauthenticated, rather than misleading403from blocked root paths)Why
This improves documentation correctness for reverse-proxy and split-domain deployments (control plane / data plane separation) without breaking existing
single-domain setups.
If
NEXT_PUBLIC_API_BASE_URLis not configured, the previous behavior is preserved.Compatibility
NEXT_PUBLIC_API_BASE_URL=https://api.example.comGreptile Summary
This PR improves the
usage-docpage to correctly display API endpoints in generated CLI/SDK configuration examples when the Web UI and API are hosted on separate domains. It introduces a newapiOriginvariable that prefersNEXT_PUBLIC_API_BASE_URLand falls back to the current site's origin, preserving backward compatibility for single-domain deployments.ANTHROPIC_BASE_URL,GOOGLE_GEMINI_BASE_URL,base_url, etc.) across Claude Code, Codex, Gemini CLI, OpenCode, and Droid examples are correctly updated to useapiOrigin.curl) is updated to useapiOriginand now targets/v1/messagesfor a more meaningful reachability check.Test-NetConnectioncommand on line 1510 was not updated — it still referencesresolvedOrigin(the web domain) instead ofapiOrigin, producing an incorrect troubleshooting command for split-domain deployments.apiOriginvalue is not sanitized for trailing slashes; a value likehttps://api.example.com/would produce double-slash URLs (e.g.,https://api.example.com//v1).Confidence Score: 4/5
Safe to merge after fixing the missed
resolvedOriginreference in the Windows connectivity test.One P1 defect remains: the Windows
Test-NetConnectioncommand still usesresolvedOrigininstead ofapiOrigin, directly contradicting the stated goal of the PR and producing incorrect docs for split-domain users on Windows. The trailing-slash sanitization gap is P2. Both are straightforward one-line fixes.src/app/[locale]/usage-doc/page.tsx— specifically lines 1510 (Windows test) and 143 (apiOrigin derivation).Important Files Changed
apiOriginderived fromNEXT_PUBLIC_API_BASE_URLto fix split-domain doc examples; all API URL references updated except the WindowsTest-NetConnectioncommand, which still usesresolvedOrigin.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[UsageDocContent renders] --> B{NEXT_PUBLIC_API_BASE_URL set?} B -- yes --> C[apiOrigin = env var trimmed] B -- no --> D[apiOrigin = resolvedOrigin\n from window.location.origin] C --> E[Used in all API URL examples\nClaude Code / Codex / Gemini\nOpenCode / Droid] D --> E E --> F[Unix network test\ncurl -i apiOrigin/v1/messages ✅] E --> G[Windows network test\nTest-NetConnection resolvedOrigin ❌\nshould use apiOrigin]Comments Outside Diff (1)
src/app/[locale]/usage-doc/page.tsx, line 1510 (link)resolvedOrigininstead ofapiOriginThis line was not updated as part of the PR, so in a split-domain deployment (the exact scenario this PR targets), the Windows connectivity test will probe the web UI domain rather than the API domain. A user following the troubleshooting guide on Windows would be testing the wrong host.
The Unix equivalent on line 1519 was correctly updated to use
apiOrigin, so this is an inconsistency introduced by the PR.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: support split web/api domains in us..." | Re-trigger Greptile
(2/5) Greptile learns from your feedback when you react with thumbs up/down!