fix: support split web/api domains in usage-doc examples#982
Conversation
📝 WalkthroughWalkthrough将 UsageDocContent 修改为从 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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.
|
hi,建议同步更新 |
- Fix Windows Test-NetConnection still using resolvedOrigin instead of apiOrigin - Strip trailing slash from NEXT_PUBLIC_API_BASE_URL to prevent double-slash URLs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Document the new env variable introduced for split-domain deployments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/app/[locale]/usage-doc/page.tsx (1)
142-143:⚠️ Potential issue | 🟠 Major请将
NEXT_PUBLIC_API_BASE_URL规范化为纯 origin(而不是仅去尾斜杠)Line 143 现在的处理会保留路径段。若配置为
https://api.example.com/v1,后续示例会出现/v1/v1,且 Windows 网络检测命令也会拿到非法主机名。该问题在之前评审里提过,当前代码仍存在。建议修复
const resolvedOrigin = origin || t("ui.currentSiteAddress"); - // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 - const apiOrigin = (process.env.NEXT_PUBLIC_API_BASE_URL?.trim() ?? "").replace(/\/$/, "") || resolvedOrigin; + // 优先使用显式配置的 API 域名,兼容 Web/UI 与 API 分域部署 + const normalizeApiOrigin = (raw?: string) => { + const value = raw?.trim() ?? ""; + if (!value) return resolvedOrigin; + try { + return new URL(value).origin; + } catch { + return value.replace(/\/+$/, "") || resolvedOrigin; + } + }; + const apiOrigin = normalizeApiOrigin(process.env.NEXT_PUBLIC_API_BASE_URL);🤖 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, The current apiOrigin assignment leaves path segments if NEXT_PUBLIC_API_BASE_URL includes them; change the logic that computes apiOrigin (the const apiOrigin assignment) to parse NEXT_PUBLIC_API_BASE_URL and normalize it to a pure origin (scheme + host + optional port) before trimming and fallback to resolvedOrigin; e.g., detect if NEXT_PUBLIC_API_BASE_URL is non-empty, create a URL object or strip everything after the host to obtain only the origin, and then use that normalized origin in place of the raw trimmed value so examples and network checks no longer inherit path segments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/`[locale]/usage-doc/page.tsx:
- Around line 142-143: The current apiOrigin assignment leaves path segments if
NEXT_PUBLIC_API_BASE_URL includes them; change the logic that computes apiOrigin
(the const apiOrigin assignment) to parse NEXT_PUBLIC_API_BASE_URL and normalize
it to a pure origin (scheme + host + optional port) before trimming and fallback
to resolvedOrigin; e.g., detect if NEXT_PUBLIC_API_BASE_URL is non-empty, create
a URL object or strip everything after the host to obtain only the origin, and
then use that normalized origin in place of the raw trimmed value so examples
and network checks no longer inherit path segments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e105ace9-0bd4-48c6-9720-9f53827f583b
📒 Files selected for processing (2)
.env.examplesrc/app/[locale]/usage-doc/page.tsx
✅ Files skipped from review due to trivial changes (1)
- .env.example
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a31e9ad46e
ℹ️ 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() ?? "").replace(/\/$/, "") || resolvedOrigin; |
There was a problem hiding this comment.
Resolve API origin from runtime config, not build-time env
This value is read inside a "use client" component, so process.env.NEXT_PUBLIC_API_BASE_URL is baked into the JS bundle at build time instead of read from container runtime env. In the default deployment path (docker-compose.yaml uses prebuilt ghcr.io/ding113/claude-code-hub:latest with .env injected at runtime), setting NEXT_PUBLIC_API_BASE_URL in .env will not change apiOrigin, so split-domain users still see incorrect examples unless they rebuild the image with that value.
Useful? React with 👍 / 👎.
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 introduces
apiOrigin— computed from a newNEXT_PUBLIC_API_BASE_URLenv var with a fallback to the current site origin — and applies it across all generated client-configuration examples on theusage-docpage, fixing split-domain (Web UI vs. API) deployments.apiOriginderivation and trailing-slash sanitization are correct; all code-block examples are updated.resolvedOrigininstead ofapiOrigin. The translation strings at those points contain API endpoint URL guidance, so in a split-domain setup the adjacent instructional text would contradict the code blocks shown above them.Confidence Score: 4/5
Safe to merge after fixing three inline i18n substitutions that still inject the Web UI origin into API-URL instructional text.
One P1 finding: the PR's primary goal (correct API origin in docs for split-domain setups) is partially unmet because three
.replace()calls still passresolvedOrigininstead ofapiOrigin, making the instructional notes inconsistent with the code blocks they annotate. All other changes are correct.src/app/[locale]/usage-doc/page.tsxlines 1169, 1269, and 1762Important Files Changed
apiOriginfromNEXT_PUBLIC_API_BASE_URLand correctly updates all code-block examples, but misses three inline i18n template substitutions (lines 1169, 1269, 1762) that still inject the Web UI origin into API-URL instructional text.NEXT_PUBLIC_API_BASE_URLwith clear documentation for split-domain deployments; no issues.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[UsageDocContent renders] --> B{NEXT_PUBLIC_API_BASE_URL set at build time?} B -->|Yes| C[trim + strip trailing slash] B -->|No| D[result is empty string] C --> E{Non-empty after sanitize?} E -->|Yes| F[apiOrigin = env var value] E -->|No| D D --> G[apiOrigin = resolvedOrigin\nwindow.location.origin] F --> H[Code blocks use apiOrigin] G --> H H --> I[Correct for split-domain] H --> J[i18n text substitutions\nlines 1169 / 1269 / 1762\nstill use resolvedOrigin] J --> K[Mismatch in split-domain:\ntext shows Web UI domain\ncode shows API domain] style K fill:#f99,stroke:#c00Comments Outside Diff (2)
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
src/app/[locale]/usage-doc/page.tsx, line 1169 (link)resolvedOrigininstead ofapiOriginThree places in this file replace the i18n placeholder in translation strings with
resolvedOrigin(the Web UI origin), while the code blocks immediately above were correctly updated to useapiOrigin. In a split-domain deployment this creates a direct contradiction: the JSON config snippet shows the API domain, but the adjacent "Important" note tells the user to use the Web UI domain for the same setting.The same mismatch exists at line 1269 (Droid
importantPoints) and line 1762 (troubleshootingendpointConfigError.points) — all three translation strings contain API endpoint URL references such as"Anthropic format: Use ... (no /v1)"and"OpenAI format: Use .../v1".The fix for all three occurrences is to change the second argument of
.replace(...)fromresolvedOrigintoapiOrigin.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "Merge branch 'dev' into fix/usage-doc-sp..." | Re-trigger Greptile