feat: add client skill market admin management - #4358
Open
Vinoxcn wants to merge 2 commits into
Open
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a client skill marketplace: new GORM model and migrations, public/admin APIs and routes for listing, creating, editing, and status toggling skills, a SkillHub download proxy endpoint, middleware tweaks (auth, cache, rate-limit), and a full React admin UI for skill market management. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as SkillMarket API (Gin)
participant Service as service.DoDownloadRequest
participant SkillHub
Client->>API: GET /api/client/skills/skillhub/download?slug=<slug>
API->>API: validate & trim slug\nbuild upstream URL (skillHubDownloadBaseURL + url.QueryEscape(slug))
API->>Service: DoDownloadRequest(upstreamURL)
Service->>SkillHub: HTTP GET upstreamURL
SkillHub-->>Service: 200/4xx/5xx + headers + body stream
Service-->>API: upstream *http.Response
API->>API: check status code, parse Content-Type and Content-Disposition\ndetermine filename fallback to path or "<slug>.zip"
API->>Client: respond 200 with headers (Content-Type, Content-Disposition, Cache-Control)\nstream body via DataFromReader
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/components/layout/SiderBar.jsx (2)
202-210: 🛠️ Refactor suggestion | 🟠 MajorSkill-market bypass conflicts with
SidebarModulesAdmindesign.Forcing
skill-marketto always render for admins (line 204) sidesteps theisModuleVisible('admin', …)check that every other admin entry uses. Per prior guidance, admin access to console modules should be governed by theSidebarModulesAdminconfiguration system rather than hardcoded allowlists. If the intent is to have the module visible by default, add'skill-market': trueto the default admin module configuration only (which this PR already does inuseSidebar.js), and let the normal gate apply.♻️ Proposed change
const filteredItems = items.filter((item) => { - // 技能管理是本地扩展核心入口，管理员下始终显示，避免被配置误隐藏。 - if (item.itemKey === 'skill-market') return isAdmin(); const configVisible = isModuleVisible('admin', item.itemKey); return configVisible; });Based on learnings: "Admin access to console.* modules should be governed by this configuration system, not bypassed with hardcoded allowlists."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/SiderBar.jsx` around lines 202 - 210, Remove the hardcoded admin bypass for the 'skill-market' item in the filteredItems logic (the special-case item.itemKey === 'skill-market' return isAdmin()) and instead let the normal visibility gate run via isModuleVisible('admin', item.itemKey); ensure the default admin config in useSidebar.js already contains 'skill-market': true so it shows by default for admins, and keep the visibility decision centralized in SidebarModulesAdmin (i.e., only use isModuleVisible('admin', ...) here rather than calling isAdmin() to force visibility).
487-498:⚠️ Potential issue | 🟠 MajorAdmin section now ignores module-visibility configuration entirely.
Dropping
hasSectionVisibleModules('admin')in favor of justisAdmin()means the admin section renders even when a deployment explicitly disables every admin module viaSidebarModulesAdmin. Other sections (chat/console/personal) still respect their visibility configs; making admin asymmetric breaks the configurable permission model and regresses the behavior called out in prior reviews.♻️ Proposed change
- {/* 管理员区域 - 管理员始终显示，避免配置误隐藏 */} - {isAdmin() && ( + {/* 管理员区域 */} + {isAdmin() && hasSectionVisibleModules('admin') && (Based on learnings: the sidebar management system uses
SidebarModulesAdminfor granular permission control — admin visibility should not be bypassed with hardcoded checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/SiderBar.jsx` around lines 487 - 498, The admin section rendering bypasses module-visibility checks by using isAdmin() alone, so when all admin modules are disabled (via SidebarModulesAdmin) the section still appears; restore the visibility guard by adding the hasSectionVisibleModules('admin') check alongside isAdmin() before rendering the admin block (the same pattern used for chat/console/personal), i.e., ensure the conditional uses both isAdmin() && hasSectionVisibleModules('admin') so adminItems are only rendered when the user is an admin and the 'admin' section has visible modules.
🧹 Nitpick comments (4)
middleware/rate-limit.go (1)
93-101: Remove or validate the loopback IP bypass more rigorously.The concern about header-derived IP spoofing is mitigated here—no
SetTrustedProxiesconfiguration exists, soc.ClientIP()reads only the actual remote address. However, the bypass itself has code smell:strings.HasPrefix(clientIP, "localhost")cannot match actual IP addresses returned byc.ClientIP(), and there is no validation that the IP is genuinely loopback (e.g., usingnet.ParseIP(...).IsLoopback()). Consider either removing this bypass entirely, or rewrite it to usenet.ParseIP()for robust loopback detection, and add a comment explaining why loopback should skip rate limiting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware/rate-limit.go` around lines 93 - 101, The loopback IP bypass in the middleware currently uses brittle string checks (clientIP == "127.0.0.1" || clientIP == "::1" || strings.HasPrefix(clientIP, "localhost")) which is incorrect and unsafe; update the rate-limit handler that wraps limiter (and references rateLimitFactory and c.ClientIP()) to either remove the bypass entirely or replace it with a robust check using net.ParseIP(clientIP) and ip.IsLoopback() to detect loopback addresses (covering IPv4 and IPv6), drop the strings.HasPrefix("localhost") check, and add a short comment above the bypass explaining why genuine loopback traffic is exempt from rate limiting if you choose to keep it.web/src/pages/SkillMarket/Editor.jsx (2)
313-335: Two bound controls for the samecategoryfield create confusing UX.The free-text
<input>and the preset<select>both read from and write toeditingSkill.category. Typing in the input will be visibly overridden the moment the user interacts with the select (and vice versa), and the "current nav" hint beneath updates only from whatever wrote last. Either consolidate into a single combobox, or keep the select purely as a "pick a preset to fill the input" helper that writes via a button/onChange without sharing the controlled value directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/SkillMarket/Editor.jsx` around lines 313 - 335, The UI currently binds both the free-text input and the preset select to the same controlled value (editingSkill.category) causing them to fight; change the select to act as a helper picker rather than a second bound control: keep the text <input> as the single source of truth for category (controlled by editingSkill.category and updateField), remove the select's value binding to editingSkill.category and onChange instead call updateField('category', chosenValue) or populate the input only when a user explicitly picks a preset (e.g., via onChange or a small “apply preset” action), and ensure getMylclawNavLabel(editingSkill.category) remains driven by the input value; reference symbols: editingSkill, updateField, getMylclawNavLabel, CATEGORY_PRESETS, fieldClassName, labelClassName.
5-91: Duplicate module-level constants/helpers withindex.jsx.
defaultSkillForm,CATEGORY_PRESETS,MYCLAW_NAV_LABELS,normalizeSkillForm,parseTags, andgetMylclawNavLabelare copy-pasted verbatim betweenEditor.jsx(lines 5–85) andindex.jsx(lines 20–100). Extract them into a singleweb/src/pages/SkillMarket/constants.js(orutils.js) so preset drift can't occur.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/SkillMarket/Editor.jsx` around lines 5 - 91, The file duplicates module-level constants and helpers (defaultSkillForm, CATEGORY_PRESETS, MYCLAW_NAV_LABELS, normalizeSkillForm, parseTags, getMylclawNavLabel, plus fieldClassName/labelClassName) that also exist in index.jsx; extract these into a new shared module (e.g., web/src/pages/SkillMarket/constants.js or utils.js), export each symbol, then replace the in-file definitions in both Editor.jsx and index.jsx with imports of those exported names; ensure normalizeSkillForm still uses defaultSkillForm and parseTags remains available where used so behavior is preserved.controller/client_skills.go (1)
254-273: Admin create accepts (and persists) a client-supplieddownloadsvalue.
normalizeClientSkillInputpassesskill.Downloadsthrough unchanged (line 141), so an admin call can seed or reset the counter to any integer, including negative values. If that's intentional (manual migration, correction), fine — but worth guarding against< 0, and consider dropping the field fromAdminCreateClientSkillso new skills always start at0(the GORM default) and rely onAdminUpdateClientSkillfor adjustments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/client_skills.go` around lines 254 - 273, AdminCreateClientSkill currently persists a client-supplied downloads value via normalizeClientSkillInput, allowing negative or arbitrary counters; fix by preventing arbitrary seeds on create: in AdminCreateClientSkill (before calling normalizeClientSkillInput) set req.Skill.Downloads = 0 (or validate and clamp to >=0), or alternatively change normalizeClientSkillInput to ignore/override Downloads when called for creates (ensure new items always have Downloads = 0), and keep AdminUpdateClientSkill as the authorized path for adjustments; reference AdminCreateClientSkill, normalizeClientSkillInput, AdminUpsertClientSkillRequest, skill.Downloads, model.DB.Create, and AdminUpdateClientSkill 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 `@controller/client_skillhub.go`:
- Line 25: The public streaming handler ClientProxySkillHubDownload currently
allows unlimited downloads; add rate limiting by attaching
middleware.DownloadRateLimit() (or equivalent request/stream bandwidth limiter)
to the route that registers ClientProxySkillHubDownload so the route reads like:
clientRoute.GET("/skills/skillhub/download", middleware.DownloadRateLimit(),
controller.ClientProxySkillHubDownload); if you cannot change the route
registration, wrap the handler with the limiter inside the handler entrypoint
(or call a limiter middleware function at start of ClientProxySkillHubDownload)
to enforce per-client/download limits and prevent bandwidth abuse.
- Around line 32-33: The code concatenates raw slug into upstreamURL
(upstreamURL := skillHubDownloadBaseURL + slug), which allows special characters
to break the upstream request; fix by URL-encoding the slug before building the
upstream URL (e.g., use url.QueryEscape(slug) or build query parameters with
url.Values) and assign upstreamURL to skillHubDownloadBaseURL + encodedSlug so
the subsequent call to service.DoDownloadRequest("skillhub skill download
proxy", slug) uses a safe upstreamURL; update references to upstreamURL and keep
the slug passed to DoDownloadRequest unchanged if the service expects the raw
slug.
In `@controller/client_skills.go`:
- Around line 275-334: AdminUpdateClientSkill currently unconditionally writes
the "downloads" field from the request which can clobber concurrent increments
performed by ClientRecordSkillDownload; remove "downloads" from the updateMap in
AdminUpdateClientSkill so admin saves don't overwrite live counters, and instead
implement a dedicated endpoint (e.g., AdminResetClientSkillDownloads or similar)
to explicitly set/reset the counter if needed; alternatively, if a mutation via
admin must adjust the counter, perform a read-modify-write inside a DB
transaction in AdminUpdateClientSkill that reads the current downloads, computes
the intended new value deterministically, and writes that value using the
transaction to avoid lost updates.
- Around line 195-218: The current call to
model.IncrementClientSkillMarketDownload in ClientRecordSkillDownload treats a
nil error as a guaranteed increment; modify the model function
IncrementClientSkillMarketDownload to return (int64, error) where int64 is
RowsAffected (use db.RowsAffected after the Update), then update
ClientRecordSkillDownload to handle the returned rows: if err != nil log and
fall through, if rows > 0 return ApiSuccess, if rows == 0: check
defaultClientPublicSkills for a matching skill ID and return success if found,
otherwise return ApiErrorMsg "技能不存在". Ensure you update all call sites and the
model implementation to surface RowsAffected from the DB update.
- Around line 3-13: The code directly imports encoding/json and calls
json.Marshal in tagsToJSON; replace that usage with the project wrapper
common.Marshal and remove the direct encoding/json import: update imports to
drop "encoding/json", change tagsToJSON to call common.Marshal(tags) (handling
returned ([]byte, error) same as before), and ensure error paths and callers
(e.g., tagsToJSON) remain unchanged in behavior while using common.Marshal.
In `@middleware/auth.go`:
- Around line 119-127: The strconv.Atoi parse error for apiUserIdStr currently
returns 401; instead, if this request is using session auth (i.e. you can get a
valid currentUserID from the context or session), treat the malformed
New-Api-User header as recoverable: on err, fetch currentUserID (e.g. from
c.Get("currentUserID") or the same session key used elsewhere), overwrite the
New-Api-User header/value with that currentUserID and continue processing (do
not c.Abort/return); keep the strict 401 behavior only for access-token auth
paths—use the auth method flag or presence/absence of access-token to decide
which branch to apply; update the code handling apiUserIdStr / apiUserId to
reflect this fallback and ensure subsequent logic reads the overwritten header
or parsed currentUserID.
In `@middleware/cache.go`:
- Around line 12-21: The current middleware sets long-lived Cache-Control for
assets but always writes revalidation headers; update the logic in the handler
that checks uri (strings.HasPrefix(uri, "/assets/")) so that c.Header("Pragma",
"no-cache") and c.Header("Expires", "0") are only set in the non-assets branch
(the else that sets "Cache-Control" to "no-store, no-cache, must-revalidate"),
leaving asset responses with only the immutable Cache-Control header; locate the
condition around strings.HasPrefix(uri, "/assets/") and move the two c.Header
calls into that else branch.
In `@model/client_skill_market.go`:
- Around line 14-24: The composite unique index
uk_client_skill_source_slug_delete_at will collide for defaulted NOT NULL
SourcePlatform='manual' and SourceSlug='' values; fix by making SourceSlug
nullable and removing its default/NOT NULL constraint in the model (update the
SourceSlug field tags to drop "not null" and "default:''" so it can be NULL), or
alternatively implement a validation in the controller to require a non-empty
SourceSlug before insert, or change the DB index to a partial unique index that
only applies when source_slug != '' — reference the SourcePlatform, SourceSlug
fields and the uk_client_skill_source_slug_delete_at unique index when making
the change.
- Around line 3-8: Remove the direct encoding/json import and replace the
json.Unmarshal call in model/client_skill_market.go with the repo JSON wrapper
from common/json.go (e.g., common.UnmarshalJSON or common.JSONUnmarshal) so all
JSON operations go through the common package; update the import list to remove
"encoding/json", call the wrapper where json.Unmarshal is used (preserving the
same input/output and error handling around that call in the function that
performs the unmarshal), and ensure the error is handled/returned exactly as
before.
- Around line 12-38: The struct ClientSkillMarketItem uses gorm:"type:json" for
the Tags and RawPayload fields which relies on implicit dialect mapping; update
their GORM tags to use type:text to follow the repo guideline and ensure
cross-database compatibility. Locate the ClientSkillMarketItem definition and
change the struct tags for the Tags (JSONValue) and RawPayload (JSONValue)
fields from gorm:"type:json" to gorm:"type:text" (JSONValue already implements
driver.Valuer/sql.Scanner so no other changes are needed). Ensure both Tags and
RawPayload use the new gorm tag so databases like SQLite explicitly store JSON
as TEXT.
In `@router/api-router.go`:
- Around line 140-175: The router references undefined symbols
middleware.SmsRateLimit, controller.ClientSmsLogin, and
controller.ClientGetSelf, breaking the build; either implement these missing
functions/middleware (exported functions matching those exact names with
appropriate signatures and registration in their packages) and wire them into
the routes, or remove the routes that use them (the POST "/client/login_sms"
route and the GET "/client/self" route) from api-router.go so the code no longer
references the undefined identifiers; ensure any dependent tests or docs are
updated accordingly.
- Around line 161-174: The POST route that records skill downloads
(clientRoute.POST("/skills/download/:id", controller.ClientRecordSkillDownload))
lacks endpoint-specific rate limiting; update that route to include
middleware.CriticalRateLimit() in its middleware chain so the route applies the
critical limiter (same as used on SubscriptionRequestEpay). Locate the
clientRoute.POST call for "skills/download/:id" and add
middleware.CriticalRateLimit() before the handler to prevent download-count
inflation.
In `@web/src/hooks/common/useSidebar.js`:
- Line 51: The mergeAdminConfig() flow is auto-enabling the new 'skill-market'
module because DEFAULT_ADMIN_CONFIG seeds missing keys as true; update the merge
logic or default to avoid exposing the module: either set 'skill-market': false
in DEFAULT_ADMIN_CONFIG, or modify mergeAdminConfig() to treat unknown/new
module keys by defaulting them to false (or require an explicit
migration/opt-in) so existing SidebarModulesAdmin configs do not gain access
automatically.
In `@web/src/pages/SkillMarket/Editor.jsx`:
- Around line 161-171: The previewUrl useMemo currently falls back to API
endpoints (previewUrl, editingSkill) which produce JSON, so either change the
fallback to the client-facing skill page (e.g., construct a URL like
`${window.location.origin}/market/skills/${editingSkill.id}` or your app's
public skill route) or return null/empty so the preview button (the code that
calls window.open(previewUrl, '_blank')) can be disabled when editingSkill.url
is not set; also tighten the useMemo dependency array to [editingSkill?.url,
editingSkill?.id] to avoid recomputing on every field change. Ensure the preview
button checks for a falsy previewUrl and disables/hides itself when previewUrl
is null.
- Around line 252-254: Remove the visible debug banner and the temporary
query-string marker used for the white-screen workaround: delete the div
rendering the build banner (the element with className 'mb-4 rounded-lg border
border-blue-200 bg-blue-50 px-3 py-2 text-xs text-blue-700' that displays
"build=skill-market-editor-plain-v2") and remove any code that appends or reads
the ts/query param value "editor-plain-v2" from the editor URL (the logic that
produces or checks "?ts=editor-plain-v2" around the query handling), ensuring no
debug build identifier is exposed in the UI or in navigation once the workaround
is no longer needed.
- Around line 249-489: The file contains hardcoded Chinese UI strings throughout
the Editor component (e.g. headings, labels, button text, placeholders and
messages such as '新增技能', '保存', '上架', '原始名称', '正在加载技能信息...', etc. used in JSX
returned by the component and in handlers like submitSkill, updateSkillStatus),
which violates the i18n guideline; fix by importing and using the
useTranslation() hook and replacing each visible literal with t('中文key') calls
(use the Chinese literal itself as the i18n key) for all strings rendered in the
Editor component (including values for labels, button text, placeholders,
pageError/editError messages and any text in conditional branches), ensuring
strings in related files index.jsx and EditSkillModal.jsx are converted
similarly and preserving existing variable names like editingSkill, submitSkill,
updateField, updateSkillStatus, pageError, editError, loading so you only change
the UI literals to t('...').
In `@web/src/pages/SkillMarket/EditSkillModal.jsx`:
- Around line 1-292: EditSkillModal is an orphaned SideSheet component that is
not used; delete the unused EditSkillModal component and its default export
(i.e., remove the file that defines EditSkillModal), remove any
imports/references to EditSkillModal across the codebase, and ensure the editing
route continues to use the full-page Editor component by keeping/importing
Editor where needed.
In `@web/src/pages/SkillMarket/index.jsx`:
- Around line 161-185: The code contains dead edit-flow state and helpers that
are no longer used by the navigation-based editor: remove the unused state
variables (editingSkill, editLoading, editError, editRequestSeqRef,
matchedEditSkill), the helper functions (loadSkillDetail, closeEditor,
submitSkill), the duplicated defaultSkillForm and any unused imports like
EditSkillModal.jsx, or alternatively wire them into the current navigation flow
(e.g., call loadSkillDetail from the edit route/component and use submitSkill in
the Editor.jsx submit handler). Update any references in this file so there are
no lingering unused symbols (loadSkillDetail, closeEditor, submitSkill,
defaultSkillForm, editingSkill, editLoading, editError, editRequestSeqRef,
matchedEditSkill) and remove related dead imports.
- Around line 440-489: The action column currently exposes two independent
controls for is_public (the Popconfirm+Button pair and the second "上架" Switch)
which can race; remove the duplicate Switch-based control and keep the
Popconfirm+Button flow for publishing while retaining the "启用" Switch for the
enabled flag. Concretely, delete the div block containing the span '上架' and the
Switch whose checked prop is record.is_public and onChange calls
updateSkillStatus(record, { is_public: checked }), leaving the Popconfirm/Button
pairs that call updateSkillStatus(record, { is_public: ... }) and the Switch for
checked={record.enabled}; ensure statusLoadingMap and updateSkillStatus usages
remain unchanged.
---
Outside diff comments:
In `@web/src/components/layout/SiderBar.jsx`:
- Around line 202-210: Remove the hardcoded admin bypass for the 'skill-market'
item in the filteredItems logic (the special-case item.itemKey ===
'skill-market' return isAdmin()) and instead let the normal visibility gate run
via isModuleVisible('admin', item.itemKey); ensure the default admin config in
useSidebar.js already contains 'skill-market': true so it shows by default for
admins, and keep the visibility decision centralized in SidebarModulesAdmin
(i.e., only use isModuleVisible('admin', ...) here rather than calling isAdmin()
to force visibility).
- Around line 487-498: The admin section rendering bypasses module-visibility
checks by using isAdmin() alone, so when all admin modules are disabled (via
SidebarModulesAdmin) the section still appears; restore the visibility guard by
adding the hasSectionVisibleModules('admin') check alongside isAdmin() before
rendering the admin block (the same pattern used for chat/console/personal),
i.e., ensure the conditional uses both isAdmin() &&
hasSectionVisibleModules('admin') so adminItems are only rendered when the user
is an admin and the 'admin' section has visible modules.
---
Nitpick comments:
In `@controller/client_skills.go`:
- Around line 254-273: AdminCreateClientSkill currently persists a
client-supplied downloads value via normalizeClientSkillInput, allowing negative
or arbitrary counters; fix by preventing arbitrary seeds on create: in
AdminCreateClientSkill (before calling normalizeClientSkillInput) set
req.Skill.Downloads = 0 (or validate and clamp to >=0), or alternatively change
normalizeClientSkillInput to ignore/override Downloads when called for creates
(ensure new items always have Downloads = 0), and keep AdminUpdateClientSkill as
the authorized path for adjustments; reference AdminCreateClientSkill,
normalizeClientSkillInput, AdminUpsertClientSkillRequest, skill.Downloads,
model.DB.Create, and AdminUpdateClientSkill when making the change.
In `@middleware/rate-limit.go`:
- Around line 93-101: The loopback IP bypass in the middleware currently uses
brittle string checks (clientIP == "127.0.0.1" || clientIP == "::1" ||
strings.HasPrefix(clientIP, "localhost")) which is incorrect and unsafe; update
the rate-limit handler that wraps limiter (and references rateLimitFactory and
c.ClientIP()) to either remove the bypass entirely or replace it with a robust
check using net.ParseIP(clientIP) and ip.IsLoopback() to detect loopback
addresses (covering IPv4 and IPv6), drop the strings.HasPrefix("localhost")
check, and add a short comment above the bypass explaining why genuine loopback
traffic is exempt from rate limiting if you choose to keep it.
In `@web/src/pages/SkillMarket/Editor.jsx`:
- Around line 313-335: The UI currently binds both the free-text input and the
preset select to the same controlled value (editingSkill.category) causing them
to fight; change the select to act as a helper picker rather than a second bound
control: keep the text <input> as the single source of truth for category
(controlled by editingSkill.category and updateField), remove the select's value
binding to editingSkill.category and onChange instead call
updateField('category', chosenValue) or populate the input only when a user
explicitly picks a preset (e.g., via onChange or a small “apply preset” action),
and ensure getMylclawNavLabel(editingSkill.category) remains driven by the input
value; reference symbols: editingSkill, updateField, getMylclawNavLabel,
CATEGORY_PRESETS, fieldClassName, labelClassName.
- Around line 5-91: The file duplicates module-level constants and helpers
(defaultSkillForm, CATEGORY_PRESETS, MYCLAW_NAV_LABELS, normalizeSkillForm,
parseTags, getMylclawNavLabel, plus fieldClassName/labelClassName) that also
exist in index.jsx; extract these into a new shared module (e.g.,
web/src/pages/SkillMarket/constants.js or utils.js), export each symbol, then
replace the in-file definitions in both Editor.jsx and index.jsx with imports of
those exported names; ensure normalizeSkillForm still uses defaultSkillForm and
parseTags remains available where used so behavior is preserved.
🪄 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: 774a2ea9-5787-43a7-875d-171911b33e39
📒 Files selected for processing (15)
controller/client_skillhub.gocontroller/client_skills.gomiddleware/auth.gomiddleware/cache.gomiddleware/rate-limit.gomodel/client_skill_market.gomodel/main.gorouter/api-router.goweb/src/App.jsxweb/src/components/layout/SiderBar.jsxweb/src/helpers/render.jsxweb/src/hooks/common/useSidebar.jsweb/src/pages/SkillMarket/EditSkillModal.jsxweb/src/pages/SkillMarket/Editor.jsxweb/src/pages/SkillMarket/index.jsx
| // 3. 把最终 zip 流直接回传给客户端 | ||
| // | ||
| // 后续如需“预缓存/镜像 zip”，可以在这里落盘缓存并优先命中本地文件。 | ||
| func ClientProxySkillHubDownload(c *gin.Context) { |
Contributor
There was a problem hiding this comment.
Add rate limiting to this public streaming proxy.
The route snippet registers this handler publicly, and the handler streams arbitrary SkillHub ZIP responses through your server. Please add middleware.DownloadRateLimit() or an equivalent limiter on the route to avoid bandwidth and upstream-request abuse.
Example route wiring:
clientRoute.GET("/skills/skillhub/download",
middleware.DownloadRateLimit(),
controller.ClientProxySkillHubDownload)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/client_skillhub.go` at line 25, The public streaming handler
ClientProxySkillHubDownload currently allows unlimited downloads; add rate
limiting by attaching middleware.DownloadRateLimit() (or equivalent
request/stream bandwidth limiter) to the route that registers
ClientProxySkillHubDownload so the route reads like:
clientRoute.GET("/skills/skillhub/download", middleware.DownloadRateLimit(),
controller.ClientProxySkillHubDownload); if you cannot change the route
registration, wrap the handler with the limiter inside the handler entrypoint
(or call a limiter middleware function at start of ClientProxySkillHubDownload)
to enforce per-client/download limits and prevent bandwidth abuse.
Comment on lines
+195
to
+218
| func ClientRecordSkillDownload(c *gin.Context) { | ||
| id, err := strconv.Atoi(strings.TrimSpace(c.Param("id"))) | ||
| if err != nil { | ||
| common.ApiErrorMsg(c, "无效的技能 ID") | ||
| return | ||
| } | ||
|
|
||
| if err = model.IncrementClientSkillMarketDownload(id); err == nil { | ||
| common.ApiSuccess(c, gin.H{"recorded": true}) | ||
| return | ||
| } | ||
| if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { | ||
| common.SysError("increment client public skill download failed: " + err.Error()) | ||
| } | ||
|
|
||
| for _, skill := range defaultClientPublicSkills { | ||
| if skill.ID == id { | ||
| common.ApiSuccess(c, gin.H{"recorded": true}) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| common.ApiErrorMsg(c, "技能不存在") | ||
| } |
Contributor
There was a problem hiding this comment.
Download increment never reports "skill not found" — always reports success.
DB.Model(...).Where(...).Update(...) returns nil even when RowsAffected == 0 (e.g., id doesn't exist, enabled=false, or is_public=false). So the err == nil branch on line 202 swallows all "no such skill" cases: the default-skills fallback on lines 210-215 is unreachable for any id that matches a disabled/non-public DB row, and ClientRecordSkillDownload responds 200 {"recorded": true} for every integer id even when nothing was actually incremented. This also means the endpoint cannot distinguish a real increment from a no-op — directly feeding incorrect telemetry.
🛠 Proposed fix
Expose RowsAffected from the model helper, then branch on it:
-func IncrementClientSkillMarketDownload(id int) error {
- return DB.Model(&ClientSkillMarketItem{}).
+func IncrementClientSkillMarketDownload(id int) (int64, error) {
+ result := DB.Model(&ClientSkillMarketItem{}).
Where("id = ? AND enabled = ? AND is_public = ?", id, true, true).
- Update("downloads", gorm.Expr("downloads + ?", 1)).Error
+ Update("downloads", gorm.Expr("downloads + ?", 1))
+ return result.RowsAffected, result.Error
}- if err = model.IncrementClientSkillMarketDownload(id); err == nil {
- common.ApiSuccess(c, gin.H{"recorded": true})
- return
- }
- if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
- common.SysError("increment client public skill download failed: " + err.Error())
- }
+ affected, err := model.IncrementClientSkillMarketDownload(id)
+ if err != nil {
+ common.SysError("increment client public skill download failed: " + err.Error())
+ }
+ if affected > 0 {
+ common.ApiSuccess(c, gin.H{"recorded": true})
+ return
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/client_skills.go` around lines 195 - 218, The current call to
model.IncrementClientSkillMarketDownload in ClientRecordSkillDownload treats a
nil error as a guaranteed increment; modify the model function
IncrementClientSkillMarketDownload to return (int64, error) where int64 is
RowsAffected (use db.RowsAffected after the Update), then update
ClientRecordSkillDownload to handle the returned rows: if err != nil log and
fall through, if rows > 0 return ApiSuccess, if rows == 0: check
defaultClientPublicSkills for a matching skill ID and return success if found,
otherwise return ApiErrorMsg "技能不存在". Ensure you update all call sites and the
model implementation to surface RowsAffected from the DB update.
Comment on lines
+275
to
+334
| func AdminUpdateClientSkill(c *gin.Context) { | ||
| id, err := strconv.Atoi(strings.TrimSpace(c.Param("id"))) | ||
| if err != nil || id <= 0 { | ||
| common.ApiErrorMsg(c, "无效的技能 ID") | ||
| return | ||
| } | ||
|
|
||
| var req AdminUpsertClientSkillRequest | ||
| if err := c.ShouldBindJSON(&req); err != nil { | ||
| common.ApiErrorMsg(c, "参数错误") | ||
| return | ||
| } | ||
|
|
||
| item, err := normalizeClientSkillInput(req.Skill) | ||
| if err != nil { | ||
| common.ApiErrorMsg(c, err.Error()) | ||
| return | ||
| } | ||
|
|
||
| updateMap := map[string]any{ | ||
| "name": item.Name, | ||
| "display_name": item.DisplayName, | ||
| "display_name_zh": item.DisplayNameZh, | ||
| "description": item.Description, | ||
| "description_zh": item.DescriptionZh, | ||
| "category": item.Category, | ||
| "tags": item.Tags, | ||
| "source": item.Source, | ||
| "source_platform": item.SourcePlatform, | ||
| "source_skill_id": item.SourceSkillID, | ||
| "source_slug": item.SourceSlug, | ||
| "source_updated_at": item.SourceUpdatedAt, | ||
| "url": item.URL, | ||
| "download_url": item.DownloadURL, | ||
| "author": item.Author, | ||
| "version": item.Version, | ||
| "downloads": item.Downloads, | ||
| "enabled": item.Enabled, | ||
| "is_public": item.IsPublic, | ||
| "sort_order": item.SortOrder, | ||
| "updated_time": common.GetTimestamp(), | ||
| } | ||
|
|
||
| result := model.DB.Model(&model.ClientSkillMarketItem{}).Where("id = ?", id).Updates(updateMap) | ||
| if result.Error != nil { | ||
| common.ApiError(c, result.Error) | ||
| return | ||
| } | ||
| if result.RowsAffected == 0 { | ||
| common.ApiErrorMsg(c, "技能不存在") | ||
| return | ||
| } | ||
|
|
||
| updated, err := model.GetClientSkillMarketItemByID(id) | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
| common.ApiSuccess(c, toClientSkill(updated)) | ||
| } |
Contributor
There was a problem hiding this comment.
AdminUpdateClientSkill writes downloads unconditionally — admin edits will clobber live counters.
The admin editor UI (Editor.jsx / index.jsx) loads the current downloads into a form field and sends it back on save. Between load and save, ClientRecordSkillDownload may have bumped the counter on the server, but the admin's PUT will overwrite it with the stale value. Either drop downloads from the updateMap (and have a dedicated counter-reset endpoint), or read-modify-write within a transaction that re-checks the persisted counter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/client_skills.go` around lines 275 - 334, AdminUpdateClientSkill
currently unconditionally writes the "downloads" field from the request which
can clobber concurrent increments performed by ClientRecordSkillDownload; remove
"downloads" from the updateMap in AdminUpdateClientSkill so admin saves don't
overwrite live counters, and instead implement a dedicated endpoint (e.g.,
AdminResetClientSkillDownloads or similar) to explicitly set/reset the counter
if needed; alternatively, if a mutation via admin must adjust the counter,
perform a read-modify-write inside a DB transaction in AdminUpdateClientSkill
that reads the current downloads, computes the intended new value
deterministically, and writes that value using the transaction to avoid lost
updates.
Comment on lines
+249
to
+489
| return ( | ||
| <div className='mt-[60px] px-2'> | ||
| <div className='rounded-2xl border border-[#e5e6eb] bg-white p-6 shadow-sm'> | ||
| <div className='mb-4 rounded-lg border border-blue-200 bg-blue-50 px-3 py-2 text-xs text-blue-700'> | ||
| build=skill-market-editor-plain-v2 | ||
| </div> | ||
|
|
||
| <div className='mb-6 flex flex-col gap-3 md:flex-row md:items-center md:justify-between'> | ||
| <div> | ||
| <h1 className='text-2xl font-semibold text-[#1f2329]'> | ||
| {isCreate ? '新增技能' : `设置技能 #${editingSkill.id || skillId}`} | ||
| </h1> | ||
| <p className='mt-1 text-sm text-[#86909c]'>纯页面表单版，专门用于绕开弹层白屏问题</p> | ||
| </div> | ||
| <div className='flex flex-wrap gap-2'> | ||
| {!isCreate && editingSkill?.id ? ( | ||
| <button | ||
| className='rounded-lg border border-[#d9d9d9] bg-white px-4 py-2 text-sm' | ||
| onClick={() => window.open(previewUrl, '_blank', 'noopener,noreferrer')} | ||
| > | ||
| 预览 | ||
| </button> | ||
| ) : null} | ||
| <button | ||
| className='rounded-lg border border-[#d9d9d9] bg-white px-4 py-2 text-sm' | ||
| onClick={() => navigate('/console/skill-market')} | ||
| > | ||
| 返回技能管理 | ||
| </button> | ||
| <button | ||
| className='rounded-lg bg-[#155eef] px-4 py-2 text-sm text-white disabled:opacity-60' | ||
| disabled={saving} | ||
| onClick={() => void submitSkill()} | ||
| > | ||
| {saving ? '保存中...' : '保存'} | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
| {pageError ? ( | ||
| <div className='mb-4 rounded-lg border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-600'> | ||
| 页面错误：{pageError} | ||
| </div> | ||
| ) : null} | ||
|
|
||
| {editError ? ( | ||
| <div className='mb-4 rounded-lg border border-amber-200 bg-amber-50 px-3 py-2 text-sm text-amber-700'> | ||
| 接口提示：{editError} | ||
| </div> | ||
| ) : null} | ||
|
|
||
| {loading ? ( | ||
| <div className='py-12 text-center text-sm text-[#86909c]'>正在加载技能信息...</div> | ||
| ) : ( | ||
| <div className='space-y-5'> | ||
| <div className='grid grid-cols-1 gap-4 md:grid-cols-2'> | ||
| <label className={labelClassName}> | ||
| 原始名称 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.name} | ||
| onChange={(event) => updateField('name', event.target.value)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 分类 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.category} | ||
| onChange={(event) => updateField('category', event.target.value)} | ||
| /> | ||
| <select | ||
| className={fieldClassName} | ||
| value={editingSkill.category || ''} | ||
| onChange={(event) => updateField('category', event.target.value)} | ||
| > | ||
| <option value=''>请选择分类</option> | ||
| {CATEGORY_PRESETS.map((item) => ( | ||
| <option key={item.value} value={item.value}> | ||
| {item.label} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| <span className='mt-1 block text-xs text-[#86909c]'> | ||
| 当前会显示到 myclaw 导航：{getMylclawNavLabel(editingSkill.category)} | ||
| </span> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 展示名 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.display_name} | ||
| onChange={(event) => updateField('display_name', event.target.value)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 中文别名 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.display_name_zh} | ||
| onChange={(event) => updateField('display_name_zh', event.target.value)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 作者 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.author} | ||
| onChange={(event) => updateField('author', event.target.value)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 版本 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.version} | ||
| onChange={(event) => updateField('version', event.target.value)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 来源平台 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.source_platform} | ||
| onChange={(event) => updateField('source_platform', event.target.value)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 来源 slug | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.source_slug} | ||
| onChange={(event) => updateField('source_slug', event.target.value)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 排序 | ||
| <input | ||
| className={fieldClassName} | ||
| type='number' | ||
| value={editingSkill.sort_order} | ||
| onChange={(event) => updateField('sort_order', Number(event.target.value) || 0)} | ||
| /> | ||
| </label> | ||
| <label className={labelClassName}> | ||
| 下载量 | ||
| <input | ||
| className={fieldClassName} | ||
| type='number' | ||
| value={editingSkill.downloads} | ||
| onChange={(event) => updateField('downloads', Number(event.target.value) || 0)} | ||
| /> | ||
| </label> | ||
| </div> | ||
|
|
||
| <label className={labelClassName}> | ||
| 标签 | ||
| <input | ||
| className={fieldClassName} | ||
| placeholder='多个标签用英文逗号分隔' | ||
| value={editingSkill.tags_text} | ||
| onChange={(event) => updateField('tags_text', event.target.value)} | ||
| /> | ||
| </label> | ||
|
|
||
| <label className={labelClassName}> | ||
| 详情地址 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.url} | ||
| onChange={(event) => updateField('url', event.target.value)} | ||
| /> | ||
| </label> | ||
|
|
||
| <label className={labelClassName}> | ||
| 下载地址 | ||
| <input | ||
| className={fieldClassName} | ||
| value={editingSkill.download_url} | ||
| onChange={(event) => updateField('download_url', event.target.value)} | ||
| /> | ||
| </label> | ||
|
|
||
| <label className={labelClassName}> | ||
| 描述 | ||
| <textarea | ||
| className={`${fieldClassName} min-h-[120px]`} | ||
| value={editingSkill.description} | ||
| onChange={(event) => updateField('description', event.target.value)} | ||
| /> | ||
| </label> | ||
|
|
||
| <label className={labelClassName}> | ||
| 中文描述 | ||
| <textarea | ||
| className={`${fieldClassName} min-h-[120px]`} | ||
| value={editingSkill.description_zh} | ||
| onChange={(event) => updateField('description_zh', event.target.value)} | ||
| /> | ||
| </label> | ||
|
|
||
| <div className='flex flex-wrap gap-6'> | ||
| <label className='flex items-center gap-2 text-sm text-[#1f2329]'> | ||
| <input | ||
| type='checkbox' | ||
| checked={Boolean(editingSkill.enabled)} | ||
| onChange={(event) => updateField('enabled', event.target.checked)} | ||
| /> | ||
| 启用 | ||
| </label> | ||
| <label className='flex items-center gap-2 text-sm text-[#1f2329]'> | ||
| <input | ||
| type='checkbox' | ||
| checked={Boolean(editingSkill.is_public)} | ||
| onChange={(event) => updateField('is_public', event.target.checked)} | ||
| /> | ||
| 公开上架 | ||
| </label> | ||
| {editingSkill.id ? ( | ||
| <> | ||
| <button | ||
| className='rounded-lg border border-[#155eef] px-4 py-2 text-sm text-[#155eef] disabled:opacity-60' | ||
| disabled={editingSkill.is_public || statusLoading} | ||
| onClick={() => void updateSkillStatus({ is_public: true })} | ||
| > | ||
| {statusLoading ? '处理中...' : '上架'} | ||
| </button> | ||
| <button | ||
| className='rounded-lg border border-[#f04438] px-4 py-2 text-sm text-[#f04438] disabled:opacity-60' | ||
| disabled={!editingSkill.is_public || statusLoading} | ||
| onClick={() => void updateSkillStatus({ is_public: false })} | ||
| > | ||
| {statusLoading ? '处理中...' : '下架'} | ||
| </button> | ||
| </> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Hardcoded Chinese UI strings bypass i18n.
Every visible label, button text, placeholder, success/error message, and heading in this file is a raw Chinese literal (e.g. '新增技能', '保存', '上架', '原始名称不能为空', '正在加载技能信息...'). The repo's i18n guideline requires useTranslation() + t('中文key') for all user-facing strings in web/src/**/*.{ts,tsx,js,jsx}, with the Chinese string as the key. The same applies to index.jsx and EditSkillModal.jsx.
As per coding guidelines: "Use useTranslation() hook and call t('中文key') in components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/SkillMarket/Editor.jsx` around lines 249 - 489, The file
contains hardcoded Chinese UI strings throughout the Editor component (e.g.
headings, labels, button text, placeholders and messages such as '新增技能', '保存',
'上架', '原始名称', '正在加载技能信息...', etc. used in JSX returned by the component and in
handlers like submitSkill, updateSkillStatus), which violates the i18n
guideline; fix by importing and using the useTranslation() hook and replacing
each visible literal with t('中文key') calls (use the Chinese literal itself as
the i18n key) for all strings rendered in the Editor component (including values
for labels, button text, placeholders, pageError/editError messages and any text
in conditional branches), ensuring strings in related files index.jsx and
EditSkillModal.jsx are converted similarly and preserving existing variable
names like editingSkill, submitSkill, updateField, updateSkillStatus, pageError,
editError, loading so you only change the UI literals to t('...').
Comment on lines
+252
to
+254
| <div className='mb-4 rounded-lg border border-blue-200 bg-blue-50 px-3 py-2 text-xs text-blue-700'> | ||
| build=skill-market-editor-plain-v2 | ||
| </div> |
Contributor
There was a problem hiding this comment.
Remove debug build banner before release.
build=skill-market-editor-plain-v2 is a visible debug artifact that leaks internal build/iteration naming to admins and also appears in the navigated URL query (?ts=editor-plain-v2 on line 219). Strip the banner and the query-string marker once the "white-screen" workaround is no longer needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/SkillMarket/Editor.jsx` around lines 252 - 254, Remove the
visible debug banner and the temporary query-string marker used for the
white-screen workaround: delete the div rendering the build banner (the element
with className 'mb-4 rounded-lg border border-blue-200 bg-blue-50 px-3 py-2
text-xs text-blue-700' that displays "build=skill-market-editor-plain-v2") and
remove any code that appends or reads the ts/query param value "editor-plain-v2"
from the editor URL (the logic that produces or checks "?ts=editor-plain-v2"
around the query handling), ensuring no debug build identifier is exposed in the
UI or in navigation once the workaround is no longer needed.
Comment on lines
+1
to
+292
| import React from 'react'; | ||
| import { | ||
| Button, | ||
| Input, | ||
| InputNumber, | ||
| SideSheet, | ||
| Space, | ||
| Switch, | ||
| Tag, | ||
| Typography, | ||
| } from '@douyinfe/semi-ui'; | ||
|
|
||
| const EditSkillModal = ({ | ||
| visible, | ||
| editingSkill, | ||
| setEditingSkill, | ||
| editLoading, | ||
| editError, | ||
| saving, | ||
| onCancel, | ||
| onSubmit, | ||
| statusLoadingMap, | ||
| updateSkillStatus, | ||
| getPreviewUrl, | ||
| matchedEditSkill, | ||
| skillsCount, | ||
| categoryPresets, | ||
| getMylclawNavLabel, | ||
| }) => ( | ||
| <SideSheet | ||
| placement='right' | ||
| visible={visible} | ||
| width={720} | ||
| closeIcon={null} | ||
| onCancel={onCancel} | ||
| title={ | ||
| <Space> | ||
| <Tag color={editingSkill?.id ? 'blue' : 'green'} shape='circle'> | ||
| {editingSkill?.id ? '编辑' : '新增'} | ||
| </Tag> | ||
| <Typography.Title heading={4} style={{ margin: 0 }}> | ||
| {editingSkill?.id ? `设置技能 #${editingSkill.id}` : '新增技能'} | ||
| </Typography.Title> | ||
| </Space> | ||
| } | ||
| footer={ | ||
| <div className='flex justify-end bg-white'> | ||
| <Space> | ||
| <Button theme='solid' loading={saving} onClick={onSubmit}> | ||
| 保存 | ||
| </Button> | ||
| <Button theme='light' type='primary' onClick={onCancel}> | ||
| 取消 | ||
| </Button> | ||
| </Space> | ||
| </div> | ||
| } | ||
| bodyStyle={{ padding: 0 }} | ||
| > | ||
| <div className='space-y-4 p-4'> | ||
| <div className='rounded-lg border border-amber-300 bg-amber-50 px-3 py-2 text-xs text-amber-800'> | ||
| <div>调试面板（模型管理同款 SideSheet 版）</div> | ||
| <div> | ||
| {`visible=${visible ? 'yes' : 'no'} | listCount=${skillsCount} | matched=${ | ||
| matchedEditSkill ? 'yes' : 'no' | ||
| } | formId=${editingSkill?.id ?? ''} | formName=${editingSkill?.name || ''}`} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className='rounded-lg border border-dashed border-[var(--semi-color-border)] bg-[var(--semi-color-bg-0)] px-3 py-2 text-xs text-gray-500'> | ||
| build=skill-market-sidesheet-v1 | ||
| </div> | ||
|
|
||
| {editError ? ( | ||
| <div className='rounded-lg border border-red-200 bg-red-50 px-3 py-2 text-xs text-red-600'> | ||
| 当前展示的是列表缓存数据：{editError} | ||
| </div> | ||
| ) : null} | ||
|
|
||
| {editLoading ? ( | ||
| <div className='py-8 text-center text-sm text-gray-500'>正在加载技能详情...</div> | ||
| ) : null} | ||
|
|
||
| <div className='grid grid-cols-1 gap-4 md:grid-cols-2'> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>原始名称</div> | ||
| <Input | ||
| value={editingSkill.name} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, name: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>分类</div> | ||
| <Input | ||
| value={editingSkill.category} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, category: value }))} | ||
| /> | ||
| <div className='mt-2'> | ||
| <div className='mb-1 text-xs text-gray-500'>导航分类快捷选择</div> | ||
| <select | ||
| className='w-full rounded-md border border-[var(--semi-color-border)] bg-[var(--semi-color-bg-0)] px-3 py-2 text-sm' | ||
| value={editingSkill.category || ''} | ||
| disabled={editLoading} | ||
| onChange={(event) => | ||
| setEditingSkill((prev) => ({ ...prev, category: event.target.value })) | ||
| } | ||
| > | ||
| <option value=''>请选择分类</option> | ||
| {categoryPresets.map((item) => ( | ||
| <option key={item.value} value={item.value}> | ||
| {item.label} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| <div className='mt-2 text-xs text-gray-500'> | ||
| 当前会显示到 myclaw 导航：{getMylclawNavLabel(editingSkill.category)} | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>展示名</div> | ||
| <Input | ||
| value={editingSkill.display_name} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, display_name: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>中文别名</div> | ||
| <Input | ||
| value={editingSkill.display_name_zh} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, display_name_zh: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>作者</div> | ||
| <Input | ||
| value={editingSkill.author} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, author: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>版本</div> | ||
| <Input | ||
| value={editingSkill.version} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, version: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>来源平台</div> | ||
| <Input | ||
| value={editingSkill.source_platform} | ||
| disabled={editLoading} | ||
| onChange={(value) => | ||
| setEditingSkill((prev) => ({ ...prev, source_platform: value })) | ||
| } | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>来源 slug</div> | ||
| <Input | ||
| value={editingSkill.source_slug} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, source_slug: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>排序</div> | ||
| <InputNumber | ||
| value={editingSkill.sort_order} | ||
| disabled={editLoading} | ||
| onChange={(value) => | ||
| setEditingSkill((prev) => ({ ...prev, sort_order: Number(value) || 0 })) | ||
| } | ||
| style={{ width: '100%' }} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>下载量</div> | ||
| <InputNumber | ||
| value={editingSkill.downloads} | ||
| disabled={editLoading} | ||
| onChange={(value) => | ||
| setEditingSkill((prev) => ({ ...prev, downloads: Number(value) || 0 })) | ||
| } | ||
| style={{ width: '100%' }} | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>标签</div> | ||
| <Input | ||
| placeholder='多个标签用英文逗号分隔' | ||
| value={editingSkill.tags_text} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, tags_text: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>详情地址</div> | ||
| <Input | ||
| value={editingSkill.url} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, url: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>下载地址</div> | ||
| <Input | ||
| value={editingSkill.download_url} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, download_url: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>描述</div> | ||
| <Input.TextArea | ||
| rows={4} | ||
| value={editingSkill.description} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, description: value }))} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <div className='mb-2 text-sm font-medium'>中文描述</div> | ||
| <Input.TextArea | ||
| rows={4} | ||
| value={editingSkill.description_zh} | ||
| disabled={editLoading} | ||
| onChange={(value) => setEditingSkill((prev) => ({ ...prev, description_zh: value }))} | ||
| /> | ||
| </div> | ||
|
|
||
| <div className='mt-4 flex flex-wrap gap-6'> | ||
| <div className='flex items-center gap-2 rounded-lg border px-3 py-2'> | ||
| <span>启用</span> | ||
| <Switch | ||
| checked={editingSkill.enabled} | ||
| disabled={editLoading} | ||
| onChange={(checked) => setEditingSkill((prev) => ({ ...prev, enabled: checked }))} | ||
| /> | ||
| </div> | ||
| <div className='flex items-center gap-2 rounded-lg border px-3 py-2'> | ||
| <span>公开上架</span> | ||
| <Switch | ||
| checked={editingSkill.is_public} | ||
| disabled={editLoading} | ||
| onChange={(checked) => setEditingSkill((prev) => ({ ...prev, is_public: checked }))} | ||
| /> | ||
| </div> | ||
| {editingSkill.id ? ( | ||
| <Button | ||
| theme='light' | ||
| onClick={() => window.open(getPreviewUrl(editingSkill), '_blank', 'noopener,noreferrer')} | ||
| > | ||
| 预览技能页 | ||
| </Button> | ||
| ) : null} | ||
| {editingSkill.id ? ( | ||
| <Button | ||
| type='primary' | ||
| theme='light' | ||
| loading={Boolean(statusLoadingMap[String(editingSkill.id)])} | ||
| disabled={editingSkill.is_public} | ||
| onClick={() => void updateSkillStatus(editingSkill, { is_public: true })} | ||
| > | ||
| 立即上架 | ||
| </Button> | ||
| ) : null} | ||
| {editingSkill.id ? ( | ||
| <Button | ||
| type='danger' | ||
| theme='light' | ||
| loading={Boolean(statusLoadingMap[String(editingSkill.id)])} | ||
| disabled={!editingSkill.is_public} | ||
| onClick={() => void updateSkillStatus(editingSkill, { is_public: false })} | ||
| > | ||
| 立即下架 | ||
| </Button> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| </SideSheet> | ||
| ); | ||
|
|
||
| export default EditSkillModal; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any consumer of EditSkillModal across the web/ tree
rg -nP "from\s+['\"].*EditSkillModal" --type-add 'web:*.{js,jsx,ts,tsx}' -tweb
rg -nP "import\s+.*EditSkillModal" --type-add 'web:*.{js,jsx,ts,tsx}' -twebRepository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Find the EditSkillModal file
find web/ -name "*EditSkillModal*" -type f
# Check what's in index.jsx and Editor.jsx in the same directory
fd -type f "index.jsx|Editor.jsx" web/src/pages/SkillMarket/Repository: QuantumNous/new-api
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Check the content of index.jsx and Editor.jsx
find web/src/pages/SkillMarket -name "index.jsx" -o -name "Editor.jsx" | head -5 | while read file; do
echo "=== File: $file ==="
cat -n "$file"
echo ""
doneRepository: QuantumNous/new-api
Length of output: 45310
🏁 Script executed:
#!/bin/bash
# Search for any dynamic imports or lazy loading patterns that might reference EditSkillModal
rg -i "editskillmodal|lazy|dynamic" web/src/pages/SkillMarket --type-add 'web:*.{js,jsx,ts,tsx}' -tweb -C 2Repository: QuantumNous/new-api
Length of output: 542
Delete the orphaned EditSkillModal component or integrate it into the editing workflow.
EditSkillModal is unused and can be safely deleted. The codebase routes editing to /console/skill-market/edit/:skillId, which loads Editor.jsx (a full-page form) rather than importing this SideSheet modal. The debug panel (lines 61–68) and build=skill-market-sidesheet-v1 marker confirm this was an intermediate iteration that was superseded by the plain form editor approach in Editor.jsx.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/SkillMarket/EditSkillModal.jsx` around lines 1 - 292,
EditSkillModal is an orphaned SideSheet component that is not used; delete the
unused EditSkillModal component and its default export (i.e., remove the file
that defines EditSkillModal), remove any imports/references to EditSkillModal
across the codebase, and ensure the editing route continues to use the full-page
Editor component by keeping/importing Editor where needed.
Comment on lines
+161
to
+185
| const loadSkillDetail = useCallback(async (id) => { | ||
| setEditLoading(true); | ||
| setEditError(''); | ||
| try { | ||
| if (id === 'new' || !id) { | ||
| setEditingSkill(defaultSkillForm); | ||
| return; | ||
| } | ||
|
|
||
| const fallback = skills.find((skill) => String(skill.id) === String(id)); | ||
| if (fallback) { | ||
| setEditingSkill(normalizeSkillForm(fallback)); | ||
| setEditError(''); | ||
| } else { | ||
| setEditingSkill(defaultSkillForm); | ||
| setEditError('当前技能未在列表中找到，请先点击“刷新”后再编辑'); | ||
| } | ||
| } catch (error) { | ||
| const errorMsg = error.message || '读取编辑数据失败'; | ||
| setEditError(errorMsg); | ||
| showError(errorMsg); | ||
| } finally { | ||
| setEditLoading(false); | ||
| } | ||
| }, [skills]); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
loadSkillDetail is dead code.
Nothing in this file invokes loadSkillDetail, closeEditor, or references editLoading/matchedEditSkill in render — the edit flow is navigation-based (openEditEditor → /console/skill-market/edit/:id). Remove the unused state (editingSkill, editLoading, editError, editRequestSeqRef, matchedEditSkill) and the helpers (loadSkillDetail, closeEditor, submitSkill), or wire them into an actual consumer. As written, ~80 lines never execute.
Similarly, submitSkill (lines 291–339), EditSkillModal.jsx import patterns it implies, and the duplicated defaultSkillForm exist only to support a modal flow that was replaced by the Editor.jsx page.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/SkillMarket/index.jsx` around lines 161 - 185, The code
contains dead edit-flow state and helpers that are no longer used by the
navigation-based editor: remove the unused state variables (editingSkill,
editLoading, editError, editRequestSeqRef, matchedEditSkill), the helper
functions (loadSkillDetail, closeEditor, submitSkill), the duplicated
defaultSkillForm and any unused imports like EditSkillModal.jsx, or
alternatively wire them into the current navigation flow (e.g., call
loadSkillDetail from the edit route/component and use submitSkill in the
Editor.jsx submit handler). Update any references in this file so there are no
lingering unused symbols (loadSkillDetail, closeEditor, submitSkill,
defaultSkillForm, editingSkill, editLoading, editError, editRequestSeqRef,
matchedEditSkill) and remove related dead imports.
Comment on lines
+440
to
+489
| <Popconfirm | ||
| title='确认上架该技能？' | ||
| content='上架后会在 myclaw 技能商店可见' | ||
| onConfirm={() => void updateSkillStatus(record, { is_public: true })} | ||
| disabled={record.is_public} | ||
| > | ||
| <Button | ||
| size='small' | ||
| type='primary' | ||
| theme='light' | ||
| disabled={record.is_public} | ||
| loading={Boolean(statusLoadingMap[String(record.id)])} | ||
| > | ||
| 上架 | ||
| </Button> | ||
| </Popconfirm> | ||
| <Popconfirm | ||
| title='确认下架该技能？' | ||
| content='下架后不会在 myclaw 技能商店展示' | ||
| onConfirm={() => void updateSkillStatus(record, { is_public: false })} | ||
| disabled={!record.is_public} | ||
| > | ||
| <Button | ||
| size='small' | ||
| type='danger' | ||
| theme='light' | ||
| disabled={!record.is_public} | ||
| loading={Boolean(statusLoadingMap[String(record.id)])} | ||
| > | ||
| 下架 | ||
| </Button> | ||
| </Popconfirm> | ||
| <div className='flex items-center gap-2'> | ||
| <span className='text-xs text-gray-500'>启用</span> | ||
| <Switch | ||
| size='small' | ||
| checked={record.enabled} | ||
| disabled={Boolean(statusLoadingMap[String(record.id)])} | ||
| onChange={(checked) => void updateSkillStatus(record, { enabled: checked })} | ||
| /> | ||
| </div> | ||
| <div className='flex items-center gap-2'> | ||
| <span className='text-xs text-gray-500'>上架</span> | ||
| <Switch | ||
| size='small' | ||
| checked={record.is_public} | ||
| disabled={Boolean(statusLoadingMap[String(record.id)])} | ||
| onChange={(checked) => void updateSkillStatus(record, { is_public: checked })} | ||
| /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Action column renders three different status controls side-by-side.
Each row now shows "上架" (Popconfirm + Button), "下架" (Popconfirm + Button), "启用" <Switch>, and "上架" <Switch> — both buttons and switches toggle is_public, so admins have two independent paths to the same state that can race against each other (the button uses a confirm dialog, the switch does not). Pick one UX (either the confirm-buttons or the switches) and remove the other to avoid confusing double controls and accidental one-click publishing via the switch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/SkillMarket/index.jsx` around lines 440 - 489, The action
column currently exposes two independent controls for is_public (the
Popconfirm+Button pair and the second "上架" Switch) which can race; remove the
duplicate Switch-based control and keep the Popconfirm+Button flow for
publishing while retaining the "启用" Switch for the enabled flag. Concretely,
delete the div block containing the span '上架' and the Switch whose checked prop
is record.is_public and onChange calls updateSkillStatus(record, { is_public:
checked }), leaving the Popconfirm/Button pairs that call
updateSkillStatus(record, { is_public: ... }) and the Switch for
checked={record.enabled}; ensure statusLoadingMap and updateSkillStatus usages
remain unchanged.
Calcium-Ion
force-pushed
the
main
branch
2 times, most recently
from
August 30, 2026 15:03
51fdfc5 to
2b6f1df
Compare
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
1 participant
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.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Improvements