feat: 支持通过 APP_BASE_PATH 配置子路径部署 - #4433
Conversation
refactor(router): 重构路由设置以支持子路径 feat(web): 添加 base-path 工具函数处理子路径逻辑 feat(common): 实现基础路径相关工具函数 test: 添加基础路径相关测试用例 docs: 更新文档说明子路径配置方法
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds configurable APP_BASE_PATH support across backend and frontend: normalization utilities, env/init wiring, request-stripping middleware, router and static serving updates, HTML/runtime base-href injection, frontend helpers and routing changes, tests, and README documentation for sub-path hosting. Changes
Sequence DiagramsequenceDiagram
participant Browser as Browser/Client
participant Gin as Gin Router
participant Middleware as StripAppBasePath<br/>Middleware
participant Handler as Route Handler
participant FS as Embedded fs.FS
Browser->>Gin: GET /new-api/console/resource?q=1
Gin->>Middleware: Incoming request
Middleware->>Middleware: Read common.AppBasePath = "/new-api"
Middleware->>Middleware: Strip "/new-api" from Path and RequestURI
Middleware->>Gin: Store original_request_path & original_request_uri in context
Gin->>Handler: Forward request with stripped path "/console/resource?q=1"
Handler->>FS: Serve index/static or invoke API handler
Handler->>Browser: 200 HTML/asset or API response
alt HTML response
Handler->>Handler: injectAppBasePath -> replace placeholder and ensure <base href="/new-api/">
Handler->>Browser: HTML with runtime appBasePath and base href
Browser->>Browser: BrowserRouter basename="/new-api" resolves client routes
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/layout/SetupCheck.js (1)
29-36:⚠️ Potential issue | 🔴 CriticalFix infinite redirect loop when APP_BASE_PATH is configured.
location.pathnamebecomes{APP_BASE_PATH}/setup(e.g.,/new-api/setup) when a base path is configured, so the hardcoded'/setup'comparison always fails. This causesredirectToApp('/setup')to re-fire on every render of the setup page, producing an infinite redirect loop and making the setup page unusable in such configurations.Compare against the base-path-aware value using
withBasePath('/setup').🐛 Proposed fix
-import { redirectToApp } from '../../helpers'; +import { redirectToApp, withBasePath } from '../../helpers'; @@ - if ( - statusState?.status?.setup === false && - location.pathname !== '/setup' - ) { + if ( + statusState?.status?.setup === false && + location.pathname !== withBasePath('/setup') + ) { redirectToApp('/setup'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/SetupCheck.js` around lines 29 - 36, The redirect comparison in SetupCheck.js uses a hardcoded '/setup' which fails when APP_BASE_PATH is set; update the useEffect to compare location.pathname against the base-aware path by calling withBasePath('/setup') instead of '/setup' so the condition (statusState?.status?.setup === false && location.pathname !== withBasePath('/setup')) prevents the infinite redirect; ensure redirectToApp('/setup') continues to be used for navigation but the equality check uses withBasePath('/setup').
🧹 Nitpick comments (9)
common/base_path.go (1)
41-61:WithAppBasePathis idempotent — document this contract.Lines 57–59 treat a
routePaththat already equals or starts withAppBasePath+"/"as already-prefixed and return it unchanged. This is the right behavior for callers that sometimes pass server-side absolute paths (e.g., redirect targets from user input) but is a non-obvious contract.Suggest adding a brief docstring so callers don't rely on double-prefixing and reviewers don't flag it as a bug in future reads.
✍️ Suggested doc
+// WithAppBasePath returns routePath prefixed with AppBasePath. It is idempotent: +// if routePath already equals AppBasePath or starts with AppBasePath+"/", it is +// returned unchanged. An empty or "/" routePath yields AppBasePath (or "/" when +// AppBasePath is empty). func WithAppBasePath(routePath string) string {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/base_path.go` around lines 41 - 61, Add a brief docstring to WithAppBasePath explaining that the function is idempotent: when AppBasePath is already present on routePath (either equal to it or as a prefix + "/") the function returns routePath unchanged, and otherwise it prefixes routePath with AppBasePath (handling empty and "/" cases). Update the comment immediately above the WithAppBasePath function to state this contract and the expected behavior for inputs like "" and "/", referencing AppBasePath and the idempotent behavior so future callers/reviewers understand it.web/vite.config.js (1)
27-69: LGTM — validation and proxy mirroring are coherent.
normalizeBasePathmirrors the Gocommon.NormalizeBasePathvalidation rules (leading/, no query/fragment, no empty/./..segments), andcreateProxyEntriescorrectly proxies both the bare prefix and the base-path-prefixed form so dev works identically with or withoutAPP_BASE_PATH. Adding/v1and/v1betais a good catch to avoid CORS/404s on relay endpoints in dev.Nit (optional):
process.env.APP_BASE_PATHis read at Vite config load, so changing it requires a dev-server restart; worth calling out in the docs for this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/vite.config.js` around lines 27 - 69, The Vite config reads process.env.APP_BASE_PATH at load via normalizeBasePath() and stores it in appBasePath, so changes to APP_BASE_PATH require restarting the dev server; update the PR/docs to state this and, in the Vite config near the appBasePath initialization (or where createProxyEntries is used), optionally add a startup console/info log that includes the resolved appBasePath to make this behavior obvious to developers.common/base_path_test.go (1)
5-69: Consider coveringStripAppBasePathand a few moreWithAppBasePathedges.Good table-driven coverage for
NormalizeBasePath. Two small gaps worth filling:
- No direct unit tests for
StripAppBasePath— the interesting branches (AppBasePath=""with empty request path, non-matching prefix returning("", false), partial-match guard like/apixwhenAppBasePath=/api) are only covered indirectly via the middleware test. A pure unit test here would lock in the contract.TestWithAppBasePathdoesn't cover the partial-match guard (e.g.,AppBasePath="/api",path="/apix"should return/api/apix, not/apix), which is arguably the subtlest branch.♻️ Suggested additions
+func TestStripAppBasePath(t *testing.T) { + original := AppBasePath + t.Cleanup(func() { AppBasePath = original }) + + AppBasePath = "/new-api" + cases := []struct { + name string + in string + want string + wantOk bool + }{ + {"exact", "/new-api", "/", true}, + {"subpath", "/new-api/console", "/console", true}, + {"no prefix", "/other", "", false}, + {"partial prefix", "/new-apix", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, ok := StripAppBasePath(tc.in) + if got != tc.want || ok != tc.wantOk { + t.Fatalf("StripAppBasePath(%q) = (%q,%v), want (%q,%v)", tc.in, got, ok, tc.want, tc.wantOk) + } + }) + } +}And in
TestWithAppBasePath:{name: "empty", path: "", want: "/new-api"}, + {name: "partial prefix not double-prefixed", path: "/new-apix", want: "/new-api/new-apix"},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/base_path_test.go` around lines 5 - 69, Add unit tests for StripAppBasePath and the partial-match behavior of WithAppBasePath: create table-driven cases calling StripAppBasePath with AppBasePath="" and request path "" (expect "", true), a non-matching prefix (expect "", false), and a partial-match guard like AppBasePath="/api" with request "/apix" (expect "", false); also extend TestWithAppBasePath to include AppBasePath="/api" and path="/apix" asserting the result is "/api/apix" (i.e., it should prepend the base not strip it). Reference the functions StripAppBasePath, WithAppBasePath and the global AppBasePath when adding these tests in common/base_path_test.go.common/embed-file-system.go (1)
34-42: Remove unused exportedEmbedFolderfunction.The generalization from
embed.FStofs.FSis safe—embed.FSsatisfiesfs.FS, so prior callers remain compatible. However,EmbedFolderhas no remaining call sites in the codebase;router/web-router.gonow callsfs.Subdirectly. Since this exported function is unused, remove it to avoid dead exported API surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/embed-file-system.go` around lines 34 - 42, Remove the unused exported function EmbedFolder: delete the entire EmbedFolder(fs.FS, targetPath string) static.ServeFileSystem function declaration and its body (which constructs an embedFileSystem by calling fs.Sub and http.FS) so the dead exported API is removed; ensure no references remain to EmbedFolder (router/web-router.go already calls fs.Sub directly) and keep the embedFileSystem type and other helpers intact.web/src/helpers/base-path.test.js (1)
36-41: Optional: add a fragment (#) rejection case.The Go
NormalizeBasePathrejects both?and#, but the JS test only covers?. Consider adding/app#fragto keep parity with the backend contract and guard against regressions.✏️ Suggested addition
- test.each(['app', '/a//b', '/a/./b', '/a/../b', '/app?x=1'])( + test.each(['app', '/a//b', '/a/./b', '/a/../b', '/app?x=1', '/app#frag'])( 'rejects invalid base path %p', (input) => { expect(() => normalizeBasePath(input)).toThrow(); }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/base-path.test.js` around lines 36 - 41, Update the test case for normalizeBasePath to also assert that fragment characters are rejected: in the test block using test.each([...]) for invalid base paths (the array currently containing '?'-case '/app?x=1'), add '/app#frag' so the test ensures normalizeBasePath throws on inputs containing '#' as well, matching the Go NormalizeBasePath behavior.web/src/helpers/base-path.js (1)
89-99: Minor:getAppOriginandgetApiBaseUrlsemantics are a little surprising.
getAppOrigin()returns${origin}${APP_BASE_PATH}with no trailing slash (e.g.,https://example.com/new-api). Callers that naively concatenate (e.g.,getAppOrigin() + 'foo') will produce/new-apifoo. Either always append/here, or document the contract in a JSDoc.getApiBaseUrl()returns the path stringAPP_BASE_PATH(e.g.,/new-api) as a fallback labeled "API base URL". It works for relative fetches on the same origin, but the name suggests a URL. A short JSDoc clarifying the contract would help future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/base-path.js` around lines 89 - 99, getAppOrigin currently returns `${origin}${APP_BASE_PATH}` which can omit a trailing slash and cause wrong string concatenation; update getAppOrigin to ensure it always returns an origin with a single trailing slash before the base path (e.g., normalize window.location.origin and APP_BASE_PATH so the result ends with '/' when appropriate) and avoid introducing duplicate slashes. Also add concise JSDoc comments for both getAppOrigin and getApiBaseUrl that state their exact return contracts: getAppOrigin returns the origin combined with APP_BASE_PATH (normalized with a trailing slash for safe concatenation), and getApiBaseUrl returns either the full server URL from import.meta.env.VITE_REACT_APP_SERVER_URL or the APP_BASE_PATH fallback (a relative path), so callers know whether they get a full URL or a path.router/main_test.go (1)
107-121: Guard against futuret.Parallel()adoption on these tests.
newTestEngineWithFrontendBaseURLmutates the package-levelcommon.AppBasePathglobal (with save/restore viat.Cleanup). That's correct for sequential runs, but if anyone later addst.Parallel()to any of these tests, the goroutines will race onAppBasePathand produce flaky failures. Adding a short comment here, or using a sync wrapper / explicit acknowledgement that these tests must not be parallelized, would help avoid that footgun.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/main_test.go` around lines 107 - 121, The helper newTestEngineWithFrontendBaseURL mutates the package-global common.AppBasePath and can race if tests are run in parallel; fix it by adding a package-level sync.Mutex (e.g., var appBasePathMu sync.Mutex) and acquire appBasePathMu.Lock() at the start of newTestEngineWithFrontendBaseURL, then restore common.AppBasePath inside t.Cleanup and call appBasePathMu.Unlock() there (use defer inside the function or unlock in the cleanup to ensure the lock is released), import sync; keep t.Setenv and other setup as-is so the global change is serialized and safe for future t.Parallel() usage.main.go (2)
216-229:<base>injection fallback is brittle to template changes.
injectAppBaseHrefonly matches an exact list of candidate strings (<base href="./" />,<base href="/" />,<base href="%BASE_URL%" />, etc.) with specific whitespace and self-closing style. If Vite ever emits<base href="/">(no trailing space, not self-closed) or adds attributes, none of the candidates will match and the fallback kicks in. The fallback then doesbytes.Replace(page, []byte("<head>"), ...)— which also silently no-ops if the build output ever produces<head lang="en">or<HEAD>.Consider using a regex or tokenizer to match any existing
<base ... />tag shape, and logging (or erroring) when neither the candidate nor the<head>replacement actually changes the page, so silent misconfiguration surfaces during startup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.go` around lines 216 - 229, The injectAppBaseHref function currently only matches exact candidate strings and a literal "<head>" which is brittle; replace the exact-string checks in injectAppBaseHref with a case-insensitive regex that matches any existing <base ...> tag (e.g. `<base\b[^>]*>` allowing optional attributes and optional self-closing slash) and replace that match with the constructed baseTag, and for the head insertion use a regex that matches the opening <head\b[^>]*> to preserve attributes; after attempting replacement, detect if the output is unchanged and emit a clear startup log/error (using existing logging) so misconfigured templates fail loudly instead of silently no-op.
207-214: Verifystrconv.Quoteoutput is safe inside the JS runtime object literal.
strconv.Quote(appBasePath)produces a Go string literal, which is a superset of a JSON string in some edge cases (e.g., Go emits\xNNfor non-printable bytes, which is not valid JSON/JS). This is currently safe becauseNormalizeBasePathrejects anything outside plain/a/bstyle ASCII paths. If the normalization rules are ever loosened (e.g., to allow Unicode or percent-encoded bytes), the injected snippet could emit invalid JS.Given the repo-wide guideline to route serialization through
common/json.gowrappers, usingcommon.Marshal(appBasePath)here (taking the resulting[]byteas the replacement) would be more future-proof and consistent.#!/bin/bash # Confirm common.Marshal exists and is the canonical wrapper for JSON strings. rg -nP '\bfunc\s+Marshal\s*\(' --type=go -g 'common/**'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.go` around lines 207 - 214, The replacement uses strconv.Quote(appBasePath) which emits Go string literal escapes that can be invalid in JS; change injectAppBasePath to serialize appBasePath with the repo's JSON wrapper by calling common.Marshal(appBasePath) and using the resulting []byte as the replacement (instead of strconv.Quote), keep the surrounding call to injectAppBaseHref(appBaseHref(appBasePath)), and update imports to remove strconv if unused and add the common package import so the code compiles; reference injectAppBasePath, strconv.Quote, common.Marshal, injectAppBaseHref, and appBaseHref when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Line 53: The healthcheck command assumes APP_BASE_PATH has no trailing slash
and will break if it does; update the docker-compose.yml healthcheck to
normalize APP_BASE_PATH in-shell (e.g., strip a trailing slash via POSIX
parameter expansion like using "${APP_BASE_PATH%/}" in the wget URL) or
alternatively add a clear comment next to APP_BASE_PATH explaining it must not
end with a slash; target the healthcheck test line and the APP_BASE_PATH
documentation/comment so the URL becomes
http://localhost:3000${NORMALIZED_APP_BASE_PATH}/api/status and always matches
the registered Gin route.
In `@router/web-router.go`:
- Around line 73-92: isAPINotFoundPath currently treats the bare "/assets" path
as an API-not-found; update the logic in isAPINotFoundPath so that only
"/assets/" (and its children) are considered API paths while the exact "/assets"
path is not; e.g., remove the exact "/assets" entry from apiPrefixes and ensure
the loop only matches "/assets/" via strings.HasPrefix(requestPath, "/assets/")
(keep other prefixes unchanged) so that GET /assets falls through to the SPA
instead of returning controller.RelayNotFound.
In `@web/src/helpers/auth.jsx`:
- Around line 44-57: PrivateRoute and AdminRoute pass state={{ from: location }}
but LoginForm.jsx doesn't consume it, so update LoginForm.jsx to import and use
useLocation() to read const from = location.state?.from?.pathname || '/' (or
'/console' for admin flow) and call navigate(from, { replace: true }) on
successful login; also ensure the login success path used by AdminRoute honors
its from fallback (e.g., '/console') and add replace: true to the Navigate calls
in PrivateRoute and AdminRoute so the protected URL doesn't remain in history.
In `@web/src/helpers/base-path.js`:
- Around line 47-56: The module-level APP_BASE_PATH currently calls
normalizeBasePath(runtimeBasePath) unconditionally which throws if the runtime
placeholder is invalid; change this so you validate or try/catch the runtime
value (window.__NEW_API_RUNTIME__.appBasePath accessed via runtimeBasePath)
before passing it to normalizeBasePath and treat any validation failure or
thrown error as "not provided" (fall back to
normalizeBasePath(import.meta.env?.BASE_URL || '/')), and log a non-fatal
warning mentioning APP_BASE_PATH and the bad runtime value instead of letting
the exception bubble and break the app.
- Around line 74-76: The hash-only branch that prepends APP_BASE_PATH when
url.startsWith('#') in the withBasePath/redirectToApp/openWithBasePath helpers
appears unused and should be removed or documented; either delete the
conditional that returns `${APP_BASE_PATH}${url}` to eliminate dead code from
withBasePath (and callers redirectToApp/openWithBasePath), or replace it with a
short comment above the url.startsWith('#') check explaining the intended
defensive behavior and when a hash-only URL should map to the app root so future
reviewers understand why it remains.
---
Outside diff comments:
In `@web/src/components/layout/SetupCheck.js`:
- Around line 29-36: The redirect comparison in SetupCheck.js uses a hardcoded
'/setup' which fails when APP_BASE_PATH is set; update the useEffect to compare
location.pathname against the base-aware path by calling withBasePath('/setup')
instead of '/setup' so the condition (statusState?.status?.setup === false &&
location.pathname !== withBasePath('/setup')) prevents the infinite redirect;
ensure redirectToApp('/setup') continues to be used for navigation but the
equality check uses withBasePath('/setup').
---
Nitpick comments:
In `@common/base_path_test.go`:
- Around line 5-69: Add unit tests for StripAppBasePath and the partial-match
behavior of WithAppBasePath: create table-driven cases calling StripAppBasePath
with AppBasePath="" and request path "" (expect "", true), a non-matching prefix
(expect "", false), and a partial-match guard like AppBasePath="/api" with
request "/apix" (expect "", false); also extend TestWithAppBasePath to include
AppBasePath="/api" and path="/apix" asserting the result is "/api/apix" (i.e.,
it should prepend the base not strip it). Reference the functions
StripAppBasePath, WithAppBasePath and the global AppBasePath when adding these
tests in common/base_path_test.go.
In `@common/base_path.go`:
- Around line 41-61: Add a brief docstring to WithAppBasePath explaining that
the function is idempotent: when AppBasePath is already present on routePath
(either equal to it or as a prefix + "/") the function returns routePath
unchanged, and otherwise it prefixes routePath with AppBasePath (handling empty
and "/" cases). Update the comment immediately above the WithAppBasePath
function to state this contract and the expected behavior for inputs like "" and
"/", referencing AppBasePath and the idempotent behavior so future
callers/reviewers understand it.
In `@common/embed-file-system.go`:
- Around line 34-42: Remove the unused exported function EmbedFolder: delete the
entire EmbedFolder(fs.FS, targetPath string) static.ServeFileSystem function
declaration and its body (which constructs an embedFileSystem by calling fs.Sub
and http.FS) so the dead exported API is removed; ensure no references remain to
EmbedFolder (router/web-router.go already calls fs.Sub directly) and keep the
embedFileSystem type and other helpers intact.
In `@main.go`:
- Around line 216-229: The injectAppBaseHref function currently only matches
exact candidate strings and a literal "<head>" which is brittle; replace the
exact-string checks in injectAppBaseHref with a case-insensitive regex that
matches any existing <base ...> tag (e.g. `<base\b[^>]*>` allowing optional
attributes and optional self-closing slash) and replace that match with the
constructed baseTag, and for the head insertion use a regex that matches the
opening <head\b[^>]*> to preserve attributes; after attempting replacement,
detect if the output is unchanged and emit a clear startup log/error (using
existing logging) so misconfigured templates fail loudly instead of silently
no-op.
- Around line 207-214: The replacement uses strconv.Quote(appBasePath) which
emits Go string literal escapes that can be invalid in JS; change
injectAppBasePath to serialize appBasePath with the repo's JSON wrapper by
calling common.Marshal(appBasePath) and using the resulting []byte as the
replacement (instead of strconv.Quote), keep the surrounding call to
injectAppBaseHref(appBaseHref(appBasePath)), and update imports to remove
strconv if unused and add the common package import so the code compiles;
reference injectAppBasePath, strconv.Quote, common.Marshal, injectAppBaseHref,
and appBaseHref when making the change.
In `@router/main_test.go`:
- Around line 107-121: The helper newTestEngineWithFrontendBaseURL mutates the
package-global common.AppBasePath and can race if tests are run in parallel; fix
it by adding a package-level sync.Mutex (e.g., var appBasePathMu sync.Mutex) and
acquire appBasePathMu.Lock() at the start of newTestEngineWithFrontendBaseURL,
then restore common.AppBasePath inside t.Cleanup and call appBasePathMu.Unlock()
there (use defer inside the function or unlock in the cleanup to ensure the lock
is released), import sync; keep t.Setenv and other setup as-is so the global
change is serialized and safe for future t.Parallel() usage.
In `@web/src/helpers/base-path.js`:
- Around line 89-99: getAppOrigin currently returns `${origin}${APP_BASE_PATH}`
which can omit a trailing slash and cause wrong string concatenation; update
getAppOrigin to ensure it always returns an origin with a single trailing slash
before the base path (e.g., normalize window.location.origin and APP_BASE_PATH
so the result ends with '/' when appropriate) and avoid introducing duplicate
slashes. Also add concise JSDoc comments for both getAppOrigin and getApiBaseUrl
that state their exact return contracts: getAppOrigin returns the origin
combined with APP_BASE_PATH (normalized with a trailing slash for safe
concatenation), and getApiBaseUrl returns either the full server URL from
import.meta.env.VITE_REACT_APP_SERVER_URL or the APP_BASE_PATH fallback (a
relative path), so callers know whether they get a full URL or a path.
In `@web/src/helpers/base-path.test.js`:
- Around line 36-41: Update the test case for normalizeBasePath to also assert
that fragment characters are rejected: in the test block using test.each([...])
for invalid base paths (the array currently containing '?'-case '/app?x=1'), add
'/app#frag' so the test ensures normalizeBasePath throws on inputs containing
'#' as well, matching the Go NormalizeBasePath behavior.
In `@web/vite.config.js`:
- Around line 27-69: The Vite config reads process.env.APP_BASE_PATH at load via
normalizeBasePath() and stores it in appBasePath, so changes to APP_BASE_PATH
require restarting the dev server; update the PR/docs to state this and, in the
Vite config near the appBasePath initialization (or where createProxyEntries is
used), optionally add a startup console/info log that includes the resolved
appBasePath to make this behavior obvious to developers.
🪄 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: 7cbd45fd-389f-4351-898b-3ae4be207c05
📒 Files selected for processing (45)
README.mdREADME.zh_CN.mdcommon/base_path.gocommon/base_path_test.gocommon/embed-file-system.gocommon/init.gocommon/sys_log.gocommon/sys_log_test.gocontroller/telegram.godocker-compose.ymlmain.gomain_test.gomiddleware/base_path.gomiddleware/base_path_test.gorouter/api-router.gorouter/dashboard.gorouter/main.gorouter/main_test.gorouter/relay-router.gorouter/video-router.gorouter/web-router.goweb/index.htmlweb/package.jsonweb/src/components/auth/LoginForm.jsxweb/src/components/auth/RegisterForm.jsxweb/src/components/layout/SetupCheck.jsweb/src/components/playground/MessageContent.jsxweb/src/components/settings/personal/cards/AccountManagement.jsxweb/src/components/setup/SetupWizard.jsxweb/src/components/table/channels/modals/ModelTestModal.jsxweb/src/components/table/tokens/index.jsxweb/src/components/table/tokens/modals/CCSwitchModal.jsxweb/src/components/topup/index.jsxweb/src/helpers/api.jsweb/src/helpers/auth.jsxweb/src/helpers/base-path.jsweb/src/helpers/base-path.test.jsweb/src/helpers/index.jsweb/src/helpers/token.jsweb/src/helpers/utils.jsxweb/src/hooks/chat/useTokenKeys.jsweb/src/hooks/tokens/useTokensData.jsxweb/src/index.jsxweb/src/pages/Home/index.jsxweb/vite.config.js
| - new-api-network | ||
| healthcheck: | ||
| test: ["CMD-SHELL", "wget -q -O - http://localhost:3000/api/status | grep -o '\"success\":\\s*true' || exit 1"] | ||
| test: ["CMD-SHELL", "wget -q -O - \"http://localhost:3000$${APP_BASE_PATH}/api/status\" | grep -o '\"success\":\\s*true' || exit 1"] |
There was a problem hiding this comment.
Healthcheck URL construction is sensitive to APP_BASE_PATH formatting.
The expansion http://localhost:3000${APP_BASE_PATH}/api/status assumes APP_BASE_PATH has no trailing slash. If a user sets APP_BASE_PATH=/new-api/, the resulting URL becomes http://localhost:3000/new-api//api/status, which won't match the Gin routes (backend normalizes AppBasePath by trimming trailing slashes, so the registered route is /new-api/api/status). Consider documenting this constraint in the adjacent comment on line 37, or using a shell normalization in the healthcheck command.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` at line 53, The healthcheck command assumes APP_BASE_PATH
has no trailing slash and will break if it does; update the docker-compose.yml
healthcheck to normalize APP_BASE_PATH in-shell (e.g., strip a trailing slash
via POSIX parameter expansion like using "${APP_BASE_PATH%/}" in the wget URL)
or alternatively add a clear comment next to APP_BASE_PATH explaining it must
not end with a slash; target the healthcheck test line and the APP_BASE_PATH
documentation/comment so the URL becomes
http://localhost:3000${NORMALIZED_APP_BASE_PATH}/api/status and always matches
the registered Gin route.
| func isAPINotFoundPath(requestPath string) bool { | ||
| apiPrefixes := []string{ | ||
| "/api", | ||
| "/assets", | ||
| "/dashboard", | ||
| "/jimeng", | ||
| "/kling", | ||
| "/mj", | ||
| "/pg", | ||
| "/suno", | ||
| "/v1", | ||
| "/v1beta", | ||
| } | ||
| for _, prefix := range apiPrefixes { | ||
| if requestPath == prefix || strings.HasPrefix(requestPath, prefix+"/") { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
Minor: /assets exact path returns an API-style 404.
isAPINotFoundPath treats /assets and /assets/... as API-like paths and routes them to controller.RelayNotFound, which returns a JSON error payload. In practice /assets/*filepath catches everything under /assets/, so this only affects the edge case of a bare GET /assets (no trailing slash and no filename) — which is low-value either way. Noting in case the intent was to fall through to the SPA for such paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router/web-router.go` around lines 73 - 92, isAPINotFoundPath currently
treats the bare "/assets" path as an API-not-found; update the logic in
isAPINotFoundPath so that only "/assets/" (and its children) are considered API
paths while the exact "/assets" path is not; e.g., remove the exact "/assets"
entry from apiPrefixes and ensure the loop only matches "/assets/" via
strings.HasPrefix(requestPath, "/assets/") (keep other prefixes unchanged) so
that GET /assets falls through to the SPA instead of returning
controller.RelayNotFound.
| function PrivateRoute({ children }) { | ||
| const location = useLocation(); | ||
| if (!localStorage.getItem('user')) { | ||
| return <Navigate to='/login' state={{ from: history.location }} />; | ||
| return <Navigate to='/login' state={{ from: location }} />; | ||
| } | ||
| return children; | ||
| } | ||
|
|
||
| export function AdminRoute({ children }) { | ||
| const location = useLocation(); | ||
| const raw = localStorage.getItem('user'); | ||
| if (!raw) { | ||
| return <Navigate to='/login' state={{ from: history.location }} />; | ||
| return <Navigate to='/login' state={{ from: location }} />; | ||
| } |
There was a problem hiding this comment.
state.from is passed but never consumed after login.
PrivateRoute/AdminRoute now forward the current location via state={{ from: location }}, but web/src/components/auth/LoginForm.jsx does not import useLocation/read location.state?.from and unconditionally calls navigate('/') / navigate('/console') on success. The attached from state is effectively dead until the login component is updated to honor it.
Also consider adding replace to these redirects (as done in AuthRedirect above) so the protected URL doesn't accumulate in browser history.
💡 Suggested redirect update
- return <Navigate to='/login' state={{ from: location }} />;
+ return <Navigate to='/login' state={{ from: location }} replace />;📝 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.
| function PrivateRoute({ children }) { | |
| const location = useLocation(); | |
| if (!localStorage.getItem('user')) { | |
| return <Navigate to='/login' state={{ from: history.location }} />; | |
| return <Navigate to='/login' state={{ from: location }} />; | |
| } | |
| return children; | |
| } | |
| export function AdminRoute({ children }) { | |
| const location = useLocation(); | |
| const raw = localStorage.getItem('user'); | |
| if (!raw) { | |
| return <Navigate to='/login' state={{ from: history.location }} />; | |
| return <Navigate to='/login' state={{ from: location }} />; | |
| } | |
| function PrivateRoute({ children }) { | |
| const location = useLocation(); | |
| if (!localStorage.getItem('user')) { | |
| return <Navigate to='/login' state={{ from: location }} replace />; | |
| } | |
| return children; | |
| } | |
| export function AdminRoute({ children }) { | |
| const location = useLocation(); | |
| const raw = localStorage.getItem('user'); | |
| if (!raw) { | |
| return <Navigate to='/login' state={{ from: location }} replace />; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/helpers/auth.jsx` around lines 44 - 57, PrivateRoute and AdminRoute
pass state={{ from: location }} but LoginForm.jsx doesn't consume it, so update
LoginForm.jsx to import and use useLocation() to read const from =
location.state?.from?.pathname || '/' (or '/console' for admin flow) and call
navigate(from, { replace: true }) on successful login; also ensure the login
success path used by AdminRoute honors its from fallback (e.g., '/console') and
add replace: true to the Navigate calls in PrivateRoute and AdminRoute so the
protected URL doesn't remain in history.
| if (url.startsWith('#')) { | ||
| return `${APP_BASE_PATH}${url}`; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find call sites passing hash-only strings (or variables) to withBasePath / redirectToApp / openWithBasePath.
rg -nP --type=js --type=jsx --type=ts --type=tsx -C2 \
'\b(withBasePath|redirectToApp|openWithBasePath)\s*\(\s*[`'"'"'"]\s*#'Repository: QuantumNous/new-api
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Fix: use valid file types (js, ts, tsx - not jsx)
# Find call sites passing hash-only strings to withBasePath / redirectToApp / openWithBasePath
rg -nP --type=js --type=ts --type=tsx -C2 \
'\b(withBasePath|redirectToApp|openWithBasePath)\s*\(\s*[`'"'"'"]\s*#'Repository: QuantumNous/new-api
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Also search for the function implementations to understand their purpose
fd -t f -e js -e ts -e tsx | xargs rg -l "export.*\b(withBasePath|redirectToApp|openWithBasePath)" | head -5Repository: QuantumNous/new-api
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Read the base-path.js file to see the full implementation
wc -l web/src/helpers/base-path.jsRepository: QuantumNous/new-api
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Read the full base-path.js file to understand the implementation
cat -n web/src/helpers/base-path.jsRepository: QuantumNous/new-api
Length of output: 3837
🏁 Script executed:
#!/bin/bash
# Search for calls to withBasePath with hash patterns using simpler approach
rg -n '\b(withBasePath|redirectToApp|openWithBasePath)\s*\(\s*[`'"'"'"#]' web/src --type js --type tsRepository: QuantumNous/new-api
Length of output: 1531
🏁 Script executed:
#!/bin/bash
# Search more broadly for hash-only URLs being passed or used with these functions
rg -n "withBasePath\|redirectToApp\|openWithBasePath" web/src --type js | grep -i "#\|hash" | head -20Repository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for any hash-only string literals that might be passed to these functions
rg -n '[\(\[].*#[a-zA-Z]' web/src --type js | head -20Repository: QuantumNous/new-api
Length of output: 819
🏁 Script executed:
#!/bin/bash
# Check if there are any dynamic hash constructions (e.g., variables named hash, anchor, etc.)
rg -n "const.*=.*'#\|const.*=.*\"#" web/src --type js | head -20Repository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if hash-only URLs are used anywhere in the codebase (e.g., in href attributes, navigation, etc.)
rg -n "href\s*=\s*['\"]#" web/src --type js | head -20Repository: QuantumNous/new-api
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Also check for hash usage in general patterns across the web directory
rg -n "navigate\|push.*['\"]#\|location.*['\"]#" web/src --type js | head -20Repository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check git history or comments to understand the intent of this code
rg -n "hash\|anchor\|fragment" web/src/helpers/base-path.jsRepository: QuantumNous/new-api
Length of output: 45
Hash-only URL prepending behavior appears to be unused code — remove or document intent.
No call sites were found passing hash-only URLs to withBasePath(), redirectToApp(), or openWithBasePath(). The code at lines 74–76 prepends APP_BASE_PATH to hash-only strings, which would navigate to the app root with an anchor rather than staying on the current page.
Either remove this dead code if it's not needed, or add a comment explaining the intended use case if it's deliberate defensive programming for future functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/helpers/base-path.js` around lines 74 - 76, The hash-only branch that
prepends APP_BASE_PATH when url.startsWith('#') in the
withBasePath/redirectToApp/openWithBasePath helpers appears unused and should be
removed or documented; either delete the conditional that returns
`${APP_BASE_PATH}${url}` to eliminate dead code from withBasePath (and callers
redirectToApp/openWithBasePath), or replace it with a short comment above the
url.startsWith('#') check explaining the intended defensive behavior and when a
hash-only URL should map to the app root so future reviewers understand why it
remains.
新增 normalizeRuntimeBasePath 函数处理运行时基础路径，支持特殊占位符和错误处理。同时添加相关测试用例验证功能： - 忽略未解析的 HTML 占位符 - 处理无效路径时警告并回退 - 优先使用有效的运行时路径
There was a problem hiding this comment.
🧹 Nitpick comments (4)
web/src/helpers/base-path.js (2)
108-126: Minor:getAppOrigin/getAbsoluteAppUrlassume a browserwindow.These will throw under SSR/test contexts where
windowis undefined. Given the frontend is CSR-only this is currently fine, but if any caller is imported by the Bun test suite at module init, it will crash. Consider guarding or documenting the browser-only contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/base-path.js` around lines 108 - 126, getAppOrigin and getAbsoluteAppUrl assume a browser global and will throw in SSR/tests; update these functions to guard access to window (e.g., check typeof window !== 'undefined') and return a safe fallback (empty string or the APP_BASE_PATH alone) or throw a clear error when window is unavailable, ensuring withBasePath is still used for path resolution; locate and modify the getAppOrigin and getAbsoluteAppUrl functions to add this guard and consistent fallback/behavior.
73-75: Nit: build-timeBASE_URLcould still throw at module init.
normalizeRuntimeBasePathis safe, but the right-hand fallbacknormalizeBasePath(import.meta.env?.BASE_URL || '/')is not wrapped. If a future Vite config setsbaseto somethingnormalizeBasePathrejects (e.g., contains..or a trailing query), the app would fail to load with no warning. Low likelihood sinceBASE_URLis developer-controlled, but wrapping this call in the same safe-normalize helper would make init fully fail-soft.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/base-path.js` around lines 73 - 75, The APP_BASE_PATH initialization currently calls normalizeBasePath(import.meta.env?.BASE_URL || '/') directly and can throw at module init; change the fallback to use the same fail-safe normalization used by normalizeRuntimeBasePath (e.g., call normalizeRuntimeBasePath(import.meta.env?.BASE_URL) or a shared safe wrapper) so that any invalid build-time BASE_URL is caught and returns the safe default instead of letting normalizeBasePath throw; update the APP_BASE_PATH expression to prefer normalizeRuntimeBasePath(runtimeBasePath) || normalizeRuntimeBasePath(import.meta.env?.BASE_URL) (or the new safe wrapper) so module initialization is fail-soft.web/src/helpers/base-path.test.js (2)
75-89: Minor: restoreconsole.warneven on synchronous throw and consider asserting warn count is 1 across all runtime tests.The try/finally correctly restores
console.warn, but since the mock is installed beforeimportBasePathWithRuntimeis awaited, a synchronous throw from the dynamic import would still be caught by the surrounding test framework and the finally runs — so this is fine as-is. Just noting: the valid-path test at lines 91–95 does not install a warn spy; if a regression causes that path to warn, it will go to the real stderr without failing the test. Optional: install the spy in abeforeEach/afterEachand assertwarnings.length === 0on the valid case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/base-path.test.js` around lines 75 - 89, The test should reliably restore and assert console.warn usage across runtime base-path tests: wrap the console.warn spy setup/teardown in a shared beforeEach/afterEach (install a spy that pushes into warnings and restore original in afterEach) and update the "valid path" test (the one asserting APP_BASE_PATH for importBasePathWithRuntime('app') when runtime is valid) to assert warnings.length === 0; keep the existing "warns and falls back..." test asserting warnings.length === 1 and the warning text, and ensure the spy is installed before any calls to importBasePathWithRuntime so synchronous throws still trigger the finally/afterEach restore.
26-45: Consider refactoring to use a factory function for clearer test isolation.The current dynamic import pattern with query string cache-busting (
?runtime-test=${runtimeImportId}) works correctly with Bun's test runner, which treats each distinct query string as a separate module instance and re-executes the module's top-level code. However, a more explicit and maintainable pattern would be to export a factory function likecomputeAppBasePath()frombase-path.jsand call it per test, avoiding implicit reliance on ESM cache semantics. This makes test expectations clearer and the dependency on Bun's behavior more explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/base-path.test.js` around lines 26 - 45, The tests rely on re-importing base-path.js with a cache-busting query via importBasePathWithRuntime and runtimeImportId; refactor base-path.js to export an explicit factory function (e.g., computeAppBasePath) that reads globalThis.window.__NEW_API_RUNTIME__.appBasePath and returns the computed base path, then update tests to set globalThis.window as they already do and call computeAppBasePath() directly (removing the dynamic import and query-string logic in importBasePathWithRuntime), ensuring each test invokes the factory to get a fresh value and making behavior independent of ESM cache semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/helpers/base-path.js`:
- Around line 108-126: getAppOrigin and getAbsoluteAppUrl assume a browser
global and will throw in SSR/tests; update these functions to guard access to
window (e.g., check typeof window !== 'undefined') and return a safe fallback
(empty string or the APP_BASE_PATH alone) or throw a clear error when window is
unavailable, ensuring withBasePath is still used for path resolution; locate and
modify the getAppOrigin and getAbsoluteAppUrl functions to add this guard and
consistent fallback/behavior.
- Around line 73-75: The APP_BASE_PATH initialization currently calls
normalizeBasePath(import.meta.env?.BASE_URL || '/') directly and can throw at
module init; change the fallback to use the same fail-safe normalization used by
normalizeRuntimeBasePath (e.g., call
normalizeRuntimeBasePath(import.meta.env?.BASE_URL) or a shared safe wrapper) so
that any invalid build-time BASE_URL is caught and returns the safe default
instead of letting normalizeBasePath throw; update the APP_BASE_PATH expression
to prefer normalizeRuntimeBasePath(runtimeBasePath) ||
normalizeRuntimeBasePath(import.meta.env?.BASE_URL) (or the new safe wrapper) so
module initialization is fail-soft.
In `@web/src/helpers/base-path.test.js`:
- Around line 75-89: The test should reliably restore and assert console.warn
usage across runtime base-path tests: wrap the console.warn spy setup/teardown
in a shared beforeEach/afterEach (install a spy that pushes into warnings and
restore original in afterEach) and update the "valid path" test (the one
asserting APP_BASE_PATH for importBasePathWithRuntime('app') when runtime is
valid) to assert warnings.length === 0; keep the existing "warns and falls
back..." test asserting warnings.length === 1 and the warning text, and ensure
the spy is installed before any calls to importBasePathWithRuntime so
synchronous throws still trigger the finally/afterEach restore.
- Around line 26-45: The tests rely on re-importing base-path.js with a
cache-busting query via importBasePathWithRuntime and runtimeImportId; refactor
base-path.js to export an explicit factory function (e.g., computeAppBasePath)
that reads globalThis.window.__NEW_API_RUNTIME__.appBasePath and returns the
computed base path, then update tests to set globalThis.window as they already
do and call computeAppBasePath() directly (removing the dynamic import and
query-string logic in importBasePathWithRuntime), ensuring each test invokes the
factory to get a fresh value and making behavior independent of ESM cache
semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c9b180d-1a1c-401b-82b5-432cc57bcadf
📒 Files selected for processing (2)
web/src/helpers/base-path.jsweb/src/helpers/base-path.test.js
SetupCheck Base Path Review 验证与处理方案Summary
修复决策
|
在OllamaModelModal和useApiRequest组件中，为fetch和SSE请求添加withBasePath处理，确保API请求路径正确包含基础路径。这解决了在特定部署环境下API请求路径不正确的问题。
51fdfc5 to
2b6f1df
Compare
1 participant
refactor(router): 重构路由设置以支持子路径
feat(web): 添加 base-path 工具函数处理子路径逻辑
feat(common): 实现基础路径相关工具函数
test: 添加基础路径相关测试用例
docs: 更新文档说明子路径配置方法
Important
📝 变更描述 / Description
增加子路径访问，在环境变量中配置
APP_BASE_PATH=/new-api首页访问路径就变成
http://127.0.0.1/new-api🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
本地运行测试
构建前端
构建后端
docker-compose运行测试
构建docker镜像
docker build -t new-api:local .修改
docker-compose.ymlnew-api:localAPP_BASE_PATH=/new-api示例：
启动docker compose
查看日志
运行截图测试
Summary by CodeRabbit
New Features
Documentation