feat(web): polish skill discovery cards and harden namespace lookups#450
feat(web): polish skill discovery cards and harden namespace lookups#450paradoxgacsd wants to merge 2 commits into
Conversation
Expose labels in skill summaries, support label attachment during publish, and refresh the public discovery UI. Tested: pnpm exec tsc --noEmit; pnpm run build; git diff --check; scanned staged diff and tracked config files for secrets.
Namespace portal operations now fail fast with DomainBadRequestException when slug lookup misses, and local dev port config is aligned on 8082. Constraint: Existing namespace APIs expose domain errors through the global exception layer Rejected: Allowing null namespace values to flow into member operations | it preserves ambiguous 500-class failures Confidence: medium Scope-risk: moderate Directive: Keep namespace slug lookups guarded before passing namespace IDs to domain services Tested: git diff --check Not-tested: Targeted Maven tests could not run because this shell has Java 1.8 and no javac; project requires Java 21 Co-authored-by: OmX <omx@oh-my-codex.dev>
There was a problem hiding this comment.
Code Review
This pull request introduces support for skill labels and summary overrides during the publishing process, alongside a significant UI overhaul of the landing page and improved error handling for namespace operations. The backend was refactored to use a new application service for publishing, and the local development port was updated to 8082. Feedback from the review highlights the need for more secure audit logging by using validated data and safe JSON serialization. Additionally, the label mapping logic should be made more robust by handling potential duplicate keys during map construction to avoid runtime exceptions.
| for (String labelSlug : labelSlugs) { | ||
| recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + labelSlug + "\"}"); | ||
| } | ||
| return toDtos(attachedLabels); |
There was a problem hiding this comment.
此处审计日志记录逻辑存在以下问题:
- 记录了未经验证的输入:循环遍历的是原始输入的 labelSlugs,这可能包含重复项、无效项或最终因权限等原因未实际绑定的标签。建议在调用 skillLabelService.attachLabels 成功后,遍历返回的已绑定标签列表进行记录,以确保审计日志的准确性。
- JSON 构造不安全:手动拼接 JSON 字符串("{"labelSlug":"" + labelSlug + ""}")在处理包含特殊字符(如引号、反斜杠)的输入时会生成非法的 JSON。建议使用可靠的 JSON 序列化方式或确保输入已通过严格校验。
List<SkillLabelDto> attachedDtos = toDtos(attachedLabels);
for (SkillLabelDto dto : attachedDtos) {
recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + dto.slug() + "\"}");
}
return attachedDtos;| Map<String, LabelDefinition> definitionsBySlug = labelDefinitionRepository.findBySlugIn(normalizedSlugs) | ||
| .stream() | ||
| .collect(Collectors.toMap( | ||
| definition -> definition.getSlug().toLowerCase(java.util.Locale.ROOT), | ||
| Function.identity() | ||
| )); |
There was a problem hiding this comment.
在使用 Collectors.toMap 构造 definitionsBySlug 时,未提供合并函数(merge function)。如果数据库中存在多个 slug 仅在大小写上有所不同的标签定义(且数据库配置为区分大小写),definition.getSlug().toLowerCase() 将产生冲突,导致抛出 IllegalStateException 并使整个操作失败。建议添加合并函数以提高鲁棒性,例如:(existing, replacement) -> existing。
Map<String, LabelDefinition> definitionsBySlug = labelDefinitionRepository.findBySlugIn(normalizedSlugs)
.stream()
.collect(Collectors.toMap(
definition -> definition.getSlug().toLowerCase(java.util.Locale.ROOT),
Function.identity(),
(existing, replacement) -> existing
));
背景
发现页技能卡片的可读性、标签层级、空态与命名空间缺失场景需要打磨;同时后端在传入空 namespace slug 时会抛出 500,需要做防御。
改动概要
Web 端
features/skill/skill-card.tsx:重新设计技能卡片视觉层级(标题、命名空间、版本、统计、标签、CTA 间距),优化 hover/focus 态。pages/landing.tsx+shared/components/landing-quick-start.tsx:发现页 quick-start 与 hero 区块整理,提升首屏信息密度。features/skill标签筛选改进:标签来源于LabelDefinition,在 hooks 层做去重与排序。index.css/app/layout.tsx:抽出复用样式,移除内联硬编码。api/types.ts里补回因为后端 schema 变化丢失的字段。Server 端
domain/label/SkillLabelService+ 相关仓储:标签与定义解耦,避免重复创建;新增SkillLabelServiceTest覆盖创建/合并路径。domain/skill/service/SkillPublishService:在解析 namespace slug 前做空值与格式校验,缺失时返回 400 而不是 500(修复8b1fd724)。SkillLabelAppServiceTest/SkillPublishAppServiceTest覆盖应用层路径。测试
验证
./mvnw -pl skillhub-app -am test编译并跑通新增单测。pnpm test跑通既有测试。Notes
b771c525与8b1fd724),其它已通过合入主线的 PR 同步过来,不会产生冲突。改动.md),多为前端样式和类型整理。Checklist