重构文档中心、账号分组与 API Key 权限管理#127
Merged
Merged
Conversation
…ts extensions - Add AccountGroup, AccountGroupsResponse, CreateAccountGroupRequest, UpdateAccountGroupRequest - Add tags/group_ids to AccountRow, allowed_group_ids to APIKeyRow - Extend UpdateAccountSchedulerRequest with proxy_url, tags, group_ids - Add cache_hit_rate to AccountUsageDetail - Add optional cache/latency fields to UsageStats (avg_first_token_ms, today_cache_rate, etc.)
Add editingId/editingName state, handleRenameKey handler calling
api.updateAPIKey(id, { name }), and inline Input+Check/X UI for
renaming API keys directly in the table row.
- Add listAccountGroups, createAccountGroup, updateAccountGroup, deleteAccountGroup - Add updateAPIKey for name and allowed_group_ids - Add no-store cache to admin request function
- Prefer Cascadia Mono/Code in --font-mono stack - Add code-panel, code-panel-header, code-panel-pre, code-inline classes - Add syntax token colors and shiki wrapper styles - Add card/button transition animations
Supports a text/meta line above the action buttons area, useful for showing context like timestamp or status info next to the header.
Supports free-text tag entry (Enter/comma to add), select-from-list mode with options prop, chips with X to remove, and overflow "+N" badge.
- Edit dialog with URL/label inputs and api.updateProxy call - Concurrent proxy testing (TEST_ALL_CONCURRENCY=4) with progress display showing done/total/failed counts - Error reporting via toast for all batch operations - Pencil edit button in each proxy table row
- Create UsageStatsSummary with Flow/Token/Cache/Health metric groups - Health uses first-token latency as primary metric - Token billing shows "Today: $X / Total: $X" format - Remove noisy "0% availability" subtext from available StatCard - Cache labels: today cache hit rate, today cached tokens, total cache hit rate
Minimal additions to api.ts required by frontend-pages features:
- updateAPIKey(id, { name?, allowed_group_ids? }) for API key rename
- updateProxy now accepts url parameter for proxy edit dialog
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughImplements multi-tenant account grouping and API-key group scoping end-to-end: DB schema and helpers, admin HTTP handlers, runtime/store enforcement and scheduler integration, API-key persistence/updates, frontend docs/highlighter, UI components/pages, migrations, and tests. ChangesDatabase and Runtime Store Account Groups and API Key Scoping
Runtime Store and Scheduler
Admin API: Handlers, Routing, Responses, and Tests
Frontend Types, API Client, Hooks, Components, and Pages
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
database/postgres.go (1)
3139-3148:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing active-account guard when updating
allowed_api_key_ids.This branch can update soft-deleted accounts because it selects by
idonly. It should use the same non-deleted predicate as the scheduler-update existence check.💡 Suggested fix
- selectQuery := `SELECT credentials FROM accounts WHERE id = $1` + selectQuery := `SELECT credentials FROM accounts WHERE id = $1 AND status <> 'deleted' AND COALESCE(error_message, '') <> 'deleted'` if db.isSQLite() { - selectQuery = `SELECT credentials FROM accounts WHERE id = ?` + selectQuery = `SELECT credentials FROM accounts WHERE id = ? AND status <> 'deleted' AND COALESCE(error_message, '') <> 'deleted'` } else { selectQuery += ` FOR UPDATE` }🤖 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 `@database/postgres.go` around lines 3139 - 3148, The SELECT that reads credentials (selectQuery) currently only filters by id and thus can lock and update soft-deleted accounts; change the query to include the same non-deleted predicate used in the scheduler-update existence check (e.g., add "AND <non-deleted-predicate>" such as "AND deleted_at IS NULL" or the project's equivalent) for both the SQLite and non-SQLite branches, preserving the FOR UPDATE for non-SQLite; ensure the parameter order passed to tx.QueryRowContext(ctx, selectQuery, id, ...) matches the added predicate parameters and update references to selectQuery, db.isSQLite(), tx.QueryRowContext and id accordingly.frontend/src/pages/Proxies.tsx (1)
179-189:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCount unsuccessful test responses as failures.
Both handlers only react to thrown requests. If the API returns HTTP 200 with
{ success: false }, the spinner clears but the row/test-all UI records no error andtestAllFailedstays wrong.Also applies to: 208-219
🤖 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 `@frontend/src/pages/Proxies.tsx` around lines 179 - 189, The API handler currently only treats thrown errors as failures; update the logic after calling api.testProxy(p.url, p.id, ipApiLang) to explicitly handle result.success === false as a failure: when result.success is false, call showToast with t('proxies.testFailed', { error: result.error || getErrorMessage(new Error('test failed')) }) (or similar), update setProxies so the row reflects the failed test (clear or set appropriate test_* fields), and ensure any test-all tracking state (e.g., testAllFailed/testAllProgress flags) is incremented/updated the same way as in the catch block; apply the same check-and-failure handling to the other handler around lines 208-219.admin/handler.go (1)
723-786:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake this scheduler PATCH atomic before touching runtime state.
Once
UpdateAccountSchedulerConfigand the earlyApplyAccount*calls succeed, a later failure inUpdateAccountTags,SetAccountGroups, orUpdateAccountProxyURLreturns 500 after part of the DB/runtime state has already changed. Wrap the DB writes in one transaction and only apply runtime mutations after every write succeeds.🤖 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 `@admin/handler.go` around lines 723 - 786, The DB updates (UpdateAccountSchedulerConfig, UpdateAccountTags, SetAccountGroups, UpdateAccountProxyURL) must be executed inside a single transaction and only after the transaction commits should you call runtime mutations (store.ApplyAccountSchedulerOverrides, ApplyAccountAllowedAPIKeys, ApplyAccountTags, ApplyAccountGroups, ApplyAccountProxyURL); change the handler to begin a transaction, perform all Update*/Set* calls using that tx (or a transactional wrapper on h.db), rollback on any error (returning the appropriate 500/404), and only after a successful commit invoke the corresponding h.store.Apply* methods in the same order so runtime state is updated atomically with the DB.
🧹 Nitpick comments (3)
database/helpers.go (1)
376-386: 💤 Low valueConsider logging marshal errors for observability.
The encode functions silently return
"[]"whenjson.Marshalfails. While marshaling[]int64or[]stringshould rarely fail in practice, logging these errors would help with debugging and monitoring.Optional: Add error logging
func encodeInt64SliceJSON(values []int64) string { if len(values) == 0 { return "[]" } b, err := json.Marshal(values) if err != nil { + // Consider: log.Printf("failed to marshal int64 slice: %v", err) return "[]" } return string(b) }Similar change for
encodeTagsJSON.Also applies to: 401-411
🤖 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 `@database/helpers.go` around lines 376 - 386, The encodeInt64SliceJSON (and similarly encodeTagsJSON) functions currently swallow json.Marshal errors and return "[]"; update them to log the marshal error for observability while preserving the existing return behavior — add a log (e.g., using the package logger or log.Printf) in the err branch of encodeInt64SliceJSON and encodeTagsJSON to record the error and the input value context, then still return "[]". Ensure you import the chosen logger (or "log") if not already present.frontend/src/hooks/useHighlighter.ts (2)
42-79: ⚖️ Poor tradeoffNote: Theme changes require component re-render.
The effect reads
document.documentElement.classList.contains('dark')to determine the theme, but doesn't include theme changes in the dependency array. If the theme switches while the component remains mounted with the samecodeandlang, the highlighted output won't update until the component re-renders for another reason.This is likely acceptable behavior since theme changes typically cause broader re-renders, but it's worth documenting if this becomes a user-facing issue.
🤖 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 `@frontend/src/hooks/useHighlighter.ts` around lines 42 - 79, The effect in useHighlighter.ts reads the DOM theme (document.documentElement.classList.contains('dark')) but doesn't react to theme changes, so update the useEffect that wraps getHighlighter()/hl.codeToHtml to either (A) include a reactive theme signal in its dependency array (derive isDark into state/prop and add that dependency) or (B) attach a MutationObserver inside the hook to update a local isDark state when the root class changes and use that state in the effect; ensure you reference the same cacheKey logic (cacheKey constructed from isDark, resolvedLang, code), keep cache eviction using htmlCache and MAX_CACHE_SIZE, and run tuneLightTheme only when appropriate, and clean up the observer on unmount as well as maintaining the cancelled flag and setHtml usage.
58-76: 💤 Low valueConsider adding error logging for debugging.
The
catchblock at line 73 silently sets the HTML to an empty string when highlighting fails. Adding aconsole.warnorconsole.errorwould help developers troubleshoot syntax highlighting issues.Optional: Add error logging
} catch { + console.warn('Syntax highlighting failed:', { code, resolvedLang }) setHtml('') }🤖 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 `@frontend/src/hooks/useHighlighter.ts` around lines 58 - 76, The catch block in the getHighlighter().then(...) call silently swallows highlighting errors; update the catch to accept the error (e.g., catch (err)) and log it with context before continuing (use console.warn or a project logger). Include identifying context such as resolvedLang, isDark, the computed cacheKey (or code length) and the error object when calling the logger, but keep the existing fallback behavior of setHtml('') so failures still render safely; modify the catch associated with getHighlighter, tuneLightTheme, htmlCache and setHtml accordingly.
🤖 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 `@admin/account_groups.go`:
- Around line 160-166: Replace direct equality checks against sql.ErrNoRows with
errors.Is so wrapped errors are detected: in the UpdateAccountGroup and
DeleteAccountGroup handlers (where UpdateAccountGroup and DeleteAccountGroup are
called) change the conditional from `if err == sql.ErrNoRows` to `if
errors.Is(err, sql.ErrNoRows)` and apply the identical change for the other
handler so not-found errors return HTTP 404 instead of falling through to 500.
In `@admin/handler.go`:
- Around line 3935-3940: Move the call to h.store.SetProxyPoolEnabled so it only
runs after h.store.ReloadProxyPool succeeds: when req.ProxyPoolEnabled is true,
first call h.store.ReloadProxyPool() and return the 500 on error, and only then
call h.store.SetProxyPoolEnabled(true) (and persist the setting if applicable);
when disabling (false) you can set it immediately. This ensures the runtime
state (SetProxyPoolEnabled) is not enabled unless ReloadProxyPool succeeds and
the settings row is persisted; update the handler logic around
SetProxyPoolEnabled, ReloadProxyPool, and req.ProxyPoolEnabled accordingly.
- Around line 4755-4765: The handler currently commits DB changes via
h.db.InsertProxies (and similar DB mutators at the other locations) and then
treats h.store.ReloadProxyPool failures as fatal, returning 500 even though the
DB change succeeded; change this so the HTTP response reflects the DB success:
after calling h.db.InsertProxies (and the analogous update/delete functions at
the other ranges) return success to the client when the DB call succeeds, and
handle ReloadProxyPool failures non-fatally by logging the error (using
processLogger or the existing logger) and optionally scheduling/asynchronously
retrying ReloadProxyPool, but do not call writeError or return 500; update the
blocks referencing h.store.ReloadProxyPool (including the ones around
InsertProxies and the other noted ranges) to implement this pattern.
In `@auth/store.go`:
- Around line 3081-3093: SetAPIKeyAllowedGroups and LoadAPIKeyAllowedGroups are
storing raw IDs which can include non-positive values, duplicates, and unsorted
slices; add a normalization step that filters out <=0 IDs, deduplicates and
sorts the remaining int64s before storing into s.apiKeyAllowedGroups (under
s.apiKeyGroupsMu). Implement a small helper (e.g., normalizeInt64IDs) and call
it from SetAPIKeyAllowedGroups and LoadAPIKeyAllowedGroups instead of using
cloneInt64Slice directly so s.apiKeyAllowedGroups[apiKeyID] always contains a
sorted, unique, positive slice for consistent runtime comparisons.
- Around line 3127-3147: APIKeyAllowsAccount is allocating a new map on every
call; instead cache the allowed-group set per API key (or return an immutable
slice to avoid remapping) so hot-path checks don't reallocate. Modify
GetAPIKeyAllowedGroups to provide either a pointer to a precomputed []int64 or
expose a cached map[apiKeyID]map[groupID]struct{} on the Store (e.g.
store.apiKeyAllowedSets) that is built/updated when API-key groups change, and
then change APIKeyAllowsAccount to look up that cached set (using it directly
for membership tests) rather than rebuilding allowedSet on each call; keep the
existing nil/zero checks and account lock usage (Account.mu) intact.
In `@docs/API.md`:
- Around line 984-987: The API docs for account group-scoped scheduling are
missing new scheduler metadata fields; update the PATCH
/api/admin/accounts/:id/scheduler endpoint documentation to include the new
request/response schema fields proxy_url, tags, and group_ids (and describe
their types and semantics), ensure examples and parameter lists reference these
fields, and mention any validation rules or defaults used by the
scheduler-related handlers (e.g., allowed values for tags, format for proxy_url,
and semantics of group_ids) so the frontend/integrators have a complete
contract.
In `@frontend/src/components/ChipInput.tsx`:
- Around line 72-79: The bug is that handleChange splits on commas and calls
addChip multiple times while closing over a stale draft/value, causing chips to
be lost; fix by updating state and adding chips using functional updaters so
each addition sees the latest state: in handleChange, when v.includes(',')
iterate trimmed parts, call addChip (or the underlying setChips) using its
functional form (e.g., setChips(prev => [...prev, partTrimmed])) for each part,
then call setDraft(lastPart) (or setDraft(() => lastPart)) — ensure addChip
either delegates to the functional setChips or is changed to accept/apply the
functional updater so no closure over the old draft occurs.
In `@frontend/src/pages/Accounts.tsx`:
- Around line 1451-1455: After deleting a group you refresh groups but not
accounts, leaving account.group_ids containing the deleted id; update the flow
in the deletion handler (around showToast, setEditGroupIds, setGroupFilter,
resetGroupDraft, reloadGroups) to also refresh accounts by calling await
reloadAccounts() after await reloadGroups(), or alternatively update the local
accounts state by mapping over accounts and removing the deleted group.id from
each account.group_ids so editors and subsequent updateAccountScheduler calls
see fresh/valid data.
In `@frontend/src/pages/APIKeys.tsx`:
- Around line 758-760: The badge currently shows "none" when allowed_group_ids
is an empty array (ids variable), but an empty array means "no restriction" (all
groups). Update the render logic in the APIKeys component where ids is used (the
if (ids.length === 0) branch) to display the "all groups" translation instead of
'apiKeys.allowedGroupsNone' — e.g., use t('apiKeys.allowedGroupsAll') or an
equivalent "all" label (with fallback to a plain "All groups" string) so the UI
reflects the backend semantics.
In `@frontend/src/pages/Docs.tsx`:
- Around line 189-203: The new hardcoded Chinese strings in Docs.tsx (e.g., the
button labels '已复制', '复制链接', '打开客户端', and the expand button text using expanded
and setExpanded) must be replaced with localized strings from the app's
i18n/translation utility (e.g., use the project's t(...) or useTranslation hook)
so /docs is fully translatable; update the Button labels and the expand toggle
to call the translation function (create keys like docs.copy.copied,
docs.copy.copyLink, docs.openClient, docs.viewFullLink and
docs.collapseFullLink) and apply the same change to the other occurrences around
the file (also check the region noted 780-897) so no UI text is hardcoded.
- Around line 1071-1085: The admin EndpointDoc components are being seeded with
the downstream API key (activeKey) and full allKeys list, causing admin requests
to prefill with the wrong credential type; in the adminEndpoints map, find the
admin credential from allKeys (e.g., const adminSeed = allKeys.find(k => k.type
=== 'admin') or filter allKeys to type 'admin') and pass that as the apiKey prop
(and replace allKeys prop with the filtered admin-only list if needed) when
rendering EndpointDoc so the dialog pre-fills X-Admin-Key with an admin
credential instead of a downstream API key.
In `@frontend/src/pages/docs/EndpointDoc.tsx`:
- Around line 63-74: The button labels and tooltip strings (e.g., the aria-label
values '已复制' and '复制' used near the handleCopy call and the conditional icon
render in EndpointDoc.tsx, and the similar hardcoded Chinese strings between
lines 226-281) must be moved to the app i18n system: replace the hardcoded
Chinese text with translation keys and call the project's existing translation
helper (e.g., the t function or useTranslation hook used elsewhere) to render
localized strings (e.g., t('docs.copy'), t('docs.copied')), import/use that
helper in EndpointDoc, and update any referenced keys in the locale files for
both Chinese and English so the labels (aria-label and visible text) are
localized across locales.
In `@frontend/src/pages/docs/quickStartTools.ts`:
- Around line 140-143: The current placeholder substitution uses
base.split('{address}').join(address).split('{key}').join(key) which injects raw
values and can break URLs; update the substitution to URL-encode the dynamic
values first (use encodeURIComponent on address and key) and then replace
'{address}' and '{key}' in base so the generated protocol/query links remain
valid; locate the substitution logic in the function that returns this base
string and replace the join arguments with the encoded variables.
In `@frontend/src/pages/Guide.tsx`:
- Around line 29-54: The client tools array CLIENT_TOOLS in Guide.tsx currently
contains hardcoded user-facing strings (id, name, badge, blurb, glyph, tone);
replace those literal labels with locale keys and call the i18n translator
(e.g., t(...) or the project's i18n lookup) when rendering so the strings come
from locale resource keys (create keys like guide.client.codex.name,
guide.client.codex.blurb, guide.client.codex.badge, etc. and use them in the
component that renders CLIENT_TOOLS); apply the same change for all other
hardcoded copy in this file referenced by the reviewer (lines ~259-436) so
headings, labels and help text are routed through the same i18n keys and
translation function.
In `@frontend/src/pages/Proxies.tsx`:
- Around line 16-23: The batch-add flow is not using validateProxyInput(), so
malformed/unsupported proxy URLs get submitted; update the add/batch add handler
to run validateProxyInput(url) for each entered URL (reuse the same allowed
protocols: http, https, socks5, socks5h) and block submission if any URL fails
validation, surfacing per-item validation errors to the user instead of sending
to the server; ensure the same validation logic used in edit mode
(validateProxyInput) is invoked before the network request and returns a clear
client-side error for each invalid entry.
---
Outside diff comments:
In `@admin/handler.go`:
- Around line 723-786: The DB updates (UpdateAccountSchedulerConfig,
UpdateAccountTags, SetAccountGroups, UpdateAccountProxyURL) must be executed
inside a single transaction and only after the transaction commits should you
call runtime mutations (store.ApplyAccountSchedulerOverrides,
ApplyAccountAllowedAPIKeys, ApplyAccountTags, ApplyAccountGroups,
ApplyAccountProxyURL); change the handler to begin a transaction, perform all
Update*/Set* calls using that tx (or a transactional wrapper on h.db), rollback
on any error (returning the appropriate 500/404), and only after a successful
commit invoke the corresponding h.store.Apply* methods in the same order so
runtime state is updated atomically with the DB.
In `@database/postgres.go`:
- Around line 3139-3148: The SELECT that reads credentials (selectQuery)
currently only filters by id and thus can lock and update soft-deleted accounts;
change the query to include the same non-deleted predicate used in the
scheduler-update existence check (e.g., add "AND <non-deleted-predicate>" such
as "AND deleted_at IS NULL" or the project's equivalent) for both the SQLite and
non-SQLite branches, preserving the FOR UPDATE for non-SQLite; ensure the
parameter order passed to tx.QueryRowContext(ctx, selectQuery, id, ...) matches
the added predicate parameters and update references to selectQuery,
db.isSQLite(), tx.QueryRowContext and id accordingly.
In `@frontend/src/pages/Proxies.tsx`:
- Around line 179-189: The API handler currently only treats thrown errors as
failures; update the logic after calling api.testProxy(p.url, p.id, ipApiLang)
to explicitly handle result.success === false as a failure: when result.success
is false, call showToast with t('proxies.testFailed', { error: result.error ||
getErrorMessage(new Error('test failed')) }) (or similar), update setProxies so
the row reflects the failed test (clear or set appropriate test_* fields), and
ensure any test-all tracking state (e.g., testAllFailed/testAllProgress flags)
is incremented/updated the same way as in the catch block; apply the same
check-and-failure handling to the other handler around lines 208-219.
---
Nitpick comments:
In `@database/helpers.go`:
- Around line 376-386: The encodeInt64SliceJSON (and similarly encodeTagsJSON)
functions currently swallow json.Marshal errors and return "[]"; update them to
log the marshal error for observability while preserving the existing return
behavior — add a log (e.g., using the package logger or log.Printf) in the err
branch of encodeInt64SliceJSON and encodeTagsJSON to record the error and the
input value context, then still return "[]". Ensure you import the chosen logger
(or "log") if not already present.
In `@frontend/src/hooks/useHighlighter.ts`:
- Around line 42-79: The effect in useHighlighter.ts reads the DOM theme
(document.documentElement.classList.contains('dark')) but doesn't react to theme
changes, so update the useEffect that wraps getHighlighter()/hl.codeToHtml to
either (A) include a reactive theme signal in its dependency array (derive
isDark into state/prop and add that dependency) or (B) attach a MutationObserver
inside the hook to update a local isDark state when the root class changes and
use that state in the effect; ensure you reference the same cacheKey logic
(cacheKey constructed from isDark, resolvedLang, code), keep cache eviction
using htmlCache and MAX_CACHE_SIZE, and run tuneLightTheme only when
appropriate, and clean up the observer on unmount as well as maintaining the
cancelled flag and setHtml usage.
- Around line 58-76: The catch block in the getHighlighter().then(...) call
silently swallows highlighting errors; update the catch to accept the error
(e.g., catch (err)) and log it with context before continuing (use console.warn
or a project logger). Include identifying context such as resolvedLang, isDark,
the computed cacheKey (or code length) and the error object when calling the
logger, but keep the existing fallback behavior of setHtml('') so failures still
render safely; modify the catch associated with getHighlighter, tuneLightTheme,
htmlCache and setHtml accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 19aeb195-df70-40bb-a0b6-8b5d23668c49
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
admin/account_groups.goadmin/handler.goadmin/handler_test.goadmin/responses.goauth/fast_scheduler.goauth/fast_scheduler_test.goauth/session_affinity_test.goauth/store.godatabase/account_groups.godatabase/helpers.godatabase/image_studio.godatabase/postgres.godatabase/sqlite.godatabase/sqlite_test.godocs/API.mdfrontend/package.jsonfrontend/src/App.tsxfrontend/src/api.tsfrontend/src/components/ChipInput.tsxfrontend/src/components/Layout.tsxfrontend/src/components/PageHeader.tsxfrontend/src/components/UsageStatsSummary.tsxfrontend/src/components/ui/card.tsxfrontend/src/hooks/useHighlighter.tsfrontend/src/index.cssfrontend/src/locales/en.jsonfrontend/src/locales/zh.jsonfrontend/src/pages/APIKeys.tsxfrontend/src/pages/Accounts.tsxfrontend/src/pages/Dashboard.tsxfrontend/src/pages/Docs.tsxfrontend/src/pages/Guide.tsxfrontend/src/pages/Proxies.tsxfrontend/src/pages/Usage.tsxfrontend/src/pages/docs/DocsTOC.tsxfrontend/src/pages/docs/EndpointDoc.tsxfrontend/src/pages/docs/docsContent.tsfrontend/src/pages/docs/quickStartTools.tsfrontend/src/types.tsproxy/handler.go
Comment on lines
+29
to
+54
| const CLIENT_TOOLS: ClientTool[] = [ | ||
| { | ||
| id: 'codex', | ||
| name: 'Codex CLI', | ||
| badge: 'Responses', | ||
| blurb: 'Write config.toml and auth.json for the OpenAI Responses wire API.', | ||
| glyph: 'CX', | ||
| tone: 'bg-emerald-500/10 text-emerald-600 dark:text-emerald-400', | ||
| }, | ||
| { | ||
| id: 'claude', | ||
| name: 'Claude Code', | ||
| badge: 'Anthropic', | ||
| blurb: 'Use environment variables or settings.json for the Messages endpoint.', | ||
| glyph: 'CC', | ||
| tone: 'bg-orange-500/10 text-orange-600 dark:text-orange-400', | ||
| }, | ||
| { | ||
| id: 'cc-switch', | ||
| name: 'CC Switch', | ||
| badge: 'Deeplink', | ||
| blurb: 'Launch the desktop switcher and import this server as a provider.', | ||
| glyph: 'CS', | ||
| tone: 'bg-fuchsia-500/10 text-fuchsia-600 dark:text-fuchsia-400', | ||
| }, | ||
| ] |
There was a problem hiding this comment.
Reintroduce i18n binding for Guide copy.
The page now hardcodes user-facing strings, which regresses localization support (especially Chinese) across this admin flow. Please route labels/headings/help text back through locale keys.
Also applies to: 259-436
🤖 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 `@frontend/src/pages/Guide.tsx` around lines 29 - 54, The client tools array
CLIENT_TOOLS in Guide.tsx currently contains hardcoded user-facing strings (id,
name, badge, blurb, glyph, tone); replace those literal labels with locale keys
and call the i18n translator (e.g., t(...) or the project's i18n lookup) when
rendering so the strings come from locale resource keys (create keys like
guide.client.codex.name, guide.client.codex.blurb, guide.client.codex.badge,
etc. and use them in the component that renders CLIENT_TOOLS); apply the same
change for all other hardcoded copy in this file referenced by the reviewer
(lines ~259-436) so headings, labels and help text are routed through the same
i18n keys and translation function.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/API.md (1)
1242-1272:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winJSON 示例里包含行内注释，示例不可直接使用。
"id": 1, // 可选...不是合法 JSON；复制后会解析失败。建议把注释放到代码块外。🛠️ Suggested fix
{ "url": "http://proxy.example.com:8080", - "id": 1, // 可选，用于持久化测试结果 + "id": 1, "lang": "zh-CN" }在代码块下补一句说明：
id可选，用于持久化测试结果。🤖 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/API.md` around lines 1242 - 1272, The JSON example for the POST /api/admin/proxies/test endpoint contains an inline comment inside the code block ("id": 1, // 可选...), which is invalid JSON; remove the inline comment from the code block and add a plain-text note immediately below the block stating that the id field in the POST /api/admin/proxies/test request is optional and is used to persist test results (e.g., "注意：id 为可选字段，用于持久化测试结果。"). Ensure the code block only contains valid JSON and the explanatory sentence references the "id" field and the POST /api/admin/proxies/test endpoint.
🤖 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/API.md`:
- Around line 653-657: 两个缺少语言标记的 fenced code block（内容分别以 "rt_xxxxxx1 /
rt_xxxxxx2 / rt_xxxxxx3" 开头和以 "data: {\"type\":\"progress\"...}" / "data:
{\"type\":\"complete\"...}" 开头）触发 MD040 lint 错误；为这两个 code block 在起始 ```
后添加语言标记（例如 ```text）以标明内容类型并修复 lint 报错，保持原有块内内容不变并对文档中相同的另一个块（内容如上）做相同修改。
In `@frontend/src/hooks/useHighlighter.ts`:
- Around line 8-29: getHighlighter currently sets a singleton highlighterPromise
which, if the dynamic imports fail, will remain rejected and "poison" future
calls; modify getHighlighter (and the similar logic at lines 78-103) to attach a
.catch handler to the Promise that resets highlighterPromise to null (or
undefined) before rethrowing the error so subsequent calls can retry
initialization, and ensure the original error is propagated after resetting the
singleton.
In `@frontend/src/pages/docs/EndpointDoc.tsx`:
- Around line 472-493: The Try-it UI should be disabled for endpoints with path
parameters or non-JSON payloads; update the render logic around the Button and
TryItDialog (symbols: Button, setTryOpen, TryItDialog, method, path,
defaultBody, apiKey, baseUrl, allKeys) to only show them when the path contains
no ":" (i.e., !path.includes(':')) and the payload is JSON-compatible for
methods that accept bodies (implement a small isJson check that attempts
JSON.parse on defaultBody and only allow for POST/PUT/PATCH when parse succeeds,
while allowing GET/DELETE with empty/defaultBody); wrap both the Button and
TryItDialog in that guard so the dialog never submits unsupported baseUrl+path
or non-JSON payloads.
---
Outside diff comments:
In `@docs/API.md`:
- Around line 1242-1272: The JSON example for the POST /api/admin/proxies/test
endpoint contains an inline comment inside the code block ("id": 1, // 可选...),
which is invalid JSON; remove the inline comment from the code block and add a
plain-text note immediately below the block stating that the id field in the
POST /api/admin/proxies/test request is optional and is used to persist test
results (e.g., "注意：id 为可选字段，用于持久化测试结果。"). Ensure the code block only contains
valid JSON and the explanatory sentence references the "id" field and the POST
/api/admin/proxies/test endpoint.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 02266c09-7018-46f4-ac33-70333ee98985
📒 Files selected for processing (19)
admin/account_groups.goadmin/handler.goauth/store.godatabase/account_groups.godatabase/postgres.godatabase/sqlite.godatabase/sqlite_test.godocs/API.mdfrontend/src/components/ChipInput.tsxfrontend/src/hooks/useHighlighter.tsfrontend/src/locales/en.jsonfrontend/src/locales/zh.jsonfrontend/src/pages/APIKeys.tsxfrontend/src/pages/Accounts.tsxfrontend/src/pages/Docs.tsxfrontend/src/pages/Proxies.tsxfrontend/src/pages/docs/EndpointDoc.tsxfrontend/src/pages/docs/docsContent.tsfrontend/src/pages/docs/quickStartTools.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/src/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/src/components/ChipInput.tsx
- auth/store.go
- admin/account_groups.go
- admin/handler.go
- frontend/src/pages/Proxies.tsx
- frontend/src/pages/APIKeys.tsx
DeliciousBuding
force-pushed
the
rebase-onto-v2.1.3
branch
from
1cc2682 to
fb405c5
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/pages/docs/EndpointDoc.tsx (2)
213-254: ⚡ Quick winAbort in-flight Try-it requests on close/resend to prevent late state writes.
handleSendissuesfetchwithout anAbortController. If the user closes the dialog mid-request or clicks Send again before the previous call resolves, the prior promise still runs to completion and writesstatus/response/durationafter the dialog has unmounted or for a stale invocation, producing flicker and out-of-order updates in the response panel. Tying a controller into both the open-effect cleanup and the start of each send avoids those races without changing the happy path.♻️ Suggested cleanup with AbortController
const [duration, setDuration] = useState<number | null>(null); + const abortRef = useRef<AbortController | null>(null); useEffect(() => { if (open) { setBody(defaultBody); setToken(apiKey); setResponse(""); setStatus(null); setDuration(null); } + return () => { + abortRef.current?.abort(); + abortRef.current = null; + }; }, [open, defaultBody, apiKey]); const handleSend = async () => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; setLoading(true); ... const res = await fetch(url, { method, headers, body: isGet ? undefined : body.trim() || undefined, + signal: controller.signal, }); ... } catch (e) { + if (controller.signal.aborted) return; setDuration(Math.round(performance.now() - start)); setResponse(`Error: ${e instanceof Error ? e.message : String(e)}`); } finally { - setLoading(false); + if (!controller.signal.aborted) setLoading(false); } };
useRefis already imported at Line 1.🤖 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 `@frontend/src/pages/docs/EndpointDoc.tsx` around lines 213 - 254, handleSend currently calls fetch without an AbortController, causing previous requests to update state after unmount or when a newer send starts; create a controller stored in a ref (e.g., requestControllerRef) and before starting a new request abort any existing controller and replace it, pass controller.signal to fetch in handleSend, and ensure the dialog/component cleanup effect aborts the active controller on unmount; additionally, when catching errors inside handleSend, treat an AbortError (or e.name === "AbortError") as a no-op so you don't call setStatus/setResponse for aborted requests, and clear the controller ref after the request completes.
103-114: 💤 Low valueStyling check uses
langinstead of the resolved language.
resolvedLang(Line 24–30) derives the highlight language fromlabelwhenlangis omitted, but the conditional that switches betweentext-[13px]andtext-smstill gates on the rawlangprop. A block invoked as<CodeBlock label="x.json" content={...} />is highlighted as JSON but rendered with the bash-sized font, producing a subtle visual mismatch in the response examples that already passlangseparately.♻️ Use `resolvedLang` for size selection
{highlightedHtml ? ( <div - className={`code-panel-pre shiki-wrapper ${lang === "json" ? "text-[13px]" : "text-sm"}`} + className={`code-panel-pre shiki-wrapper ${resolvedLang === "json" ? "text-[13px]" : "text-sm"}`} dangerouslySetInnerHTML={{ __html: highlightedHtml }} /> ) : ( <pre - className={`code-panel-pre ${lang === "json" ? "text-[13px]" : "text-sm"}`} + className={`code-panel-pre ${resolvedLang === "json" ? "text-[13px]" : "text-sm"}`} > <code>{content}</code> </pre> )}🤖 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 `@frontend/src/pages/docs/EndpointDoc.tsx` around lines 103 - 114, The font-size conditional is using the raw prop lang instead of the computed resolvedLang (which may derive from label), so update the className ternaries in both branches that render highlightedHtml (the div with dangerouslySetInnerHTML) and the fallback <pre><code> branch to use resolvedLang instead of lang; locate the resolvedLang variable (lines ~24–30) and swap the conditionals that choose "text-[13px]" vs "text-sm" to base on resolvedLang so JSON-labelled content gets the correct size when highlighted.
🤖 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.
Nitpick comments:
In `@frontend/src/pages/docs/EndpointDoc.tsx`:
- Around line 213-254: handleSend currently calls fetch without an
AbortController, causing previous requests to update state after unmount or when
a newer send starts; create a controller stored in a ref (e.g.,
requestControllerRef) and before starting a new request abort any existing
controller and replace it, pass controller.signal to fetch in handleSend, and
ensure the dialog/component cleanup effect aborts the active controller on
unmount; additionally, when catching errors inside handleSend, treat an
AbortError (or e.name === "AbortError") as a no-op so you don't call
setStatus/setResponse for aborted requests, and clear the controller ref after
the request completes.
- Around line 103-114: The font-size conditional is using the raw prop lang
instead of the computed resolvedLang (which may derive from label), so update
the className ternaries in both branches that render highlightedHtml (the div
with dangerouslySetInnerHTML) and the fallback <pre><code> branch to use
resolvedLang instead of lang; locate the resolvedLang variable (lines ~24–30)
and swap the conditionals that choose "text-[13px]" vs "text-sm" to base on
resolvedLang so JSON-labelled content gets the correct size when highlighted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d9694e25-8178-4427-9f37-d3b4883626a2
📒 Files selected for processing (9)
database/account_groups.godatabase/sqlite_test.godocs/API.mdfrontend/src/hooks/useHighlighter.tsfrontend/src/pages/Docs.tsxfrontend/src/pages/Proxies.tsxfrontend/src/pages/docs/EndpointDoc.tsxfrontend/src/pages/docs/docsContent.tsfrontend/src/pages/docs/quickStartTools.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/hooks/useHighlighter.ts
- frontend/src/pages/Docs.tsx
- frontend/src/pages/docs/quickStartTools.ts
- frontend/src/pages/Proxies.tsx
- database/sqlite_test.go
Owner
|
真不错啊哈哈哈 |
Replaces direct == comparison with errors.Is to correctly handle wrapped errors from database drivers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更概览
本 PR 基于
v2.1.3，对管理后台做了一轮较完整的产品化重构和能力补完，重点不在单点修 bug，而在于把文档中心、账号管理、API Key 管理、仪表盘与基础交互体系整体拉到一个更可用、可维护、可升级的状态。这次改动可以概括为五条主线：
/admin/docs主要贡献
1. 重构文档中心
/admin/docsdocs/API.md同步补齐，保证仓库文档与站内文档一致。2. 完善账号管理
GET /api/admin/account-groupsPOST /api/admin/account-groupsPATCH /api/admin/account-groups/:idDELETE /api/admin/account-groups/:idtagsgroup_idsproxy_urlallowed_api_key_idsgroup_ids。3. 完善 API Key 管理
allowed_group_ids0/null可清空额度或过期时间4. 仪表盘、使用统计、代理页与前端基础体验优化
UsageStatsSummary，将流量、Token、缓存、健康四组信息统一整理。ChipInputUsageStatsSummaryDocsTOCEndpointDoc5. review 后补的稳定性修复
升级兼容性
api_keys新增allowed_group_ids字段。account_groups/usage_stats_baseline缺列场景会自动补齐。usage_stats_baseline现在会同时保留缓存命中统计和首字延迟样本，避免清日志后 Dashboard 相关指标掉空。allowed_group_ids为空数组仍表示不限制账号分组。quota_limit/quota传0或null可清空额度限制。expires_at传null或expires_in_days传0可清空过期时间。验证
已通过：
go test ./admin ./database ./auth ./proxynpm run typecheck --prefix frontendnpm run build --prefix frontendgit diff --check页面截图
仪表盘
文档中心
Summary by CodeRabbit
New Features
Documentation
Bug Fixes