diff --git a/README.md b/README.md index 8f0e4860..d47ef5cc 100644 --- a/README.md +++ b/README.md @@ -119,10 +119,6 @@ skillhub login --token sk_xxx --registry https://skill.xfyun.cn skillhub search pdf skillhub install pdf-parser --agent codex -# Verify the bundled example skill after a fresh deployment -skillhub search skillhub-hello -skillhub install skillhub-hello --agent codex - # List installed skills skillhub list ``` @@ -290,7 +286,6 @@ Recommended production baseline: - keep PostgreSQL / Redis bound to `127.0.0.1` - use external S3 / OSS via `SKILLHUB_STORAGE_S3_*` - change `BOOTSTRAP_ADMIN_PASSWORD` to a strong password (`validate-release-config.sh` rejects the default `ChangeMe!2026`) -- set `SKILLHUB_BUILTIN_SKILLS_ENABLED=false` if you do not want the bundled `skillhub-hello` skill initialized in `@global` - rotate or disable the bootstrap admin after initial setup - run `make validate-release-config` before `docker compose up -d` @@ -426,8 +421,6 @@ clawhub login --token YOUR_API_TOKEN npx clawhub search email npx clawhub install my-skill npx clawhub install my-namespace--my-skill -npx clawhub search skillhub-hello -npx clawhub install skillhub-hello # Publish to global namespace npx clawhub publish ./my-skill --slug my-skill --version 1.0.0 diff --git a/cli/src/commands/install.ts b/cli/src/commands/install.ts index e9937a7b..0feed791 100644 --- a/cli/src/commands/install.ts +++ b/cli/src/commands/install.ts @@ -5,6 +5,7 @@ import { installSkill } from '../services/install-service' import { resolveInstallTargets } from '../agents/resolver' import { CliError } from '../shared/errors' import { EXIT } from '../shared/constants' +import { parseSkillName } from '../shared/skill-name-parser' export interface InstallCommandOptions { namespace?: string | undefined @@ -74,7 +75,7 @@ async function defaultPromptScope(): Promise<'user' | 'project'> { } export async function installCommand( - slug: string, + skillNameArg: string, options: InstallCommandOptions, deps: InstallCommandDeps = {} ): Promise { @@ -92,7 +93,10 @@ export async function installCommand( const credentialsStore = new CredentialsStore() const registry = resolveRegistry(options, process.env, await configStore.read()) const token = resolveToken(options, process.env, await credentialsStore.getToken(registry)) - const namespace = options.namespace ?? 'global' + + const parsed = parseSkillName(skillNameArg) + const namespace = options.namespace ?? parsed.namespace + const slug = parsed.slug const resolveTargets = deps.resolveInstallTargets ?? resolveInstallTargets const targets = await resolveTargets({ diff --git a/cli/src/commands/remove.ts b/cli/src/commands/remove.ts index 67f47f1d..4e8543b7 100644 --- a/cli/src/commands/remove.ts +++ b/cli/src/commands/remove.ts @@ -5,6 +5,7 @@ import { resolveRegistry, resolveToken } from '../services/registry-service' import { removeLocalSkill } from '../services/remove-service' import { CliError } from '../shared/errors' import { EXIT } from '../shared/constants' +import { parseSkillName } from '../shared/skill-name-parser' export interface RemoveCommandOptions { agent?: string[] | undefined @@ -17,7 +18,7 @@ export interface RemoveCommandOptions { json?: boolean | undefined } -export async function removeCommand(slug: string, options: RemoveCommandOptions): Promise { +export async function removeCommand(skillNameArg: string, options: RemoveCommandOptions): Promise { if (options.all && options.agent?.length) { throw new CliError('--all cannot be used with --agent', EXIT.usage) } @@ -29,9 +30,12 @@ export async function removeCommand(slug: string, options: RemoveCommandOptions) const credentialsStore = new CredentialsStore() const registry = resolveRegistry(options, process.env, await configStore.read()) + const parsed = parseSkillName(skillNameArg) + const namespace = options.namespace ?? parsed.namespace + const slug = parsed.slug + if (options.remote) { const token = resolveToken(options, process.env, await credentialsStore.getToken(registry)) - const namespace = options.namespace ?? 'global' if (!options.hard && process.stdout.isTTY) { const prompts = await import('prompts') diff --git a/cli/src/shared/skill-name-parser.ts b/cli/src/shared/skill-name-parser.ts new file mode 100644 index 00000000..05e0662b --- /dev/null +++ b/cli/src/shared/skill-name-parser.ts @@ -0,0 +1,27 @@ +export interface ParsedSkillName { + namespace: string + slug: string +} + +export function parseSkillName(skillName: string, defaultNamespace = 'global'): ParsedSkillName { + const separatorIndex = skillName.indexOf('--') + + if (separatorIndex <= 0) { + return { + namespace: defaultNamespace, + slug: separatorIndex === 0 ? skillName.slice(2) : skillName + } + } + + if (separatorIndex === skillName.length - 2) { + return { + namespace: defaultNamespace, + slug: skillName.slice(0, -2) + } + } + + return { + namespace: skillName.slice(0, separatorIndex), + slug: skillName.slice(separatorIndex + 2) + } +} diff --git a/cli/test/unit/shared/skill-name-parser.test.ts b/cli/test/unit/shared/skill-name-parser.test.ts new file mode 100644 index 00000000..b86771ce --- /dev/null +++ b/cli/test/unit/shared/skill-name-parser.test.ts @@ -0,0 +1,90 @@ +import { describe, test, expect } from 'bun:test' +import { parseSkillName } from '../../../src/shared/skill-name-parser' + +describe('parseSkillName', () => { + describe('with namespace--slug format', () => { + test('should parse namespace and slug separated by double dash', () => { + const result = parseSkillName('astroclaw--api-gateway') + expect(result).toEqual({ + namespace: 'astroclaw', + slug: 'api-gateway' + }) + }) + + test('should handle namespace and slug with single dashes', () => { + const result = parseSkillName('my-org--my-skill-name') + expect(result).toEqual({ + namespace: 'my-org', + slug: 'my-skill-name' + }) + }) + + test('should handle multiple double dashes by using first as separator', () => { + const result = parseSkillName('namespace--slug--with--dashes') + expect(result).toEqual({ + namespace: 'namespace', + slug: 'slug--with--dashes' + }) + }) + }) + + describe('with slug only format', () => { + test('should use default namespace when no separator present', () => { + const result = parseSkillName('api-gateway') + expect(result).toEqual({ + namespace: 'global', + slug: 'api-gateway' + }) + }) + + test('should use custom default namespace when provided', () => { + const result = parseSkillName('api-gateway', 'myorg') + expect(result).toEqual({ + namespace: 'myorg', + slug: 'api-gateway' + }) + }) + + test('should handle slug with single dashes', () => { + const result = parseSkillName('my-skill-name') + expect(result).toEqual({ + namespace: 'global', + slug: 'my-skill-name' + }) + }) + }) + + describe('edge cases', () => { + test('should handle separator at start', () => { + const result = parseSkillName('--api-gateway') + expect(result).toEqual({ + namespace: 'global', + slug: 'api-gateway' + }) + }) + + test('should handle separator at end', () => { + const result = parseSkillName('astroclaw--') + expect(result).toEqual({ + namespace: 'global', + slug: 'astroclaw' + }) + }) + + test('should handle empty string', () => { + const result = parseSkillName('') + expect(result).toEqual({ + namespace: 'global', + slug: '' + }) + }) + + test('should handle just separator', () => { + const result = parseSkillName('--') + expect(result).toEqual({ + namespace: 'global', + slug: '' + }) + }) + }) +}) diff --git a/docs/07-skill-protocol.md b/docs/07-skill-protocol.md index 6e9db3cc..4ee36a08 100644 --- a/docs/07-skill-protocol.md +++ b/docs/07-skill-protocol.md @@ -66,7 +66,7 @@ my-skill/ ``` 校验规则: -- 根目录必须包含 `SKILL.md` +- 根目录必须包含规范入口文件 `SKILL.md`;上传时服务端兼容 `skill.md`、`Skill.md` 等大小写变体,并在内部归一化为 `SKILL.md` - 文件类型白名单:`.md`, `.txt`, `.json`, `.yaml`, `.yml`, `.js`, `.cjs`, `.mjs`, `.ts`, `.py`, `.sh`, `.png`, `.jpg`, `.svg` - 单文件大小限制:1MB(可配置) - 总包大小限制:10MB(可配置) diff --git a/docs/15-backend-time-governance-plan.md b/docs/15-backend-time-governance-plan.md index 17952e50..6db6eac6 100644 --- a/docs/15-backend-time-governance-plan.md +++ b/docs/15-backend-time-governance-plan.md @@ -57,6 +57,10 @@ - 数据库列统一为 `TIMESTAMPTZ` - 读写都按 UTC 绝对时间处理 +进度登记: + +- `audit_log.created_at` 已通过 V42 迁移到 `TIMESTAMPTZ`,详见 `docs/16-backend-time-inventory.md` §3.1 + ### 3.2 业务输入时间 适用场景: diff --git a/docs/16-backend-time-inventory.md b/docs/16-backend-time-inventory.md index 565b444f..d0c8bfcb 100644 --- a/docs/16-backend-time-inventory.md +++ b/docs/16-backend-time-inventory.md @@ -131,6 +131,8 @@ - `review_task.submitted_at / reviewed_at` - `promotion_request.submitted_at / reviewed_at` - `idempotency_record.created_at / expires_at` +- `V42__audit_log_created_at_timestamptz.sql` + - `audit_log.created_at` ### 3.2 当前状态 diff --git a/docs/20-builtin-skills-design.md b/docs/20-builtin-skills-design.md deleted file mode 100644 index d968df5a..00000000 --- a/docs/20-builtin-skills-design.md +++ /dev/null @@ -1,382 +0,0 @@ -# Built-in Skills Initialization Design - -## Context - -New SkillHub deployments currently start without a guaranteed installable skill in the registry. -Users must first understand publishing or find an external package before they can verify search, -detail, download, and CLI installation flows. - -This design adds a small, built-in example skill that is bundled with the Java service and published -automatically to `@global` during application startup. - -The MVP example skill is `skillhub-hello`. It is intentionally generic and does not encode an -AgentGuard-specific product decision. Future official skills can reuse the same initialization -mechanism. - -## Goals - -- Bundle one or more directory-form skills in the Java service resources. -- Enable built-in skill initialization by default. -- Publish built-in skills to the fixed `global` namespace. -- Publish as `PUBLIC` and `PUBLISHED` so the skill is immediately searchable and installable. -- Use a fixed system publisher, `builtin-skill-publisher`, for owner and audit traceability. -- Reuse the existing `SkillPublishService.publishFromEntries(...)` pipeline. -- Keep initialization idempotent across repeated container deployments. -- Treat published versions as immutable: same version with changed content is skipped with a warning. -- Avoid new seed state tables and distributed locks in the MVP. -- Keep initialization failures non-fatal to application startup. -- Document `skillhub-hello` as an out-of-the-box verification skill. - -## Non-Goals - -- No new database table for seed state. -- No Redis or database distributed lock. -- No zip-based built-in skill packages. -- No configurable target namespace; built-in skills always publish to `global`. -- No label creation or binding for `skillhub-hello`. -- No landing page or frontend recommendation slot. -- No direct SQL/JPA insertion into `skill`, `skill_version`, or `skill_file`. -- No changes to ordinary publish, review, promotion, or lifecycle behavior. - -## Key Decisions - -| Decision | Choice | Reason | -|----------|--------|--------| -| Source location | Java service classpath resources | The runtime artifact always contains the built-in package | -| Directory | `server/skillhub-app/src/main/resources/builtin-skills/` | Spring Boot resource packaging is predictable | -| First built-in skill | `skillhub-hello` | Generic verification skill, not product-specific | -| Startup default | Enabled | Supports out-of-the-box discovery and installation | -| Target namespace | Fixed `global` | Built-in examples are platform-level public skills | -| Publication state | `PUBLIC + PUBLISHED` | Immediately searchable and installable | -| Publisher | `builtin-skill-publisher` | Stable owner and audit source | -| Version mutability | Same version is never overwritten | Published versions remain reproducible | -| Seed state table | None | Existing skill/version/file records are enough for MVP idempotency | -| Distributed lock | None | Conflict-tolerant startup is sufficient for a small built-in set | -| Labels | None | MVP validates the built-in publish mechanism only | -| Failure behavior | Log and continue | A sample skill must not make the service unavailable | -| Package format | Directory only | Easier review and classpath loading | - -## Resource Layout - -Built-in skills live under the `skillhub-app` resource tree: - -```text -server/skillhub-app/src/main/resources/builtin-skills/ - skillhub-hello/ - SKILL.md - README.md -``` - -Each direct child directory under `builtin-skills/` is treated as one skill package. - -Rules: - -- The directory must contain root-level `SKILL.md`. -- File paths are relative to the skill directory. -- Files are converted into `PackageEntry` values. -- Files outside the skill directory are ignored. -- Zip packages are not supported in the MVP. - -Suggested `skillhub-hello/SKILL.md`: - -```markdown ---- -name: skillhub-hello -description: A built-in example skill that verifies SkillHub discovery and installation. -version: 1.0.0 ---- -# SkillHub Hello - -This skill is bundled with SkillHub as a minimal example for validating discovery and installation. -``` - -Published coordinate: - -```text -@global/skillhub-hello -``` - -ClawHub canonical slug: - -```text -skillhub-hello -``` - -## Configuration - -Add one configuration property: - -```yaml -skillhub: - builtin-skills: - enabled: true -``` - -Environment override: - -```bash -SKILLHUB_BUILTIN_SKILLS_ENABLED=false -``` - -The MVP does not expose a namespace or locations property. The implementation uses the fixed -classpath location: - -```text -classpath*:builtin-skills/*/SKILL.md -``` - -## Backend Design - -### Components - -Add the following app-layer bootstrap components: - -- `BuiltinSkillProperties` -- `BuiltinSkillPackageLoader` -- `BuiltinSkillInitializer` - -Responsibilities: - -| Component | Responsibility | -|-----------|----------------| -| `BuiltinSkillProperties` | Bind `skillhub.builtin-skills.enabled` | -| `BuiltinSkillPackageLoader` | Read classpath skill directories and construct `PackageEntry` values | -| `BuiltinSkillInitializer` | Ensure publisher, evaluate idempotency, and call the publish pipeline | - -These classes belong in: - -```text -server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/ -``` - -### Publish Pipeline - -The initializer must call the existing domain publish service: - -```java -skillPublishService.publishFromEntries( - "global", - entries, - "builtin-skill-publisher", - SkillVisibility.PUBLIC, - Set.of("SUPER_ADMIN"), - false -); -``` - -This preserves existing behavior for: - -- package policy validation -- `SKILL.md` parsing -- slug generation -- `Skill` creation or reuse -- `SkillVersion` creation -- `PUBLISHED` state assignment -- `latestVersionId` updates -- `SkillFile` records -- object storage writes -- bundle zip creation -- `SkillPublishedEvent` -- after-commit search index rebuild - -The initializer must not create skill/version/file rows directly. -Passing `false` for warning confirmation means built-in packages with validation warnings are treated -as package quality failures and skipped instead of being silently accepted. - -### System Publisher - -The initializer ensures this system user exists: - -```text -userId: builtin-skill-publisher -displayName: SkillHub Built-in Publisher -email: builtin-skill-publisher@example.invalid -``` - -Requirements: - -- Create `UserAccount` if missing. -- Ensure the user is an `OWNER` member of `@global`. -- Do not create a local login credential. -- Do not require a persisted platform role binding. -- Pass `Set.of("SUPER_ADMIN")` only for the publish call to reuse auto-publish behavior. - -## Idempotency And Version Policy - -### Content Fingerprint - -The initializer computes a package fingerprint from current classpath resources: - -1. Sort entries by normalized path. -2. Hash each file content with SHA-256. -3. Build a canonical stream of `path + fileSha256`. -4. Hash that stream to produce the package fingerprint. - -For an existing published version, the initializer recomputes the same fingerprint from `skill_file`: - -1. Query the target `SkillVersion`. -2. Query its `SkillFile` rows. -3. Sort by `filePath`. -4. Build the canonical stream from `filePath + sha256`. -5. Hash that stream. - -No new persistence field is added. - -### Startup Rules - -For each built-in skill: - -| Existing state | Action | -|----------------|--------| -| Skill does not exist | Publish | -| Skill exists, same version does not exist | Publish new version | -| Same version is `PUBLISHED` and fingerprint matches | Skip | -| Same version is `PUBLISHED` and fingerprint differs | Warn and skip | -| Same version exists but is not `PUBLISHED` | Warn and skip | - -Same-version content changes must bump the version in `SKILL.md`. The initializer must never -overwrite a published version. - -### Concurrent Startup - -The MVP does not use a distributed lock. - -If multiple application instances start at the same time: - -- each instance performs the idempotency check; -- one instance may publish first; -- later instances may hit an existing-version conflict; -- conflict handling should re-read the existing version and skip when a valid published version is present; -- conflicts must be logged but must not fail application startup. - -## Failure Behavior - -Built-in skill initialization is best-effort. - -Rules: - -- Failure in one built-in skill does not prevent other built-in skills from being processed. -- Any initialization failure is logged at error level. -- Same-version fingerprint drift is logged at warning level. -- Same-version matching content is logged at info level. -- Successful publishing is logged at info level. -- Exceptions are contained inside the initializer and do not abort Spring Boot startup. - -Log context should include: - -- skill directory -- resolved slug -- resolved version -- namespace `global` -- action -- error message when applicable - -## Object Storage And Search - -Classpath resources are only the source for initialization. Published files still go through the -configured object storage backend: - -- LocalFile -- MinIO -- S3 - -The initializer does not write object storage directly. - -Search index updates also remain event-driven. The publish service emits `SkillPublishedEvent`, and -the existing search listener rebuilds the search document after transaction commit. - -## User Experience - -The MVP does not add frontend UI. - -After startup, users can discover and install the built-in skill through existing flows: - -- search for `skillhub-hello`; -- open the skill detail page; -- copy the existing install command; -- install with ClawHub/OpenClaw. - -Expected command: - -```bash -npx clawhub install skillhub-hello --registry -``` - -Because labels and recommendation slots are out of scope, this design does not guarantee permanent -homepage prominence for `skillhub-hello`. - -## Documentation - -Update the user-facing docs to mention the built-in verification skill: - -- `README.md` -- `docs/openclaw-integration.md` -- `docs/openclaw-integration-en.md` - -Recommended example: - -```bash -npx clawhub search skillhub-hello --registry -npx clawhub install skillhub-hello --registry -``` - -The docs should explain: - -- `skillhub-hello` is bundled with SkillHub; -- it validates registry search and installation; -- operators can disable initialization with `SKILLHUB_BUILTIN_SKILLS_ENABLED=false`. - -## Testing Strategy - -### Unit Tests - -Add backend tests for: - -- `enabled=false` skips all publishing. -- first startup publishes `skillhub-hello`; -- same version and same fingerprint skips; -- same version and different fingerprint warns and skips; -- same version in a non-`PUBLISHED` state warns and skips; -- publish exceptions are swallowed after logging; -- missing system publisher is created; -- missing `@global` membership is created; -- loader reads classpath directories into stable `PackageEntry` order; -- loader reports a directory missing `SKILL.md`. - -### Local Validation - -Run: - -```bash -make test-backend-app -``` - -If implementation touches runtime packaging or staging startup behavior, also run: - -```bash -make staging -``` - -### Manual Validation - -After local startup: - -```bash -curl "http://localhost:8080/api/web/skills?q=skillhub-hello" -npx clawhub search skillhub-hello --registry http://localhost:8080 -npx clawhub install skillhub-hello --registry http://localhost:8080 -``` - -## Future Extensions - -Potential follow-up work: - -- external filesystem source locations; -- zip package support; -- seed state table; -- distributed lock; -- official label binding; -- landing page official/recommended slot; -- AgentGuard or other official skills built on the same mechanism. - -These are outside the MVP. diff --git a/docs/openclaw-integration-en.md b/docs/openclaw-integration-en.md index 77d93e5e..32cb2878 100644 --- a/docs/openclaw-integration-en.md +++ b/docs/openclaw-integration-en.md @@ -72,10 +72,6 @@ npx clawhub search find-skills npx clawhub search find-skills --limit 5 npx clawhub inspect find-skills -# Bundled verification skill initialized by default on new deployments -npx clawhub search skillhub-hello -npx clawhub inspect skillhub-hello - # Help npx clawhub search --help npx clawhub inspect --help @@ -104,9 +100,6 @@ npx clawhub list npx clawhub --dir ~/.claude/skills install find-skills CLAWHUB_WORKDIR=~/.claude/skills npx clawhub install find-skills -# Install the bundled verification skill -npx clawhub install skillhub-hello - # Help npx clawhub install --help npx clawhub update --help @@ -217,12 +210,6 @@ export CLAWHUB_REGISTRY=https://skillhub.your-company.com clawhub login --token sk_your_api_token_here ``` -New SkillHub deployments initialize the bundled `skillhub-hello` skill in `@global` by default. To disable built-in skill initialization, set this before starting the backend: - -```bash -export SKILLHUB_BUILTIN_SKILLS_ENABLED=false -``` - ## FAQ ### Q: How do I switch back to public ClawHub? diff --git a/docs/openclaw-integration.md b/docs/openclaw-integration.md index 353676c3..f85f588e 100644 --- a/docs/openclaw-integration.md +++ b/docs/openclaw-integration.md @@ -72,10 +72,6 @@ npx clawhub search find-skills npx clawhub search find-skills --limit 5 npx clawhub inspect find-skills -# 新部署默认内置的验证 Skill -npx clawhub search skillhub-hello -npx clawhub inspect skillhub-hello - # 使用帮助 npx clawhub search --help npx clawhub inspect --help @@ -104,9 +100,6 @@ npx clawhub list npx clawhub --dir ~/.claude/skills install find-skills CLAWHUB_WORKDIR=~/.claude/skills npx clawhub install find-skills -# 安装默认内置的验证 Skill -npx clawhub install skillhub-hello - # 使用帮助 npx clawhub install --help npx clawhub update --help @@ -217,12 +210,6 @@ export CLAWHUB_REGISTRY=https://skillhub.your-company.com clawhub login --token sk_your_api_token_here ``` -SkillHub 新部署默认会在 `@global` 初始化内置 `skillhub-hello`。如需关闭内置 Skill 初始化,请在启动后端前设置: - -```bash -export SKILLHUB_BUILTIN_SKILLS_ENABLED=false -``` - ## 常见问题 ### Q: 如何切换回公共 ClawHub? diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillFingerprints.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillFingerprints.java deleted file mode 100644 index 836f733f..00000000 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillFingerprints.java +++ /dev/null @@ -1,42 +0,0 @@ -package com.iflytek.skillhub.bootstrap; - -import com.iflytek.skillhub.domain.skill.SkillFile; -import com.iflytek.skillhub.domain.skill.validation.PackageEntry; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; -import java.util.Comparator; -import java.util.HexFormat; -import java.util.List; - -final class BuiltinSkillFingerprints { - - private BuiltinSkillFingerprints() { - } - - static String fromEntries(List entries) { - StringBuilder canonical = new StringBuilder(); - entries.stream() - .sorted(Comparator.comparing(PackageEntry::path)) - .map(entry -> entry.path() + "\0" + sha256(entry.content())) - .forEach(line -> canonical.append(line).append('\n')); - return sha256(canonical.toString().getBytes(StandardCharsets.UTF_8)); - } - - static String fromFiles(List files) { - StringBuilder canonical = new StringBuilder(); - files.stream() - .sorted(Comparator.comparing(SkillFile::getFilePath)) - .map(file -> file.getFilePath() + "\0" + file.getSha256()) - .forEach(line -> canonical.append(line).append('\n')); - return sha256(canonical.toString().getBytes(StandardCharsets.UTF_8)); - } - - private static String sha256(byte[] content) { - try { - MessageDigest digest = MessageDigest.getInstance("SHA-256"); - return HexFormat.of().formatHex(digest.digest(content)); - } catch (Exception exception) { - throw new IllegalStateException("Failed to calculate SHA-256 fingerprint", exception); - } - } -} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java deleted file mode 100644 index 21c58f95..00000000 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java +++ /dev/null @@ -1,295 +0,0 @@ -package com.iflytek.skillhub.bootstrap; - -import com.iflytek.skillhub.domain.namespace.Namespace; -import com.iflytek.skillhub.domain.namespace.NamespaceMember; -import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.skill.Skill; -import com.iflytek.skillhub.domain.skill.SkillFileRepository; -import com.iflytek.skillhub.domain.skill.SkillRepository; -import com.iflytek.skillhub.domain.skill.SkillVersion; -import com.iflytek.skillhub.domain.skill.SkillVersionRepository; -import com.iflytek.skillhub.domain.skill.SkillVersionStatus; -import com.iflytek.skillhub.domain.skill.SkillVisibility; -import com.iflytek.skillhub.domain.skill.metadata.SkillMetadata; -import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; -import com.iflytek.skillhub.domain.skill.service.SkillPublishService; -import com.iflytek.skillhub.domain.skill.validation.PackageEntry; -import com.iflytek.skillhub.domain.skill.validation.SkillPackageValidator; -import com.iflytek.skillhub.domain.skill.validation.ValidationResult; -import com.iflytek.skillhub.domain.namespace.SlugValidator; -import com.iflytek.skillhub.domain.user.UserAccount; -import com.iflytek.skillhub.domain.user.UserAccountRepository; -import com.iflytek.skillhub.domain.user.UserStatus; -import java.nio.charset.StandardCharsets; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.boot.ApplicationArguments; -import org.springframework.boot.ApplicationRunner; -import org.springframework.stereotype.Component; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.support.TransactionTemplate; - -/** - * Publishes bundled example skills into the global namespace during startup. - */ -@Component -public class BuiltinSkillInitializer implements ApplicationRunner { - - static final String BUILTIN_PUBLISHER_ID = "builtin-skill-publisher"; - static final String GLOBAL_NAMESPACE = "global"; - private static final String BUILTIN_PUBLISHER_NAME = "SkillHub Built-in Publisher"; - private static final String BUILTIN_PUBLISHER_EMAIL = "builtin-skill-publisher@example.invalid"; - private static final Logger log = LoggerFactory.getLogger(BuiltinSkillInitializer.class); - - private final BuiltinSkillProperties properties; - private final BuiltinSkillPackageLoader packageLoader; - private final SkillMetadataParser metadataParser; - private final SkillPackageValidator packageValidator; - private final SkillPublishService skillPublishService; - private final NamespaceRepository namespaceRepository; - private final NamespaceMemberRepository namespaceMemberRepository; - private final UserAccountRepository userAccountRepository; - private final SkillRepository skillRepository; - private final SkillVersionRepository skillVersionRepository; - private final SkillFileRepository skillFileRepository; - private final TransactionTemplate transactionTemplate; - - public BuiltinSkillInitializer(BuiltinSkillProperties properties, - BuiltinSkillPackageLoader packageLoader, - SkillMetadataParser metadataParser, - SkillPackageValidator packageValidator, - SkillPublishService skillPublishService, - NamespaceRepository namespaceRepository, - NamespaceMemberRepository namespaceMemberRepository, - UserAccountRepository userAccountRepository, - SkillRepository skillRepository, - SkillVersionRepository skillVersionRepository, - SkillFileRepository skillFileRepository, - PlatformTransactionManager transactionManager) { - this.properties = properties; - this.packageLoader = packageLoader; - this.metadataParser = metadataParser; - this.packageValidator = packageValidator; - this.skillPublishService = skillPublishService; - this.namespaceRepository = namespaceRepository; - this.namespaceMemberRepository = namespaceMemberRepository; - this.userAccountRepository = userAccountRepository; - this.skillRepository = skillRepository; - this.skillVersionRepository = skillVersionRepository; - this.skillFileRepository = skillFileRepository; - this.transactionTemplate = new TransactionTemplate(transactionManager); - } - - @Override - public void run(ApplicationArguments args) { - if (!properties.isEnabled()) { - log.info("Built-in skill initialization is disabled"); - return; - } - - Namespace globalNamespace; - try { - globalNamespace = ensurePublisher(); - } catch (RuntimeException exception) { - log.error("Failed to prepare built-in skill publisher, skipping built-in skill initialization", - exception); - return; - } - if (globalNamespace == null) { - return; - } - - List packages; - try { - packages = packageLoader.loadPackages(); - } catch (Exception exception) { - log.error("Failed to load built-in skill packages", exception); - return; - } - - for (BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage : packages) { - try { - initializePackage(globalNamespace, skillPackage); - } catch (Exception exception) { - log.error("Failed to initialize built-in skill package [directory={}]", - skillPackage.directory(), exception); - } - } - } - - private Namespace ensurePublisher() { - return transactionTemplate.execute(status -> { - Namespace globalNamespace = namespaceRepository.findBySlug(GLOBAL_NAMESPACE) - .orElse(null); - if (globalNamespace == null) { - log.error("Missing built-in global namespace, skipping built-in skill initialization"); - return null; - } - - UserAccount publisher = userAccountRepository.findById(BUILTIN_PUBLISHER_ID) - .orElseGet(() -> new UserAccount( - BUILTIN_PUBLISHER_ID, - BUILTIN_PUBLISHER_NAME, - BUILTIN_PUBLISHER_EMAIL, - null - )); - publisher.setDisplayName(BUILTIN_PUBLISHER_NAME); - publisher.setEmail(BUILTIN_PUBLISHER_EMAIL); - publisher.setStatus(UserStatus.ACTIVE); - userAccountRepository.save(publisher); - - NamespaceMember member = namespaceMemberRepository - .findByNamespaceIdAndUserId(globalNamespace.getId(), BUILTIN_PUBLISHER_ID) - .orElseGet(() -> new NamespaceMember(globalNamespace.getId(), BUILTIN_PUBLISHER_ID, NamespaceRole.OWNER)); - if (member.getRole() != NamespaceRole.OWNER) { - member.setRole(NamespaceRole.OWNER); - } - namespaceMemberRepository.save(member); - return globalNamespace; - }); - } - - private void initializePackage(Namespace globalNamespace, - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage) { - List entries = skillPackage.entries(); - SkillMetadata metadata = parseMetadata(skillPackage.directory(), entries); - if (metadata.version() == null || metadata.version().isBlank()) { - log.error("Built-in skill package is missing an explicit version [directory={}]", - skillPackage.directory()); - return; - } - String skillSlug = SlugValidator.slugify(metadata.name()); - - ValidationResult validation = packageValidator.validate(entries); - if (!validation.passed() || validation.hasWarnings()) { - log.error("Built-in skill package failed validation [directory={}, slug={}, version={}, errors={}, warnings={}]", - skillPackage.directory(), skillSlug, metadata.version(), validation.errors(), validation.warnings()); - return; - } - - if (hasPublishedOtherOwnerConflict(globalNamespace.getId(), skillSlug)) { - log.warn("Skipping built-in skill because another owner already published the slug [directory={}, namespace={}, slug={}]", - skillPackage.directory(), GLOBAL_NAMESPACE, skillSlug); - return; - } - - Optional existingBuiltInSkill = - skillRepository.findByNamespaceIdAndSlugAndOwnerId(globalNamespace.getId(), skillSlug, BUILTIN_PUBLISHER_ID); - if (existingBuiltInSkill.isPresent() - && shouldSkipExistingVersion(skillPackage, existingBuiltInSkill.get(), metadata.version())) { - return; - } - - publishPackage(skillPackage, skillSlug, metadata.version()); - } - - private SkillMetadata parseMetadata(String directory, List entries) { - PackageEntry skillMd = entries.stream() - .filter(entry -> "SKILL.md".equals(entry.path())) - .findFirst() - .orElseThrow(() -> new IllegalArgumentException("Built-in package missing SKILL.md: " + directory)); - return metadataParser.parse(new String(skillMd.content(), StandardCharsets.UTF_8)); - } - - private boolean hasPublishedOtherOwnerConflict(Long namespaceId, String skillSlug) { - return skillRepository.findByNamespaceIdAndSlug(namespaceId, skillSlug).stream() - .filter(skill -> !BUILTIN_PUBLISHER_ID.equals(skill.getOwnerId())) - .anyMatch(skill -> !skillVersionRepository - .findBySkillIdAndStatus(skill.getId(), SkillVersionStatus.PUBLISHED) - .isEmpty()); - } - - private boolean shouldSkipExistingVersion(BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage, - Skill skill, - String version) { - Optional existingVersion = skillVersionRepository.findBySkillIdAndVersion(skill.getId(), version); - if (existingVersion.isEmpty()) { - return false; - } - - SkillVersion skillVersion = existingVersion.get(); - if (skillVersion.getStatus() != SkillVersionStatus.PUBLISHED) { - log.warn("Skipping built-in skill because the same version is not published [directory={}, skillId={}, version={}, status={}]", - skillPackage.directory(), skill.getId(), version, skillVersion.getStatus()); - return true; - } - - String currentFingerprint = BuiltinSkillFingerprints.fromEntries(skillPackage.entries()); - String existingFingerprint = BuiltinSkillFingerprints.fromFiles(skillFileRepository.findByVersionId(skillVersion.getId())); - if (currentFingerprint.equals(existingFingerprint)) { - log.info("Built-in skill version already exists, skipping [directory={}, skillId={}, version={}]", - skillPackage.directory(), skill.getId(), version); - } else { - log.warn("Skipping built-in skill because same published version has different content [directory={}, skillId={}, version={}]", - skillPackage.directory(), skill.getId(), version); - } - return true; - } - - private void publishPackage(BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage, - String skillSlug, - String version) { - try { - SkillPublishService.PublishResult result = skillPublishService.publishFromEntries( - GLOBAL_NAMESPACE, - skillPackage.entries(), - BUILTIN_PUBLISHER_ID, - SkillVisibility.PUBLIC, - Set.of("SUPER_ADMIN"), - false - ); - log.info("Published built-in skill [directory={}, namespace={}, slug={}, version={}, status={}]", - skillPackage.directory(), GLOBAL_NAMESPACE, result.slug(), - result.version().getVersion(), result.version().getStatus()); - } catch (RuntimeException exception) { - ConcurrentVersionState concurrentVersionState = - findConcurrentPublishedBuiltInVersionState(skillPackage, skillSlug, version); - if (concurrentVersionState == ConcurrentVersionState.MATCHING) { - log.warn("Built-in skill was already published concurrently, skipping [directory={}, namespace={}, slug={}, version={}]", - skillPackage.directory(), GLOBAL_NAMESPACE, skillSlug, version); - return; - } - if (concurrentVersionState == ConcurrentVersionState.DIFFERENT) { - log.warn("Skipping built-in skill because concurrently published same version has different content [directory={}, namespace={}, slug={}, version={}]", - skillPackage.directory(), GLOBAL_NAMESPACE, skillSlug, version); - return; - } - throw exception; - } - } - - private ConcurrentVersionState findConcurrentPublishedBuiltInVersionState( - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage, - String skillSlug, - String version) { - return namespaceRepository.findBySlug(GLOBAL_NAMESPACE) - .flatMap(namespace -> skillRepository.findByNamespaceIdAndSlugAndOwnerId( - namespace.getId(), - skillSlug, - BUILTIN_PUBLISHER_ID - )) - .flatMap(skill -> skillVersionRepository.findBySkillIdAndVersion(skill.getId(), version)) - .filter(skillVersion -> skillVersion.getStatus() == SkillVersionStatus.PUBLISHED) - .map(skillVersion -> { - String currentFingerprint = BuiltinSkillFingerprints.fromEntries(skillPackage.entries()); - String existingFingerprint = BuiltinSkillFingerprints.fromFiles( - skillFileRepository.findByVersionId(skillVersion.getId())); - if (currentFingerprint.equals(existingFingerprint)) { - return ConcurrentVersionState.MATCHING; - } - return ConcurrentVersionState.DIFFERENT; - }) - .orElse(ConcurrentVersionState.MISSING); - } - - private enum ConcurrentVersionState { - MISSING, - MATCHING, - DIFFERENT - } -} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageLoader.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageLoader.java deleted file mode 100644 index 9775691f..00000000 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageLoader.java +++ /dev/null @@ -1,107 +0,0 @@ -package com.iflytek.skillhub.bootstrap; - -import com.iflytek.skillhub.controller.support.SkillPackageContentTypeResolver; -import com.iflytek.skillhub.domain.skill.validation.PackageEntry; -import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy; -import java.io.IOException; -import java.io.InputStream; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.List; -import java.util.Set; -import java.util.TreeSet; -import org.springframework.core.io.Resource; -import org.springframework.core.io.support.PathMatchingResourcePatternResolver; -import org.springframework.core.io.support.ResourcePatternResolver; -import org.springframework.stereotype.Component; - -/** - * Loads directory-form built-in skill packages from classpath resources. - */ -@Component -public class BuiltinSkillPackageLoader { - - static final String BUILTIN_SKILLS_PATTERN = "classpath*:builtin-skills/*/SKILL.md"; - private static final String ROOT_MARKER = "builtin-skills/"; - private static final String SKILL_MD = "SKILL.md"; - - private final ResourcePatternResolver resourcePatternResolver; - - public BuiltinSkillPackageLoader() { - this(new PathMatchingResourcePatternResolver()); - } - - BuiltinSkillPackageLoader(ResourcePatternResolver resourcePatternResolver) { - this.resourcePatternResolver = resourcePatternResolver; - } - - public List loadPackages() throws IOException { - Set directories = discoverPackageDirectories(); - List packages = new ArrayList<>(); - for (String directory : directories) { - List packageEntries = loadPackageEntries(directory); - boolean hasSkillMd = packageEntries.stream().anyMatch(packageEntry -> SKILL_MD.equals(packageEntry.path())); - if (hasSkillMd) { - packages.add(new BuiltinSkillPackage(directory, packageEntries)); - } - } - return packages; - } - - private Set discoverPackageDirectories() throws IOException { - Resource[] skillMdResources = resourcePatternResolver.getResources(BUILTIN_SKILLS_PATTERN); - Set directories = new TreeSet<>(); - for (Resource resource : skillMdResources) { - String relativePath = relativeBuiltinPath(resource); - if (relativePath == null || relativePath.isBlank()) { - continue; - } - int separatorIndex = relativePath.indexOf('/'); - if (separatorIndex > 0 && SKILL_MD.equals(relativePath.substring(separatorIndex + 1))) { - directories.add(relativePath.substring(0, separatorIndex)); - } - } - return directories; - } - - private List loadPackageEntries(String directory) throws IOException { - Resource[] resources = resourcePatternResolver.getResources("classpath*:builtin-skills/" + directory + "/**"); - List entries = new ArrayList<>(); - for (Resource resource : resources) { - String relativePath = relativeBuiltinPath(resource); - if (relativePath == null || relativePath.isBlank() || relativePath.endsWith("/")) { - continue; - } - String directoryPrefix = directory + "/"; - if (!relativePath.startsWith(directoryPrefix) || directoryPrefix.length() == relativePath.length()) { - continue; - } - - String packagePath = SkillPackagePolicy.normalizeEntryPath(relativePath.substring(directoryPrefix.length())); - try (InputStream inputStream = resource.getInputStream()) { - byte[] content = inputStream.readAllBytes(); - entries.add(new PackageEntry( - packagePath, - content, - content.length, - SkillPackageContentTypeResolver.determineContentType(packagePath) - )); - } - } - return entries.stream() - .sorted(Comparator.comparing(PackageEntry::path)) - .toList(); - } - - private String relativeBuiltinPath(Resource resource) throws IOException { - String url = resource.getURL().toExternalForm(); - int markerIndex = url.lastIndexOf(ROOT_MARKER); - if (markerIndex < 0) { - return null; - } - return url.substring(markerIndex + ROOT_MARKER.length()); - } - - public record BuiltinSkillPackage(String directory, List entries) { - } -} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillProperties.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillProperties.java deleted file mode 100644 index 298a1af5..00000000 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillProperties.java +++ /dev/null @@ -1,21 +0,0 @@ -package com.iflytek.skillhub.bootstrap; - -import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.stereotype.Component; - -/** - * Configuration for publishing bundled example skills at application startup. - */ -@Component -@ConfigurationProperties(prefix = "skillhub.builtin-skills") -public class BuiltinSkillProperties { - private boolean enabled = true; - - public boolean isEnabled() { - return enabled; - } - - public void setEnabled(boolean enabled) { - this.enabled = enabled; - } -} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java index f5869eb1..0dd5f2ba 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java @@ -34,6 +34,8 @@ public class MeController extends BaseApiController { @RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "10") int size, @RequestParam(required = false) String filter, + @RequestParam(required = false) String q, + @RequestParam(required = false) String namespace, @AuthenticationPrincipal PlatformPrincipal principal) { if (principal == null) { throw new UnauthorizedException("error.auth.required"); @@ -41,7 +43,7 @@ public class MeController extends BaseApiController { return ok( "response.success.read", - mySkillAppService.listMySkills(principal.userId(), page, size, filter, principal.platformRoles()) + mySkillAppService.listMySkills(principal.userId(), page, size, filter, q, namespace, principal.platformRoles()) ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java index 481cda72..e1a0a57c 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.config.SkillPublishProperties; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy; import org.springframework.stereotype.Component; import org.springframework.web.multipart.MultipartFile; @@ -85,7 +86,7 @@ public class MultipartPackageExtractor { normalizedPath, content, content.length, - SkillPackageContentTypeResolver.determineContentType(normalizedPath) + determineContentType(normalizedPath) )); } } @@ -110,7 +111,15 @@ public class MultipartPackageExtractor { throw new DomainBadRequestException("error.skill.publish.package.invalid", "Unsafe package path: " + path); } - return path; + return SkillPackagePolicy.canonicalizeSkillMdPath(path); } + private String determineContentType(String filename) { + if (filename.endsWith(".py")) return "text/x-python"; + if (filename.endsWith(".json")) return "application/json"; + if (filename.endsWith(".yaml") || filename.endsWith(".yml")) return "application/x-yaml"; + if (filename.endsWith(".txt")) return "text/plain"; + if (filename.endsWith(".md")) return "text/markdown"; + return "application/octet-stream"; + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java index 489928bd..e2aa41c3 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java @@ -74,7 +74,7 @@ public class SkillPackageArchiveExtractor { normalizedPath, content, content.length, - SkillPackageContentTypeResolver.determineContentType(normalizedPath) + determineContentType(normalizedPath) )); zis.closeEntry(); } @@ -189,4 +189,28 @@ public class SkillPackageArchiveExtractor { return outputStream.toByteArray(); } + private String determineContentType(String filename) { + String lower = filename.toLowerCase(); + if (lower.endsWith(".py")) return "text/x-python"; + if (lower.endsWith(".json")) return "application/json"; + if (lower.endsWith(".yaml") || lower.endsWith(".yml")) return "application/x-yaml"; + if (lower.endsWith(".txt")) return "text/plain"; + if (lower.endsWith(".md")) return "text/markdown"; + if (lower.endsWith(".html")) return "text/html"; + if (lower.endsWith(".css")) return "text/css"; + if (lower.endsWith(".csv")) return "text/csv"; + if (lower.endsWith(".xml")) return "application/xml"; + if (lower.endsWith(".js") || lower.endsWith(".cjs") || lower.endsWith(".mjs")) return "text/javascript"; + if (lower.endsWith(".ts")) return "text/typescript"; + if (lower.endsWith(".sh") || lower.endsWith(".bash") || lower.endsWith(".zsh")) return "text/x-shellscript"; + if (lower.endsWith(".png")) return "image/png"; + if (lower.endsWith(".jpg") || lower.endsWith(".jpeg")) return "image/jpeg"; + if (lower.endsWith(".gif")) return "image/gif"; + if (lower.endsWith(".svg")) return "image/svg+xml"; + if (lower.endsWith(".webp")) return "image/webp"; + if (lower.endsWith(".ico")) return "image/x-icon"; + if (lower.endsWith(".pdf")) return "application/pdf"; + if (lower.endsWith(".toml")) return "application/toml"; + return "application/octet-stream"; + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageContentTypeResolver.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageContentTypeResolver.java deleted file mode 100644 index 25224e96..00000000 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageContentTypeResolver.java +++ /dev/null @@ -1,35 +0,0 @@ -package com.iflytek.skillhub.controller.support; - -/** - * Resolves package entry content types from filenames for upload and built-in package ingestion. - */ -public final class SkillPackageContentTypeResolver { - - private SkillPackageContentTypeResolver() { - } - - public static String determineContentType(String filename) { - String lower = filename.toLowerCase(); - if (lower.endsWith(".py")) return "text/x-python"; - if (lower.endsWith(".json")) return "application/json"; - if (lower.endsWith(".yaml") || lower.endsWith(".yml")) return "application/x-yaml"; - if (lower.endsWith(".txt")) return "text/plain"; - if (lower.endsWith(".md")) return "text/markdown"; - if (lower.endsWith(".html")) return "text/html"; - if (lower.endsWith(".css")) return "text/css"; - if (lower.endsWith(".csv")) return "text/csv"; - if (lower.endsWith(".xml")) return "application/xml"; - if (lower.endsWith(".js") || lower.endsWith(".cjs") || lower.endsWith(".mjs")) return "text/javascript"; - if (lower.endsWith(".ts")) return "text/typescript"; - if (lower.endsWith(".sh") || lower.endsWith(".bash") || lower.endsWith(".zsh")) return "text/x-shellscript"; - if (lower.endsWith(".png")) return "image/png"; - if (lower.endsWith(".jpg") || lower.endsWith(".jpeg")) return "image/jpeg"; - if (lower.endsWith(".gif")) return "image/gif"; - if (lower.endsWith(".svg")) return "image/svg+xml"; - if (lower.endsWith(".webp")) return "image/webp"; - if (lower.endsWith(".ico")) return "image/x-icon"; - if (lower.endsWith(".pdf")) return "application/pdf"; - if (lower.endsWith(".toml")) return "application/toml"; - return "application/octet-stream"; - } -} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/ZipPackageExtractor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/ZipPackageExtractor.java index e9c993f6..a23e8cc5 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/ZipPackageExtractor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/ZipPackageExtractor.java @@ -3,6 +3,7 @@ package com.iflytek.skillhub.controller.support; import com.iflytek.skillhub.config.SkillPublishProperties; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy; import org.springframework.stereotype.Component; import org.springframework.web.multipart.MultipartFile; @@ -66,7 +67,7 @@ public class ZipPackageExtractor { normalizedPath, content, content.length, - SkillPackageContentTypeResolver.determineContentType(normalizedPath) + determineContentType(normalizedPath) )); zis.closeEntry(); } @@ -112,11 +113,35 @@ public class ZipPackageExtractor { throw new DomainBadRequestException("error.skill.publish.package.invalid", "Unsafe package path: " + path); } - return normalizedPath; + return SkillPackagePolicy.canonicalizeSkillMdPath(normalizedPath); } catch (InvalidPathException ex) { throw new DomainBadRequestException("error.skill.publish.package.invalid", "Invalid package path: " + path); } } + private String determineContentType(String filename) { + String lower = filename.toLowerCase(); + if (lower.endsWith(".py")) return "text/x-python"; + if (lower.endsWith(".json")) return "application/json"; + if (lower.endsWith(".yaml") || lower.endsWith(".yml")) return "application/x-yaml"; + if (lower.endsWith(".txt")) return "text/plain"; + if (lower.endsWith(".md")) return "text/markdown"; + if (lower.endsWith(".html")) return "text/html"; + if (lower.endsWith(".css")) return "text/css"; + if (lower.endsWith(".csv")) return "text/csv"; + if (lower.endsWith(".xml")) return "application/xml"; + if (lower.endsWith(".js")) return "text/javascript"; + if (lower.endsWith(".ts")) return "text/typescript"; + if (lower.endsWith(".sh") || lower.endsWith(".bash") || lower.endsWith(".zsh")) return "text/x-shellscript"; + if (lower.endsWith(".png")) return "image/png"; + if (lower.endsWith(".jpg") || lower.endsWith(".jpeg")) return "image/jpeg"; + if (lower.endsWith(".gif")) return "image/gif"; + if (lower.endsWith(".svg")) return "image/svg+xml"; + if (lower.endsWith(".webp")) return "image/webp"; + if (lower.endsWith(".ico")) return "image/x-icon"; + if (lower.endsWith(".pdf")) return "application/pdf"; + if (lower.endsWith(".toml")) return "application/toml"; + return "application/octet-stream"; + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminAuditLogAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminAuditLogAppService.java index 0de4d935..4ca927b7 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminAuditLogAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminAuditLogAppService.java @@ -8,8 +8,11 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.StringUtils; -import java.sql.Timestamp; +import java.sql.ResultSet; +import java.sql.SQLException; import java.time.Instant; +import java.time.OffsetDateTime; +import java.time.ZoneOffset; import java.util.Collection; import java.util.List; @@ -109,7 +112,7 @@ public class AdminAuditLogAppService { rs.getString("request_id"), rs.getString("target_type"), toResourceId(rs.getObject("target_id")), - toInstant(rs.getTimestamp("created_at"))) + readInstant(rs, "created_at")) ); return new PageResponse<>(items, total == null ? 0 : total, page, size); @@ -151,15 +154,21 @@ public class AdminAuditLogAppService { } if (startTime != null) { clause.append(" AND al.created_at >= :startTime"); - parameters.addValue("startTime", Timestamp.from(startTime)); + parameters.addValue("startTime", toUtcOffsetDateTime(startTime)); } if (endTime != null) { clause.append(" AND al.created_at <= :endTime"); - parameters.addValue("endTime", Timestamp.from(endTime)); + parameters.addValue("endTime", toUtcOffsetDateTime(endTime)); } return clause.toString(); } + // Bind via OffsetDateTime so pgjdbc sends a TIMESTAMPTZ literal anchored to UTC, + // bypassing JVM-default-timezone interpretation that caused the 8h-offset bug. + private static OffsetDateTime toUtcOffsetDateTime(Instant instant) { + return OffsetDateTime.ofInstant(instant, ZoneOffset.UTC); + } + private String renderDetails(String detailJson, String targetType, Object targetId) { if (StringUtils.hasText(detailJson)) { return detailJson; @@ -170,8 +179,11 @@ public class AdminAuditLogAppService { return targetType + ":" + targetId; } - private Instant toInstant(Timestamp timestamp) { - return timestamp == null ? null : timestamp.toInstant(); + // Read via getObject(OffsetDateTime.class) to bypass JVM-TZ interpretation + // that caused the 8h-offset bug (getTimestamp() applies JVM default TZ). + private static Instant readInstant(ResultSet rs, String column) throws SQLException { + OffsetDateTime odt = rs.getObject(column, OffsetDateTime.class); + return odt == null ? null : odt.toInstant(); } private String toResourceId(Object targetId) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java index 9ad9e6f3..e36dc201 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java @@ -1,5 +1,7 @@ package com.iflytek.skillhub.service; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; @@ -36,6 +38,7 @@ public class MySkillAppService { private final SkillSubscriptionRepository skillSubscriptionRepository; private final MySkillQueryRepository mySkillQueryRepository; private final SkillLifecycleProjectionService skillLifecycleProjectionService; + private final NamespaceRepository namespaceRepository; public MySkillAppService( SkillRepository skillRepository, @@ -43,17 +46,19 @@ public class MySkillAppService { SkillStarRepository skillStarRepository, SkillSubscriptionRepository skillSubscriptionRepository, MySkillQueryRepository mySkillQueryRepository, - SkillLifecycleProjectionService skillLifecycleProjectionService) { + SkillLifecycleProjectionService skillLifecycleProjectionService, + NamespaceRepository namespaceRepository) { this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; this.skillStarRepository = skillStarRepository; this.skillSubscriptionRepository = skillSubscriptionRepository; this.mySkillQueryRepository = mySkillQueryRepository; this.skillLifecycleProjectionService = skillLifecycleProjectionService; + this.namespaceRepository = namespaceRepository; } public PageResponse listMySkills(String userId, int page, int size) { - return listMySkills(userId, page, size, null, java.util.Set.of()); + return listMySkills(userId, page, size, null, null, null, java.util.Set.of()); } public PageResponse listMySkills(String userId, @@ -61,10 +66,27 @@ public class MySkillAppService { int size, String filter, java.util.Set platformRoles) { + return listMySkills(userId, page, size, filter, null, null, platformRoles); + } + + public PageResponse listMySkills(String userId, + int page, + int size, + String filter, + String keyword, + String namespace, + java.util.Set platformRoles) { MySkillFilter normalizedFilter = parseFilter(filter); - Page skillPage = normalizedFilter == MySkillFilter.ALL - ? skillRepository.findByOwnerId(userId, PageRequest.of(page, size)) - : filterSkillsByLifecycle(userId, page, size, normalizedFilter, platformRoles); + + Page skillPage; + if (normalizedFilter == MySkillFilter.ALL + && (keyword == null || keyword.isBlank()) + && (namespace == null || namespace.isBlank())) { + skillPage = skillRepository.findByOwnerId(userId, PageRequest.of(page, size)); + } else { + skillPage = filterSkills(userId, page, size, normalizedFilter, keyword, namespace, platformRoles); + } + List items = mySkillQueryRepository.getSkillSummaries(skillPage.getContent(), userId); return new PageResponse<>(items, skillPage.getTotalElements(), skillPage.getNumber(), skillPage.getSize()); @@ -118,15 +140,34 @@ public class MySkillAppService { return new PageResponse<>(items, subPage.getTotalElements(), subPage.getNumber(), subPage.getSize()); } - private Page filterSkillsByLifecycle(String userId, - int page, - int size, - MySkillFilter filter, - java.util.Set platformRoles) { + private Page filterSkills(String userId, + int page, + int size, + MySkillFilter filter, + String keyword, + String namespace, + java.util.Set platformRoles) { List skills = skillRepository.findByOwnerId(userId); + + // Namespace filter + Long namespaceId = null; + if (namespace != null && !namespace.isBlank()) { + namespaceId = namespaceRepository.findBySlug(namespace.trim()) + .map(Namespace::getId) + .orElse(-1L); + } + + final Long finalNamespaceId = namespaceId; + String normalizedKeyword = keyword != null && !keyword.isBlank() + ? keyword.trim().toLowerCase(java.util.Locale.ROOT) + : null; + List filtered = skills.stream() + .filter(skill -> matchesNamespace(skill, finalNamespaceId)) + .filter(skill -> matchesKeyword(skill, normalizedKeyword)) .filter(skill -> matchesFilter(skill, filter, platformRoles)) .toList(); + int fromIndex = Math.min(page * size, filtered.size()); int toIndex = Math.min(fromIndex + size, filtered.size()); return new PageImpl<>( @@ -136,6 +177,35 @@ public class MySkillAppService { ); } + private boolean matchesNamespace(Skill skill, Long namespaceId) { + if (namespaceId == null) { + return true; + } + if (namespaceId == -1L) { + return false; + } + return skill.getNamespaceId().equals(namespaceId); + } + + private boolean matchesKeyword(Skill skill, String keyword) { + if (keyword == null) { + return true; + } + String displayName = skill.getDisplayName() != null ? skill.getDisplayName().toLowerCase(java.util.Locale.ROOT) : ""; + String slug = skill.getSlug() != null ? skill.getSlug().toLowerCase(java.util.Locale.ROOT) : ""; + String summary = skill.getSummary() != null ? skill.getSummary().toLowerCase(java.util.Locale.ROOT) : ""; + + return displayName.contains(keyword) || slug.contains(keyword) || summary.contains(keyword); + } + + private Page filterSkillsByLifecycle(String userId, + int page, + int size, + MySkillFilter filter, + java.util.Set platformRoles) { + return filterSkills(userId, page, size, filter, null, null, platformRoles); + } + private boolean matchesFilter(Skill skill, MySkillFilter filter, java.util.Set platformRoles) { if (filter == MySkillFilter.HIDDEN) { return platformRoles.contains("SUPER_ADMIN") && skill.isHidden(); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java index ea40dfee..c5b320c3 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java @@ -42,7 +42,7 @@ public class IdempotencyCleanupTask { Instant threshold = Instant.now(clock).minusSeconds(STALE_THRESHOLD_MINUTES * 60); int updated = idempotencyRecordRepository.markStaleAsFailed(threshold); if (updated > 0) { - logger.info("Marked {} stale processing records as failed", updated); + logger.info("Marked {} stale processing records as failed before threshold={}", updated, threshold); } } } diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 7d9a1cbc..a592b035 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -93,8 +93,6 @@ spring: enable: ${SPRING_MAIL_SMTP_STARTTLS_ENABLE:false} skillhub: - builtin-skills: - enabled: ${SKILLHUB_BUILTIN_SKILLS_ENABLED:true} auth: mock: enabled: ${SKILLHUB_AUTH_MOCK_ENABLED:false} diff --git a/server/skillhub-app/src/main/resources/builtin-skills/skillhub-hello/README.md b/server/skillhub-app/src/main/resources/builtin-skills/skillhub-hello/README.md deleted file mode 100644 index 4cff8bc4..00000000 --- a/server/skillhub-app/src/main/resources/builtin-skills/skillhub-hello/README.md +++ /dev/null @@ -1,5 +0,0 @@ -# SkillHub Hello - -This built-in example skill is published to `@global` when SkillHub starts. - -Use it to verify that skill discovery and CLI installation are working in a new deployment. diff --git a/server/skillhub-app/src/main/resources/builtin-skills/skillhub-hello/SKILL.md b/server/skillhub-app/src/main/resources/builtin-skills/skillhub-hello/SKILL.md deleted file mode 100644 index 8a313b0c..00000000 --- a/server/skillhub-app/src/main/resources/builtin-skills/skillhub-hello/SKILL.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: skillhub-hello -description: A built-in example skill that verifies SkillHub discovery and installation. -version: 1.0.0 ---- -# SkillHub Hello - -This skill is bundled with SkillHub as a minimal example for validating discovery and installation. diff --git a/server/skillhub-app/src/main/resources/db/migration/V42__audit_log_created_at_timestamptz.sql b/server/skillhub-app/src/main/resources/db/migration/V42__audit_log_created_at_timestamptz.sql new file mode 100644 index 00000000..5939f144 --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V42__audit_log_created_at_timestamptz.sql @@ -0,0 +1,39 @@ +-- Fix audit_log.created_at timezone issue +-- Background: TIMESTAMP (without timezone) causes 8-hour offset when JVM timezone != UTC +-- Solution: Upgrade to TIMESTAMPTZ and anchor existing data as UTC +-- Related: docs/15-backend-time-governance-plan.md section 3.1 +-- +-- Operational notes: +-- * ALTER COLUMN ... TYPE rewrites the entire audit_log table and rebuilds +-- idx_audit_log_created_at, idx_audit_log_actor_time, idx_audit_log_action_time +-- under ACCESS EXCLUSIVE lock. Run during a low-traffic window. +-- * Before applying in production, check table size: +-- SELECT pg_size_pretty(pg_total_relation_size('audit_log')); +-- Tables in the multi-GB range may need a maintenance window. +-- * SET LOCAL lock_timeout below makes a contended ALTER fail fast (rather than +-- queueing behind long-running readers); operators may re-run the migration +-- after clearing contention. The DO block guards against re-running on a +-- column that has already been migrated, so retries are safe. + +SET LOCAL lock_timeout = '30s'; + +DO $$ +DECLARE + current_type text; +BEGIN + SELECT data_type + INTO current_type + FROM information_schema.columns + WHERE table_schema = current_schema() + AND table_name = 'audit_log' + AND column_name = 'created_at'; + + IF current_type = 'timestamp without time zone' THEN + ALTER TABLE audit_log + ALTER COLUMN created_at TYPE TIMESTAMPTZ + USING created_at AT TIME ZONE 'UTC'; + RAISE NOTICE 'V42: audit_log.created_at -> TIMESTAMPTZ (UTC anchored)'; + ELSE + RAISE NOTICE 'V42: audit_log.created_at already % (skipped)', current_type; + END IF; +END $$; diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java deleted file mode 100644 index 299a4ddb..00000000 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java +++ /dev/null @@ -1,383 +0,0 @@ -package com.iflytek.skillhub.bootstrap; - -import com.iflytek.skillhub.domain.namespace.Namespace; -import com.iflytek.skillhub.domain.namespace.NamespaceMember; -import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.skill.Skill; -import com.iflytek.skillhub.domain.skill.SkillFile; -import com.iflytek.skillhub.domain.skill.SkillFileRepository; -import com.iflytek.skillhub.domain.skill.SkillRepository; -import com.iflytek.skillhub.domain.skill.SkillVersion; -import com.iflytek.skillhub.domain.skill.SkillVersionRepository; -import com.iflytek.skillhub.domain.skill.SkillVersionStatus; -import com.iflytek.skillhub.domain.skill.SkillVisibility; -import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; -import com.iflytek.skillhub.domain.skill.service.SkillPublishService; -import com.iflytek.skillhub.domain.skill.validation.PackageEntry; -import com.iflytek.skillhub.domain.skill.validation.SkillPackageValidator; -import com.iflytek.skillhub.domain.skill.validation.ValidationResult; -import com.iflytek.skillhub.domain.user.UserAccount; -import com.iflytek.skillhub.domain.user.UserAccountRepository; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; -import java.util.HexFormat; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.boot.DefaultApplicationArguments; -import org.springframework.test.util.ReflectionTestUtils; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.support.SimpleTransactionStatus; - -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.lenient; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -@ExtendWith(MockitoExtension.class) -class BuiltinSkillInitializerTest { - - private static final String PUBLISHER_ID = BuiltinSkillInitializer.BUILTIN_PUBLISHER_ID; - - @Mock private BuiltinSkillPackageLoader packageLoader; - @Mock private SkillPackageValidator packageValidator; - @Mock private SkillPublishService skillPublishService; - @Mock private NamespaceRepository namespaceRepository; - @Mock private NamespaceMemberRepository namespaceMemberRepository; - @Mock private UserAccountRepository userAccountRepository; - @Mock private SkillRepository skillRepository; - @Mock private SkillVersionRepository skillVersionRepository; - @Mock private SkillFileRepository skillFileRepository; - @Mock private PlatformTransactionManager transactionManager; - - private BuiltinSkillProperties properties; - private BuiltinSkillInitializer initializer; - private Namespace globalNamespace; - - @BeforeEach - void setUp() { - properties = new BuiltinSkillProperties(); - globalNamespace = new Namespace("global", "Global", "system"); - ReflectionTestUtils.setField(globalNamespace, "id", 1L); - lenient().when(transactionManager.getTransaction(any())).thenAnswer(ignored -> new SimpleTransactionStatus()); - lenient().when(packageValidator.validate(any())).thenReturn(ValidationResult.pass()); - - initializer = new BuiltinSkillInitializer( - properties, - packageLoader, - new SkillMetadataParser(), - packageValidator, - skillPublishService, - namespaceRepository, - namespaceMemberRepository, - userAccountRepository, - skillRepository, - skillVersionRepository, - skillFileRepository, - transactionManager - ); - } - - @Test - void disabledInitializerDoesNotLoadPackages() throws Exception { - properties.setEnabled(false); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(packageLoader, never()).loadPackages(); - verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), anyBoolean()); - } - - @Test - void firstStartupCreatesPublisherMembershipAndPublishesPackage() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.empty()); - when(skillPublishService.publishFromEntries( - eq("global"), - eq(skillPackage.entries()), - eq(PUBLISHER_ID), - eq(SkillVisibility.PUBLIC), - eq(Set.of("SUPER_ADMIN")), - eq(false) - )).thenReturn(publishResult("1.0.0")); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(userAccountRepository).save(any(UserAccount.class)); - verify(namespaceMemberRepository).save(any(NamespaceMember.class)); - verify(skillPublishService).publishFromEntries( - "global", - skillPackage.entries(), - PUBLISHER_ID, - SkillVisibility.PUBLIC, - Set.of("SUPER_ADMIN"), - false - ); - } - - @Test - void existingPublisherAndMembershipAreReusedIdempotently() throws Exception { - UserAccount existingPublisher = new UserAccount(PUBLISHER_ID, "Old name", "old@example.invalid", null); - NamespaceMember existingMember = new NamespaceMember(1L, PUBLISHER_ID, NamespaceRole.MEMBER); - when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(globalNamespace)); - when(userAccountRepository.findById(PUBLISHER_ID)).thenReturn(Optional.of(existingPublisher)); - when(userAccountRepository.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); - when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER_ID)) - .thenReturn(Optional.of(existingMember)); - when(namespaceMemberRepository.save(any(NamespaceMember.class))).thenAnswer(invocation -> invocation.getArgument(0)); - when(packageLoader.loadPackages()).thenReturn(List.of()); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(userAccountRepository).save(existingPublisher); - verify(namespaceMemberRepository).save(existingMember); - org.assertj.core.api.Assertions.assertThat(existingMember.getRole()).isEqualTo(NamespaceRole.OWNER); - } - - @Test - void validationWarningsSkipPublishWithoutConfirmingWarnings() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(packageValidator.validate(skillPackage.entries())) - .thenReturn(new ValidationResult(true, List.of(), List.of("warning"))); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), anyBoolean()); - } - - @Test - void samePublishedVersionWithSameFingerprintSkipsPublish() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - Skill skill = builtInSkill(11L); - SkillVersion version = version(11L, 22L, "1.0.0", SkillVersionStatus.PUBLISHED); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(skill)); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.of(skill)); - when(skillVersionRepository.findBySkillIdAndVersion(11L, "1.0.0")).thenReturn(Optional.of(version)); - when(skillFileRepository.findByVersionId(22L)).thenReturn(filesFor(version.getId(), skillPackage.entries())); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), anyBoolean()); - } - - @Test - void samePublishedVersionWithDifferentFingerprintSkipsPublish() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - Skill skill = builtInSkill(11L); - SkillVersion version = version(11L, 22L, "1.0.0", SkillVersionStatus.PUBLISHED); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(skill)); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.of(skill)); - when(skillVersionRepository.findBySkillIdAndVersion(11L, "1.0.0")).thenReturn(Optional.of(version)); - when(skillFileRepository.findByVersionId(22L)).thenReturn(List.of( - new SkillFile(22L, "SKILL.md", 10L, "text/markdown", "different", "key") - )); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), anyBoolean()); - } - - @Test - void sameNonPublishedVersionSkipsPublish() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - Skill skill = builtInSkill(11L); - SkillVersion version = version(11L, 22L, "1.0.0", SkillVersionStatus.UPLOADED); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(skill)); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.of(skill)); - when(skillVersionRepository.findBySkillIdAndVersion(11L, "1.0.0")).thenReturn(Optional.of(version)); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(skillFileRepository, never()).findByVersionId(any()); - verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), anyBoolean()); - } - - @Test - void newerBuiltInVersionPublishesWhenOlderVersionExists() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.1", "Hello"); - Skill skill = builtInSkill(11L); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(skill)); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.of(skill)); - when(skillVersionRepository.findBySkillIdAndVersion(11L, "1.0.1")).thenReturn(Optional.empty()); - when(skillPublishService.publishFromEntries(any(), any(), any(), any(), any(), eq(false))) - .thenReturn(publishResult("1.0.1")); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(skillPublishService).publishFromEntries( - "global", - skillPackage.entries(), - PUBLISHER_ID, - SkillVisibility.PUBLIC, - Set.of("SUPER_ADMIN"), - false - ); - } - - @Test - void userOwnedPublishedSlugSkipsBuiltInPublish() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - Skill otherSkill = new Skill(1L, "skillhub-hello", "user-1", SkillVisibility.PUBLIC); - ReflectionTestUtils.setField(otherSkill, "id", 33L); - SkillVersion otherPublishedVersion = version(33L, 44L, "1.0.0", SkillVersionStatus.PUBLISHED); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(otherSkill)); - when(skillVersionRepository.findBySkillIdAndStatus(33L, SkillVersionStatus.PUBLISHED)) - .thenReturn(List.of(otherPublishedVersion)); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(skillRepository, never()).findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID); - verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), anyBoolean()); - } - - @Test - void publishFailureIsContainedAndDoesNotAbortStartup() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.empty()); - when(skillPublishService.publishFromEntries(any(), any(), any(), any(), any(), eq(false))) - .thenThrow(new IllegalStateException("storage unavailable")); - - assertThatCode(() -> initializer.run(new DefaultApplicationArguments(new String[0]))) - .doesNotThrowAnyException(); - } - - @Test - void concurrentPublishedSameFingerprintSkipsAfterPublishFailure() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - Skill skill = builtInSkill(11L); - SkillVersion version = version(11L, 22L, "1.0.0", SkillVersionStatus.PUBLISHED); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.empty(), Optional.of(skill)); - when(skillPublishService.publishFromEntries(any(), any(), any(), any(), any(), eq(false))) - .thenThrow(new IllegalStateException("version exists")); - when(skillVersionRepository.findBySkillIdAndVersion(11L, "1.0.0")).thenReturn(Optional.of(version)); - when(skillFileRepository.findByVersionId(22L)).thenReturn(filesFor(version.getId(), skillPackage.entries())); - - assertThatCode(() -> initializer.run(new DefaultApplicationArguments(new String[0]))) - .doesNotThrowAnyException(); - } - - @Test - void concurrentPublishedDifferentFingerprintSkipsAfterPublishFailure() throws Exception { - BuiltinSkillPackageLoader.BuiltinSkillPackage skillPackage = packageWithVersion("1.0.0", "Hello"); - Skill skill = builtInSkill(11L); - SkillVersion version = version(11L, 22L, "1.0.0", SkillVersionStatus.PUBLISHED); - setupPublisher(); - when(packageLoader.loadPackages()).thenReturn(List.of(skillPackage)); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); - when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "skillhub-hello", PUBLISHER_ID)) - .thenReturn(Optional.empty(), Optional.of(skill)); - when(skillPublishService.publishFromEntries(any(), any(), any(), any(), any(), eq(false))) - .thenThrow(new IllegalStateException("version exists")); - when(skillVersionRepository.findBySkillIdAndVersion(11L, "1.0.0")).thenReturn(Optional.of(version)); - when(skillFileRepository.findByVersionId(22L)).thenReturn(List.of( - new SkillFile(22L, "SKILL.md", 10L, "text/markdown", "different", "key") - )); - - assertThatCode(() -> initializer.run(new DefaultApplicationArguments(new String[0]))) - .doesNotThrowAnyException(); - } - - private void setupPublisher() { - when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(globalNamespace)); - when(userAccountRepository.findById(PUBLISHER_ID)).thenReturn(Optional.empty()); - when(userAccountRepository.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); - when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER_ID)).thenReturn(Optional.empty()); - when(namespaceMemberRepository.save(any(NamespaceMember.class))).thenAnswer(invocation -> invocation.getArgument(0)); - } - - private BuiltinSkillPackageLoader.BuiltinSkillPackage packageWithVersion(String version, String body) { - byte[] skillMd = (""" - --- - name: skillhub-hello - description: SkillHub hello - version: %s - --- - # SkillHub Hello - %s - """.formatted(version, body)).getBytes(StandardCharsets.UTF_8); - byte[] readme = "# SkillHub Hello\n".getBytes(StandardCharsets.UTF_8); - return new BuiltinSkillPackageLoader.BuiltinSkillPackage("skillhub-hello", List.of( - new PackageEntry("README.md", readme, readme.length, "text/markdown"), - new PackageEntry("SKILL.md", skillMd, skillMd.length, "text/markdown") - )); - } - - private Skill builtInSkill(Long id) { - Skill skill = new Skill(1L, "skillhub-hello", PUBLISHER_ID, SkillVisibility.PUBLIC); - ReflectionTestUtils.setField(skill, "id", id); - return skill; - } - - private SkillVersion version(Long skillId, Long versionId, String version, SkillVersionStatus status) { - SkillVersion skillVersion = new SkillVersion(skillId, version, PUBLISHER_ID); - skillVersion.setStatus(status); - ReflectionTestUtils.setField(skillVersion, "id", versionId); - return skillVersion; - } - - private SkillPublishService.PublishResult publishResult(String version) { - SkillVersion skillVersion = version(11L, 22L, version, SkillVersionStatus.PUBLISHED); - return new SkillPublishService.PublishResult(11L, "skillhub-hello", skillVersion); - } - - private List filesFor(Long versionId, List entries) { - return entries.stream() - .map(entry -> new SkillFile( - versionId, - entry.path(), - entry.size(), - entry.contentType(), - sha256(entry.content()), - "skills/11/" + versionId + "/" + entry.path() - )) - .toList(); - } - - private String sha256(byte[] content) { - try { - MessageDigest digest = MessageDigest.getInstance("SHA-256"); - return HexFormat.of().formatHex(digest.digest(content)); - } catch (Exception exception) { - throw new IllegalStateException(exception); - } - } -} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageLoaderTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageLoaderTest.java deleted file mode 100644 index 143da879..00000000 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageLoaderTest.java +++ /dev/null @@ -1,124 +0,0 @@ -package com.iflytek.skillhub.bootstrap; - -import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; -import com.iflytek.skillhub.domain.skill.validation.SkillPackageValidator; -import java.net.URLClassLoader; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.List; -import java.util.jar.JarEntry; -import java.util.jar.JarOutputStream; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; -import org.springframework.core.io.support.PathMatchingResourcePatternResolver; - -import static org.assertj.core.api.Assertions.assertThat; - -class BuiltinSkillPackageLoaderTest { - - @TempDir - Path tempDir; - - @Test - void loadsSkillhubHelloFromClasspath() throws Exception { - BuiltinSkillPackageLoader loader = - new BuiltinSkillPackageLoader(new PathMatchingResourcePatternResolver()); - - var packages = loader.loadPackages(); - - var skillhubHello = packages.stream() - .filter(skillPackage -> skillPackage.directory().equals("skillhub-hello")) - .findFirst() - .orElseThrow(); - assertThat(skillhubHello.entries()) - .extracting(entry -> entry.path()) - .containsExactly("README.md", "SKILL.md"); - assertThat(skillhubHello.entries()) - .allSatisfy(entry -> assertThat(entry.contentType()).isEqualTo("text/markdown")); - } - - @Test - void productionSkillhubHelloResourcePassesPackageValidation() throws Exception { - BuiltinSkillPackageLoader loader = - new BuiltinSkillPackageLoader(new PathMatchingResourcePatternResolver()); - SkillPackageValidator validator = new SkillPackageValidator(new SkillMetadataParser()); - - var skillhubHello = loader.loadPackages().stream() - .filter(skillPackage -> skillPackage.directory().equals("skillhub-hello")) - .findFirst() - .orElseThrow(); - - var result = validator.validate(skillhubHello.entries()); - assertThat(result.passed()).isTrue(); - assertThat(result.warnings()).isEmpty(); - } - - @Test - void loadsBuiltInPackageFromJarClasspath() throws Exception { - Path jarPath = tempDir.resolve("builtin-skills.jar"); - try (JarOutputStream jarOutputStream = new JarOutputStream(Files.newOutputStream(jarPath))) { - writeJarDirectory(jarOutputStream, "builtin-skills/"); - writeJarDirectory(jarOutputStream, "builtin-skills/skillhub-hello/"); - writeJarEntry(jarOutputStream, "builtin-skills/skillhub-hello/SKILL.md", """ - --- - name: skillhub-hello - description: SkillHub hello - version: 1.0.0 - --- - # SkillHub Hello - """); - writeJarEntry(jarOutputStream, "builtin-skills/skillhub-hello/README.md", "# SkillHub Hello\n"); - } - - try (URLClassLoader classLoader = new URLClassLoader( - new java.net.URL[]{jarPath.toUri().toURL()}, - null - )) { - BuiltinSkillPackageLoader loader = - new BuiltinSkillPackageLoader(new PathMatchingResourcePatternResolver(classLoader)); - - var packages = loader.loadPackages(); - - assertThat(packages).hasSize(1); - assertThat(packages.getFirst().directory()).isEqualTo("skillhub-hello"); - assertThat(packages.getFirst().entries()) - .extracting(entry -> entry.path()) - .containsExactly("README.md", "SKILL.md"); - } - } - - @Test - void fingerprintsAreStableAcrossEntryOrdering() { - byte[] skillMd = """ - --- - name: demo - description: Demo - version: 1.0.0 - --- - # Demo - """.getBytes(java.nio.charset.StandardCharsets.UTF_8); - byte[] readme = "# Demo\n".getBytes(java.nio.charset.StandardCharsets.UTF_8); - var first = List.of( - new com.iflytek.skillhub.domain.skill.validation.PackageEntry( - "SKILL.md", skillMd, skillMd.length, "text/markdown"), - new com.iflytek.skillhub.domain.skill.validation.PackageEntry( - "README.md", readme, readme.length, "text/markdown") - ); - var second = List.of(first.get(1), first.get(0)); - - assertThat(BuiltinSkillFingerprints.fromEntries(first)) - .isEqualTo(BuiltinSkillFingerprints.fromEntries(second)); - } - - private void writeJarDirectory(JarOutputStream jarOutputStream, String name) throws Exception { - jarOutputStream.putNextEntry(new JarEntry(name)); - jarOutputStream.closeEntry(); - } - - private void writeJarEntry(JarOutputStream jarOutputStream, String name, String content) throws Exception { - jarOutputStream.putNextEntry(new JarEntry(name)); - jarOutputStream.write(content.getBytes(StandardCharsets.UTF_8)); - jarOutputStream.closeEntry(); - } -} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPropertiesBindingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPropertiesBindingTest.java deleted file mode 100644 index 04461428..00000000 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPropertiesBindingTest.java +++ /dev/null @@ -1,47 +0,0 @@ -package com.iflytek.skillhub.bootstrap; - -import java.util.List; -import java.util.Map; -import org.junit.jupiter.api.Test; -import org.springframework.boot.context.properties.bind.Binder; -import org.springframework.boot.context.properties.source.ConfigurationPropertySources; -import org.springframework.boot.env.YamlPropertySourceLoader; -import org.springframework.core.env.ConfigurableEnvironment; -import org.springframework.core.env.StandardEnvironment; -import org.springframework.core.env.SystemEnvironmentPropertySource; -import org.springframework.core.io.ClassPathResource; - -import static org.assertj.core.api.Assertions.assertThat; - -class BuiltinSkillPropertiesBindingTest { - - @Test - void defaultConfigEnablesBuiltinSkills() throws Exception { - BuiltinSkillProperties properties = bindProperties(Map.of()); - - assertThat(properties.isEnabled()).isTrue(); - } - - @Test - void environmentVariableCanDisableBuiltinSkills() throws Exception { - BuiltinSkillProperties properties = bindProperties(Map.of("SKILLHUB_BUILTIN_SKILLS_ENABLED", "false")); - - assertThat(properties.isEnabled()).isFalse(); - } - - private BuiltinSkillProperties bindProperties(Map envVars) throws Exception { - ConfigurableEnvironment environment = new StandardEnvironment(); - environment.getPropertySources().addFirst(new SystemEnvironmentPropertySource("test-env", envVars)); - - YamlPropertySourceLoader loader = new YamlPropertySourceLoader(); - for (org.springframework.core.env.PropertySource propertySource : - loader.load("application.yml", new ClassPathResource("application.yml"))) { - environment.getPropertySources().addLast(propertySource); - } - ConfigurationPropertySources.attach(environment); - - return Binder.get(environment) - .bind("skillhub.builtin-skills", BuiltinSkillProperties.class) - .orElseThrow(() -> new IllegalStateException("Failed to bind built-in skill properties")); - } -} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java index af16f4dd..e4e7b80b 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java @@ -56,7 +56,7 @@ class MeControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER")) ); - given(mySkillAppService.listMySkills("user-42", 1, 5, null, Set.of("USER"))) + given(mySkillAppService.listMySkills("user-42", 1, 5, null, null, null, Set.of("USER"))) .willReturn(new PageResponse<>( List.of(new SkillSummaryResponse( 7L, @@ -103,7 +103,7 @@ class MeControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) ); - given(mySkillAppService.listMySkills("user-42", 0, 10, "HIDDEN", Set.of("SUPER_ADMIN"))) + given(mySkillAppService.listMySkills("user-42", 0, 10, "HIDDEN", null, null, Set.of("SUPER_ADMIN"))) .willReturn(new PageResponse<>(List.of(), 0, 0, 10)); mockMvc.perform(get("/api/v1/me/skills") diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractorTest.java new file mode 100644 index 00000000..f3be9120 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractorTest.java @@ -0,0 +1,35 @@ +package com.iflytek.skillhub.controller.support; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.config.SkillPublishProperties; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockMultipartFile; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MultipartPackageExtractorTest { + + @Test + void extractCanonicalizesCaseInsensitiveSkillMd() throws Exception { + MultipartPackageExtractor extractor = new MultipartPackageExtractor( + new SkillPublishProperties(), + new ObjectMapper() + ); + MockMultipartFile skillMd = new MockMultipartFile( + "files", + "skill.md", + "text/markdown", + "---\nname: test\n---\n".getBytes() + ); + + MultipartPackageExtractor.ExtractedPackage extracted = extractor.extract( + new MockMultipartFile[] {skillMd}, + "{\"namespace\":\"global\",\"slug\":\"test\"}" + ); + + assertEquals(1, extracted.entries().size()); + assertTrue(extracted.entries().stream().anyMatch(e -> e.path().equals("SKILL.md"))); + assertTrue(extracted.entries().stream().noneMatch(e -> e.path().equals("skill.md"))); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java index ba5cc2f7..c849fc4c 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java @@ -85,6 +85,21 @@ class SkillPackageArchiveExtractorTest { assertTrue(entries.stream().anyMatch(e -> e.path().equals("config.json"))); } + @Test + void canonicalizesCaseInsensitiveSkillMdAtRoot() throws Exception { + byte[] zipBytes = createZip(Map.of( + "skill.md", "---\nname: test\n---\n".getBytes(), + "README.md", "# readme".getBytes() + )); + MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zipBytes); + + SkillPackageArchiveExtractor.ExtractionResult result = extractor.extractWithWarnings(file); + + assertTrue(result.entries().stream().anyMatch(e -> e.path().equals("SKILL.md"))); + assertTrue(result.entries().stream().noneMatch(e -> e.path().equals("skill.md"))); + assertTrue(result.warnings().isEmpty()); + } + @Test void doesNotStripWhenMultipleRootEntries() throws Exception { byte[] zipBytes = createZip(Map.of( @@ -144,6 +159,23 @@ class SkillPackageArchiveExtractorTest { assertTrue(result.warnings().stream().anyMatch(w -> w.contains("other.txt"))); } + @Test + void promotesCaseInsensitiveSkillMdFromSubdirectory() throws Exception { + byte[] zipBytes = createZip(Map.of( + "my-skill/skill.md", "---\nname: test\n---\n".getBytes(), + "my-skill/README.md", "# readme".getBytes(), + "other.txt", "stray file".getBytes() + )); + MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zipBytes); + + SkillPackageArchiveExtractor.ExtractionResult result = extractor.extractWithWarnings(file); + + assertEquals(2, result.entries().size()); + assertTrue(result.entries().stream().anyMatch(e -> e.path().equals("SKILL.md"))); + assertTrue(result.entries().stream().anyMatch(e -> e.path().equals("README.md"))); + assertTrue(result.warnings().stream().anyMatch(w -> w.contains("other.txt"))); + } + @Test void rejectsAmbiguousMultipleSkillMdInSubdirectories() throws Exception { byte[] zipBytes = createZip(Map.of( diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageContentTypeResolverTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageContentTypeResolverTest.java deleted file mode 100644 index 90166104..00000000 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageContentTypeResolverTest.java +++ /dev/null @@ -1,17 +0,0 @@ -package com.iflytek.skillhub.controller.support; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -class SkillPackageContentTypeResolverTest { - - @Test - void determinesKnownContentTypes() { - assertThat(SkillPackageContentTypeResolver.determineContentType("SKILL.md")).isEqualTo("text/markdown"); - assertThat(SkillPackageContentTypeResolver.determineContentType("script.py")).isEqualTo("text/x-python"); - assertThat(SkillPackageContentTypeResolver.determineContentType("config.yaml")).isEqualTo("application/x-yaml"); - assertThat(SkillPackageContentTypeResolver.determineContentType("image.png")).isEqualTo("image/png"); - assertThat(SkillPackageContentTypeResolver.determineContentType("archive.bin")).isEqualTo("application/octet-stream"); - } -} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/ZipPackageExtractorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/ZipPackageExtractorTest.java new file mode 100644 index 00000000..02f26214 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/ZipPackageExtractorTest.java @@ -0,0 +1,47 @@ +package com.iflytek.skillhub.controller.support; + +import com.iflytek.skillhub.config.SkillPublishProperties; +import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockMultipartFile; + +import java.io.ByteArrayOutputStream; +import java.util.List; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class ZipPackageExtractorTest { + + @Test + void extractCanonicalizesCaseInsensitiveSkillMd() throws Exception { + ZipPackageExtractor extractor = new ZipPackageExtractor(new SkillPublishProperties()); + byte[] zipBytes = createZip(Map.of( + "skill.md", "---\nname: test\n---\n".getBytes(), + "README.md", "# readme".getBytes() + )); + MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zipBytes); + + List entries = extractor.extract(file); + + assertEquals(2, entries.size()); + assertTrue(entries.stream().anyMatch(e -> e.path().equals("SKILL.md"))); + assertTrue(entries.stream().noneMatch(e -> e.path().equals("skill.md"))); + } + + private byte[] createZip(Map entries) throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(baos)) { + for (Map.Entry e : entries.entrySet()) { + ZipEntry entry = new ZipEntry(e.getKey()); + zos.putNextEntry(entry); + zos.write(e.getValue()); + zos.closeEntry(); + } + } + return baos.toByteArray(); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminAuditLogAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminAuditLogAppServiceTest.java index 2b2b255e..3687f414 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminAuditLogAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminAuditLogAppServiceTest.java @@ -2,13 +2,21 @@ package com.iflytek.skillhub.service; import com.iflytek.skillhub.dto.AuditLogItemResponse; import com.iflytek.skillhub.dto.PageResponse; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.ArgumentCaptor; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import java.sql.ResultSet; import java.time.Instant; +import java.time.OffsetDateTime; +import java.time.ZoneOffset; import java.util.List; +import java.util.TimeZone; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.*; @@ -16,8 +24,14 @@ import static org.mockito.Mockito.*; class AdminAuditLogAppServiceTest { - private final NamedParameterJdbcTemplate jdbcTemplate = mock(NamedParameterJdbcTemplate.class); - private final AdminAuditLogAppService service = new AdminAuditLogAppService(jdbcTemplate); + private NamedParameterJdbcTemplate jdbcTemplate; + private AdminAuditLogAppService service; + + @BeforeEach + void setUp() { + jdbcTemplate = mock(NamedParameterJdbcTemplate.class); + service = new AdminAuditLogAppService(jdbcTemplate); + } @Test void listAuditLogs_returnsJdbcBackedPage() { @@ -62,4 +76,121 @@ class AdminAuditLogAppServiceTest { any(MapSqlParameterSource.class), any(RowMapper.class)); } + + /** + * Regression for the 8-hour offset bug: row mapper must read created_at via + * getObject(OffsetDateTime.class) so the returned Instant is independent of + * the JVM default timezone. + */ + @Test + void rowMapper_readsCreatedAtAsInstant() throws Exception { + RowMapper rowMapper = captureRowMapper(); + ResultSet rs = stubRowWithCreatedAt( + OffsetDateTime.of(2026, 5, 29, 8, 53, 0, 0, ZoneOffset.UTC)); + + AuditLogItemResponse item = rowMapper.mapRow(rs, 0); + + assertThat(item).isNotNull(); + assertThat(item.timestamp()).isEqualTo(Instant.parse("2026-05-29T08:53:00Z")); + verify(rs, never()).getTimestamp(anyString()); + } + + @Test + void rowMapper_normalisesNonUtcOffsetToInstant() throws Exception { + RowMapper rowMapper = captureRowMapper(); + ResultSet rs = stubRowWithCreatedAt( + OffsetDateTime.of(2026, 5, 29, 16, 53, 0, 0, ZoneOffset.ofHours(8))); + + AuditLogItemResponse item = rowMapper.mapRow(rs, 0); + + assertThat(item.timestamp()).isEqualTo(Instant.parse("2026-05-29T08:53:00Z")); + } + + @Test + void rowMapper_returnsNullTimestampWhenColumnIsNull() throws Exception { + RowMapper rowMapper = captureRowMapper(); + ResultSet rs = stubRowWithCreatedAt(null); + + AuditLogItemResponse item = rowMapper.mapRow(rs, 0); + + assertThat(item.timestamp()).isNull(); + } + + @ParameterizedTest + @CsvSource(nullValues = "NULL", value = { + "2026-03-13T00:00:00Z, 2026-03-14T00:00:00Z", + "2026-03-13T00:00:00Z, NULL", + "NULL, 2026-03-14T00:00:00Z" + }) + void buildWhereClause_bindsTimeRangeAsOffsetDateTime(String startStr, String endStr) { + when(jdbcTemplate.queryForObject(contains("COUNT(*)"), any(MapSqlParameterSource.class), eq(Long.class))) + .thenReturn(0L); + when(jdbcTemplate.query(contains("FROM audit_log"), any(MapSqlParameterSource.class), any(RowMapper.class))) + .thenReturn(List.of()); + Instant startTime = startStr == null ? null : Instant.parse(startStr); + Instant endTime = endStr == null ? null : Instant.parse(endStr); + + service.listAuditLogs(0, 20, null, null, null, null, null, null, startTime, endTime); + + ArgumentCaptor paramsCaptor = ArgumentCaptor.forClass(MapSqlParameterSource.class); + verify(jdbcTemplate).query(contains("FROM audit_log"), paramsCaptor.capture(), any(RowMapper.class)); + MapSqlParameterSource params = paramsCaptor.getValue(); + if (startTime != null) { + assertThat(params.getValue("startTime")) + .isEqualTo(OffsetDateTime.ofInstant(startTime, ZoneOffset.UTC)); + } else { + assertThat(params.hasValue("startTime")).isFalse(); + } + if (endTime != null) { + assertThat(params.getValue("endTime")) + .isEqualTo(OffsetDateTime.ofInstant(endTime, ZoneOffset.UTC)); + } else { + assertThat(params.hasValue("endTime")).isFalse(); + } + } + + @Test + void rowMapper_isIndependentOfJvmDefaultTimezone() throws Exception { + TimeZone original = TimeZone.getDefault(); + try { + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Shanghai")); + RowMapper rowMapper = captureRowMapper(); + ResultSet rs = stubRowWithCreatedAt( + OffsetDateTime.of(2026, 5, 29, 8, 53, 0, 0, ZoneOffset.UTC)); + + AuditLogItemResponse item = rowMapper.mapRow(rs, 0); + + assertThat(item).isNotNull(); + assertThat(item.timestamp()).isEqualTo(Instant.parse("2026-05-29T08:53:00Z")); + verify(rs, never()).getTimestamp(anyString()); + } finally { + TimeZone.setDefault(original); + } + } + + @SuppressWarnings("unchecked") + private RowMapper captureRowMapper() { + when(jdbcTemplate.queryForObject(contains("COUNT(*)"), any(MapSqlParameterSource.class), eq(Long.class))) + .thenReturn(0L); + ArgumentCaptor> captor = ArgumentCaptor.forClass(RowMapper.class); + when(jdbcTemplate.query(contains("FROM audit_log"), any(MapSqlParameterSource.class), captor.capture())) + .thenReturn(List.of()); + service.listAuditLogs(0, 20, null, null, null, null, null, null, null, null); + return captor.getValue(); + } + + private static ResultSet stubRowWithCreatedAt(OffsetDateTime createdAt) throws Exception { + ResultSet rs = mock(ResultSet.class); + when(rs.getLong("id")).thenReturn(1L); + when(rs.getString("action")).thenReturn("PROMOTION_SUBMIT"); + when(rs.getString("actor_user_id")).thenReturn("user-1"); + when(rs.getString("display_name")).thenReturn("alice"); + when(rs.getString("detail_json")).thenReturn("{}"); + when(rs.getString("target_type")).thenReturn("PROMOTION"); + when(rs.getObject("target_id")).thenReturn(42L); + when(rs.getString("client_ip")).thenReturn("127.0.0.1"); + when(rs.getString("request_id")).thenReturn("req-1"); + when(rs.getObject("created_at", OffsetDateTime.class)).thenReturn(createdAt); + return rs; + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java index 1bb04758..7c8bbb3e 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java @@ -75,7 +75,8 @@ class MySkillAppServiceTest { skillStarRepository, skillSubscriptionRepository, mySkillQueryRepository, - skillLifecycleProjectionService + skillLifecycleProjectionService, + namespaceRepository ); } @@ -273,9 +274,8 @@ class MySkillAppServiceTest { given(skillRepository.findByOwnerId("user-1", PageRequest.of(0, 10))) .willReturn(new PageImpl<>(List.of(skill), PageRequest.of(0, 10), 1)); + given(skillVersionRepository.findBySkillIdAndStatus(6L, SkillVersionStatus.PUBLISHED)).willReturn(List.of(publishedVersion)); given(skillVersionRepository.findBySkillId(6L)).willReturn(List.of(rejectedVersion, publishedVersion)); - given(skillVersionRepository.findBySkillIdAndStatus(6L, SkillVersionStatus.PUBLISHED)) - .willReturn(List.of(publishedVersion)); given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); var result = service.listMySkills("user-1", 0, 10); @@ -286,6 +286,84 @@ class MySkillAppServiceTest { assertThat(result.items().get(0).ownerPreviewVersion()).isNull(); } + @Test + void listMySkills_filtersByKeywordAcrossDisplayNameSlugAndSummary() { + Skill alpha = createSkill(1L, 101L, "alpha-tool", "user-1"); + alpha.setDisplayName("Alpha Assistant"); + Skill beta = createSkill(2L, 101L, "beta-tool", "user-1"); + beta.setDisplayName("Beta Tool"); + beta.setSummary("This tool helps with alpha testing"); + Skill gamma = createSkill(3L, 101L, "gamma-tool", "user-1"); + gamma.setDisplayName("Gamma Service"); + SkillVersion publishedVersion = createVersion(1L, 10L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(alpha, beta, gamma)); + given(skillVersionRepository.findBySkillId(1L)).willReturn(List.of(publishedVersion)); + given(skillVersionRepository.findBySkillId(2L)).willReturn(List.of()); + given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); + + var result = service.listMySkills("user-1", 0, 10, null, "alpha", null, Set.of("USER")); + + assertThat(result.total()).isEqualTo(2); + assertThat(result.items()).extracting("slug") + .containsExactlyInAnyOrder("alpha-tool", "beta-tool"); + } + + @Test + void listMySkills_filtersByNamespaceSlug() { + Skill aiSkill = createSkill(1L, 101L, "ai-tool", "user-1"); + Skill mlSkill = createSkill(2L, 102L, "ml-tool", "user-1"); + SkillVersion v1 = createVersion(1L, 10L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(aiSkill, mlSkill)); + given(skillVersionRepository.findBySkillId(1L)).willReturn(List.of(v1)); + given(namespaceRepository.findBySlug("team-ai")).willReturn(java.util.Optional.of(namespace(101L, "team-ai"))); + given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); + + var result = service.listMySkills("user-1", 0, 10, null, null, "team-ai", Set.of("USER")); + + assertThat(result.total()).isEqualTo(1); + assertThat(result.items()).extracting("slug").containsExactly("ai-tool"); + } + + @Test + void listMySkills_returnsEmptyWhenNamespaceSlugNotFound() { + Skill skill = createSkill(1L, 101L, "ai-tool", "user-1"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(skill)); + given(namespaceRepository.findBySlug("missing-namespace")).willReturn(java.util.Optional.empty()); + + var result = service.listMySkills("user-1", 0, 10, null, null, "missing-namespace", Set.of("USER")); + + assertThat(result.total()).isZero(); + assertThat(result.items()).isEmpty(); + } + + @Test + void listMySkills_combinesKeywordNamespaceAndStatusFilters() { + Skill aiAlpha = createSkill(1L, 101L, "ai-alpha", "user-1"); + aiAlpha.setDisplayName("AI Alpha"); + Skill aiBeta = createSkill(2L, 101L, "ai-beta", "user-1"); + aiBeta.setDisplayName("AI Beta"); + Skill mlAlpha = createSkill(3L, 102L, "ml-alpha", "user-1"); + mlAlpha.setDisplayName("ML Alpha"); + SkillVersion v1 = createVersion(1L, 10L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + SkillVersion v2 = createVersion(2L, 20L, "1.0.0", SkillVersionStatus.REJECTED, "2026-03-15T09:30:00Z"); + SkillVersion v3 = createVersion(3L, 30L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(aiAlpha, aiBeta, mlAlpha)); + given(skillVersionRepository.findBySkillIdAndStatus(1L, SkillVersionStatus.PUBLISHED)).willReturn(List.of(v1)); + given(skillVersionRepository.findBySkillId(1L)).willReturn(List.of(v1)); + given(namespaceRepository.findBySlug("team-ai")).willReturn(java.util.Optional.of(namespace(101L, "team-ai"))); + given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); + + var result = service.listMySkills("user-1", 0, 10, "PUBLISHED", "alpha", "team-ai", Set.of("USER")); + + assertThat(result.total()).isEqualTo(1); + assertThat(result.items()).extracting("slug").containsExactly("ai-alpha"); + } + + private Skill createSkill(Long id, Long namespaceId, String slug, String ownerId) { Skill skill = new Skill(namespaceId, slug, ownerId, SkillVisibility.PUBLIC); skill.setDisplayName(slug); diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java index 0206e395..8499ee4d 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java @@ -73,6 +73,23 @@ class RouteSecurityPolicyRegistryTest { assertTrue(matchedWeb); } + @Test + void authorizationPolicies_shouldNotDeclareNamespaceBundleDownloadRoutes() { + String v1Route = "/api/v1/namespaces/*/skills/" + "download"; + String webRoute = "/api/web/namespaces/*/skills/" + "download"; + boolean matchedV1 = registry.authorizationPolicies().stream() + .anyMatch(policy -> policy.method() == HttpMethod.GET + && v1Route.equals(policy.pattern())); + boolean matchedWeb = registry.authorizationPolicies().stream() + .anyMatch(policy -> policy.method() == HttpMethod.GET + && webRoute.equals(policy.pattern())); + + assertFalse(matchedV1); + assertFalse(matchedWeb); + assertFalse(registry.authorizeApiToken("GET", "/api/v1/namespaces/global/skills/" + "download", Set.of()).allowed()); + assertFalse(registry.authorizeApiToken("GET", "/api/web/namespaces/global/skills/" + "download", Set.of()).allowed()); + } + @Test void apiTokenPolicySupportsNativeCliRoutes() { assertTrue(registry.authorizeApiToken("GET", "/api/cli/v1/auth/whoami", Set.of()).allowed()); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java index 3bb194ff..b53a6c63 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java @@ -4,7 +4,6 @@ import com.iflytek.skillhub.domain.event.SkillDownloadedEvent; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.namespace.NamespaceType; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.skill.*; @@ -169,16 +168,20 @@ public class SkillDownloadService { // Only increment download count for PUBLISHED versions if (version.getStatus() == SkillVersionStatus.PUBLISHED) { - skillRepository.incrementDownloadCount(skill.getId()); - skillVersionStatsRepository.incrementDownloadCount(version.getId(), skill.getId()); - eventPublisher.publishEvent(new SkillDownloadedEvent(skill.getId(), version.getId())); + recordPublishedDownload(skill, version); } return result; } + private void recordPublishedDownload(Skill skill, SkillVersion version) { + skillRepository.incrementDownloadCount(skill.getId()); + skillVersionStatsRepository.incrementDownloadCount(version.getId(), skill.getId()); + eventPublisher.publishEvent(new SkillDownloadedEvent(skill.getId(), version.getId())); + } + private DownloadResult buildDownloadResult(Skill skill, SkillVersion version) { - String storageKey = String.format("packages/%d/%d/bundle.zip", skill.getId(), version.getId()); + String storageKey = buildBundleStorageKey(skill, version); DownloadResult result; if (objectStorageService.exists(storageKey)) { @@ -205,6 +208,10 @@ public class SkillDownloadService { return result; } + private String buildBundleStorageKey(Skill skill, SkillVersion version) { + return String.format("packages/%d/%d/bundle.zip", skill.getId(), version.getId()); + } + private DownloadResult buildBundleFromFiles(Skill skill, SkillVersion version) { List files = skillFileRepository.findByVersionId(version.getId()).stream() .filter(file -> objectStorageService.exists(file.getStorageKey())) @@ -268,7 +275,7 @@ public class SkillDownloadService { Skill skill, String currentUserId, Map userNsRoles) { - if (currentUserId == null && !isAnonymousDownloadAllowed(namespace, skill)) { + if (currentUserId == null && !isAnonymousDownloadAllowed(skill)) { throw new DomainForbiddenException("error.skill.access.denied", skill.getSlug()); } if (!visibilityChecker.canAccess(skill, currentUserId, userNsRoles)) { @@ -276,9 +283,8 @@ public class SkillDownloadService { } } - private boolean isAnonymousDownloadAllowed(Namespace namespace, Skill skill) { - return namespace.getType() == NamespaceType.GLOBAL - && skill.getVisibility() == SkillVisibility.PUBLIC; + private boolean isAnonymousDownloadAllowed(Skill skill) { + return skill.getVisibility() == SkillVisibility.PUBLIC; } private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index 820bbba9..bbb748f8 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -182,12 +182,15 @@ public class SkillGovernanceService { deleteStorageAfterCommit(skill, namespaceSlug, storageKeys); skillFileRepository.deleteByVersionId(version.getId()); securityScanService.softDeleteByVersionId(version.getId()); - skillVersionRepository.delete(version); + // FK 约束 fk_skill_latest_version 阻止删除 skill_version 当 skill.latest_version_id 还指向它。 + // 必须先解开引用并 flush,让 PG 在 delete 时看不到引用。 if (version.getId().equals(skill.getLatestVersionId())) { skill.setLatestVersionId(findLatestPublishedVersionId(skill.getId())); skill.setUpdatedBy(actorUserId); skillRepository.save(skill); + skillRepository.flush(); } + skillVersionRepository.delete(version); auditLogService.record( actorUserId, "DELETE_SKILL_VERSION", diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java index 6c9c4ce5..368ae850 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java @@ -67,9 +67,9 @@ public class SkillLifecycleProjectionService { VersionProjection publishedVersion = toProjection(published); VersionProjection ownerPreviewVersion = toProjection(preview); VersionProjection headlineVersion = publishedVersion != null ? publishedVersion : ownerPreviewVersion; - ResolutionMode resolutionMode = published != null ? ResolutionMode.PUBLISHED - : preview != null ? ResolutionMode.OWNER_PREVIEW - : ResolutionMode.NONE; + ResolutionMode resolutionMode = headlineVersion == null ? ResolutionMode.NONE + : publishedVersion != null ? ResolutionMode.PUBLISHED + : ResolutionMode.OWNER_PREVIEW; return new Projection(headlineVersion, publishedVersion, ownerPreviewVersion, resolutionMode); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java index fed90cfc..d4271bf8 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java @@ -564,6 +564,14 @@ public class SkillPublishService { throw new DomainBadRequestException("error.skill.version.exists", version.getVersion()); } + // FK 约束 fk_skill_latest_version 阻止删除 skill_version 当 skill.latest_version_id 还指向它。 + // 必须先解开引用并 flush,让 PG 在 delete 时看不到引用。 + if (version.getId().equals(skill.getLatestVersionId())) { + skill.setLatestVersionId(null); + skillRepository.save(skill); + skillRepository.flush(); + } + reviewTaskRepository.findBySkillVersionIdAndStatus(version.getId(), ReviewTaskStatus.PENDING) .ifPresent(reviewTaskRepository::delete); @@ -579,10 +587,6 @@ public class SkillPublishService { securityScanService.softDeleteByVersionId(version.getId()); skillVersionRepository.delete(version); skillVersionRepository.flush(); - - if (version.getId().equals(skill.getLatestVersionId())) { - skill.setLatestVersionId(null); - } } private String resolveNamespaceSlug(Long namespaceId) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackagePolicy.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackagePolicy.java index eab8d2e2..ee3880ae 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackagePolicy.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackagePolicy.java @@ -64,7 +64,19 @@ public final class SkillPackagePolicy { throw new IllegalArgumentException("Package entry path must be normalized: " + rawPath); } - return canonical; + return canonicalizeSkillMdPath(canonical); + } + + public static String canonicalizeSkillMdPath(String normalizedPath) { + int slashIndex = normalizedPath.lastIndexOf('/'); + String fileName = slashIndex >= 0 ? normalizedPath.substring(slashIndex + 1) : normalizedPath; + if (!SKILL_MD_PATH.equalsIgnoreCase(fileName)) { + return normalizedPath; + } + if (slashIndex < 0) { + return SKILL_MD_PATH; + } + return normalizedPath.substring(0, slashIndex + 1) + SKILL_MD_PATH; } public static boolean hasAllowedExtension(String path) { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java index ba24003b..4bc20d34 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java @@ -6,7 +6,6 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceType; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; -import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.skill.*; import com.iflytek.skillhub.storage.ObjectMetadata; import com.iflytek.skillhub.storage.ObjectStorageService; @@ -318,26 +317,38 @@ class SkillDownloadServiceTest { } @Test - void testDownloadVersion_RejectsAnonymousForTeamNamespacePublicSkill() throws Exception { + void testDownloadVersion_AllowsAnonymousForTeamNamespacePublicSkill() throws Exception { Namespace namespace = new Namespace("team-ai", "Team AI", "owner-1"); setId(namespace, 2L); namespace.setType(NamespaceType.TEAM); Skill skill = new Skill(2L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); setId(skill, 1L); + skill.setDisplayName("Demo Skill"); skill.setStatus(SkillStatus.ACTIVE); skill.setLatestVersionId(10L); + SkillVersion version = new SkillVersion(1L, "1.0.0", "owner-1"); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + when(namespaceRepository.findBySlug("team-ai")).thenReturn(Optional.of(namespace)); when(skillRepository.findByNamespaceIdAndSlug(2L, "demo-skill")).thenReturn(List.of(skill)); + when(visibilityChecker.canAccess(skill, null, Map.of())).thenReturn(true); + when(skillVersionRepository.findBySkillIdAndVersion(1L, "1.0.0")).thenReturn(Optional.of(version)); + when(objectStorageService.exists("packages/1/10/bundle.zip")).thenReturn(false); + when(skillFileRepository.findByVersionId(10L)).thenReturn(List.of( + new SkillFile(10L, "SKILL.md", 4L, "text/markdown", "hash", "skills/1/10/SKILL.md"))); + when(objectStorageService.exists("skills/1/10/SKILL.md")).thenReturn(true); + when(objectStorageService.getObject("skills/1/10/SKILL.md")).thenReturn(new ByteArrayInputStream("test".getBytes())); - assertThrows(DomainForbiddenException.class, () -> - service.downloadVersion("team-ai", "demo-skill", "1.0.0", null, Map.of())); + SkillDownloadService.DownloadResult result = service.downloadVersion("team-ai", "demo-skill", "1.0.0", null, Map.of()); - verify(visibilityChecker, never()).canAccess(any(), any(), anyMap()); - verify(skillRepository, never()).incrementDownloadCount(anyLong()); - verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); - verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + assertNotNull(result); + assertEquals("Demo Skill-1.0.0.zip", result.filename()); + verify(skillRepository).incrementDownloadCount(1L); + verify(skillVersionStatsRepository).incrementDownloadCount(10L, 1L); + verify(eventPublisher).publishEvent(any(SkillDownloadedEvent.class)); } private void setId(Object entity, Long id) throws Exception { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java index d473a361..c2b6b63c 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java @@ -40,6 +40,35 @@ class SkillPackageValidatorTest { assertTrue(result.errors().isEmpty()); } + @Test + void normalizesSkillMdFilenameCase() { + assertEquals("SKILL.md", SkillPackagePolicy.normalizeEntryPath("skill.md")); + assertEquals("SKILL.md", SkillPackagePolicy.normalizeEntryPath("Skill.MD")); + assertEquals("nested/SKILL.md", SkillPackagePolicy.normalizeEntryPath("nested/skill.md")); + } + + @Test + void acceptsSkillMdFilenameWithDifferentCase() { + String skillMdContent = """ + --- + name: test-skill + description: A test skill + version: 1.0.0 + --- + # Test Skill + """; + + List entries = List.of( + new PackageEntry("skill.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown"), + new PackageEntry("README.md", "readme".getBytes(), 6, "text/markdown") + ); + + ValidationResult result = validator.validate(entries); + + assertTrue(result.passed()); + assertTrue(result.errors().isEmpty()); + } + @Test void testMissingSkillMd() { List entries = List.of( diff --git a/web/e2e/helpers/test-data-builder.ts b/web/e2e/helpers/test-data-builder.ts index f2cb47d6..a6b77adc 100644 --- a/web/e2e/helpers/test-data-builder.ts +++ b/web/e2e/helpers/test-data-builder.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { execFileSync } from 'node:child_process' import path from 'node:path' @@ -65,6 +65,11 @@ export interface SeedSkillOptions { description?: string version?: string readmeHeading?: string + readmeBody?: string + extraFiles?: Array<{ + path: string + content: string + }> } function asApiErrorBody(value: unknown): string { @@ -130,8 +135,13 @@ function buildSkillPackageZipBuffer(suffix: string, options?: SeedSkillOptions): execFileSync('mkdir', ['-p', packageDir]) writeFileSync(path.join(packageDir, 'SKILL.md'), skillMd, 'utf8') - writeFileSync(path.join(packageDir, 'README.md'), `# ${readmeHeading}\n`, 'utf8') - execFileSync('zip', ['-q', '-r', zipPath, 'SKILL.md', 'README.md'], { cwd: packageDir }) + writeFileSync(path.join(packageDir, 'README.md'), options?.readmeBody ?? `# ${readmeHeading}\n`, 'utf8') + for (const extraFile of options?.extraFiles ?? []) { + const targetPath = path.join(packageDir, extraFile.path) + mkdirSync(path.dirname(targetPath), { recursive: true }) + writeFileSync(targetPath, extraFile.content, 'utf8') + } + execFileSync('zip', ['-q', '-r', zipPath, '.'], { cwd: packageDir }) return readFileSync(zipPath) } finally { rmSync(tempRoot, { recursive: true, force: true }) @@ -146,8 +156,13 @@ function createSkillPackageZipFile(suffix: string, options?: SeedSkillOptions): execFileSync('mkdir', ['-p', packageDir]) writeFileSync(path.join(packageDir, 'SKILL.md'), skillMd, 'utf8') - writeFileSync(path.join(packageDir, 'README.md'), `# ${readmeHeading}\n`, 'utf8') - execFileSync('zip', ['-q', '-r', zipPath, 'SKILL.md', 'README.md'], { cwd: packageDir }) + writeFileSync(path.join(packageDir, 'README.md'), options?.readmeBody ?? `# ${readmeHeading}\n`, 'utf8') + for (const extraFile of options?.extraFiles ?? []) { + const targetPath = path.join(packageDir, extraFile.path) + mkdirSync(path.dirname(targetPath), { recursive: true }) + writeFileSync(targetPath, extraFile.content, 'utf8') + } + execFileSync('zip', ['-q', '-r', zipPath, '.'], { cwd: packageDir }) return { filePath: zipPath, diff --git a/web/e2e/namespace-search.spec.ts b/web/e2e/namespace-search.spec.ts new file mode 100644 index 00000000..39a48ec1 --- /dev/null +++ b/web/e2e/namespace-search.spec.ts @@ -0,0 +1,106 @@ +import { expect, test, type Page } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' +import { E2eTestDataBuilder } from './helpers/test-data-builder' + +function waitForSkillSearch(page: Page, options: { namespace?: string; q?: string; sort?: string }) { + return page.waitForResponse((response) => { + if (!response.ok() || !response.url().includes('/api/web/skills?')) { + return false + } + + const url = new URL(response.url()) + const namespace = url.searchParams.get('namespace') ?? '' + const query = url.searchParams.get('q') ?? '' + const sort = url.searchParams.get('sort') ?? '' + + return namespace === (options.namespace ?? '') + && query === (options.q ?? '') + && (!options.sort || sort === options.sort) + }) +} + +test.describe('Namespace Search (Real API)', () => { + test.beforeEach(async ({ page }) => { + await setEnglishLocale(page) + await page.context().setExtraHTTPHeaders({ + 'X-Mock-User-Id': 'local-admin', + }) + }) + + test('submits @namespace keyword search and clears the namespace filter', async ({ page }, testInfo) => { + const builder = new E2eTestDataBuilder(page, testInfo) + await builder.init() + + try { + const namespace = await builder.createNamespace('e2e-pm-search') + const otherNamespace = await builder.createNamespace('e2e-dev-search') + const namespaceSkill = await builder.publishSkill(namespace.slug, { + name: 'roadmap-discovery', + description: 'Roadmap planning skill for namespace search regression.', + }) + const otherSkill = await builder.publishSkill(otherNamespace.slug, { + name: 'roadmap-backend', + description: 'Roadmap planning skill outside the selected namespace.', + }) + await builder.waitForSearchResults('roadmap', [namespaceSkill.slug, otherSkill.slug]) + + await page.goto('/search') + await page.getByPlaceholder('Search skills...').fill(`@${namespace.slug} roadmap`) + + const filteredSearch = waitForSkillSearch(page, { namespace: namespace.slug, q: 'roadmap' }) + await page.getByRole('button', { name: 'Search', exact: true }).click() + await filteredSearch + + await expect(page).toHaveURL(new RegExp(`namespace=${namespace.slug}`)) + await expect(page).toHaveURL(/q=roadmap/) + await expect(page.getByRole('button', { name: `@${namespace.slug}` })).toBeVisible() + await expect(page.getByRole('heading', { name: namespaceSkill.slug })).toBeVisible() + await expect(page.getByText(`@${otherNamespace.slug}`)).toHaveCount(0) + + await page.goto(`/search?q=roadmap&namespace=${namespace.slug}&sort=downloads&page=1&starredOnly=false`) + await expect(page.getByRole('button', { name: `@${namespace.slug}` })).toBeVisible() + + const unfilteredSearch = waitForSkillSearch(page, { q: 'roadmap', sort: 'downloads' }) + await page.getByRole('button', { name: `@${namespace.slug}` }).click() + await unfilteredSearch + + await expect(page).toHaveURL(/q=roadmap/) + await expect(page).toHaveURL(/sort=downloads/) + await expect(page).toHaveURL(/page=0/) + await expect(page).not.toHaveURL(new RegExp(`namespace=${namespace.slug}`)) + await expect(page.getByRole('heading', { name: namespaceSkill.slug })).toBeVisible() + await expect(page.getByRole('heading', { name: otherSkill.slug })).toBeVisible() + } finally { + await builder.cleanup() + } + }) + + test('supports a sixty-four character namespace slug in search input', async ({ page }, testInfo) => { + const builder = new E2eTestDataBuilder(page, testInfo) + await builder.init() + + try { + const namespace = await builder.createNamespace('e2e-namespace-64-slug-search-case-alphaab') + expect(namespace.slug).toHaveLength(64) + const skill = await builder.publishSkill(namespace.slug, { + name: 'boundary-search-agent', + description: 'Boundary namespace search regression skill.', + }) + await builder.waitForSearchResult('boundary', skill.slug) + + await page.goto('/search') + await page.getByPlaceholder('Search skills...').fill(`@${namespace.slug} boundary`) + + const filteredSearch = waitForSkillSearch(page, { namespace: namespace.slug, q: 'boundary' }) + await page.getByRole('button', { name: 'Search', exact: true }).click() + await filteredSearch + + await expect(page).toHaveURL(new RegExp(`namespace=${namespace.slug}`)) + await expect(page).toHaveURL(/q=boundary/) + await expect(page.getByRole('button', { name: `@${namespace.slug}` })).toBeVisible() + await expect(page.getByRole('heading', { name: skill.slug })).toBeVisible() + } finally { + await builder.cleanup() + } + }) +}) diff --git a/web/e2e/public-skill-detail-anonymous.spec.ts b/web/e2e/public-skill-detail-anonymous.spec.ts index 56ba8885..720c2796 100644 --- a/web/e2e/public-skill-detail-anonymous.spec.ts +++ b/web/e2e/public-skill-detail-anonymous.spec.ts @@ -11,6 +11,10 @@ function latestSeed(seed: PreparedSearchSeed) { } } +function escapeRegExp(value: string) { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + let seeded: PreparedSearchSeed | undefined test.describe('Public Skill Detail Anonymous Access (Real API)', () => { @@ -36,11 +40,25 @@ test.describe('Public Skill Detail Anonymous Access (Real API)', () => { await card.click() - await expect(page).toHaveURL(new RegExp(`/space/${current.skill.namespace}/${current.skill.slug}$`)) + await expect(page).toHaveURL(new RegExp(`/space/${current.skill.namespace}/${current.skill.slug}(\\?|$)`)) await expect(page).not.toHaveURL(/\/login\?returnTo=/) await expect(page.getByRole('heading', { name: current.skillName, exact: true })).toBeVisible() await expect(page.getByText('Install', { exact: true })).toBeVisible() - await expect(page.getByText(new RegExp(`npx clawhub install ${current.skill.slug}`))).toBeVisible() + const clawhubTarget = current.skill.namespace === 'global' + ? current.skill.slug + : `${current.skill.namespace}--${current.skill.slug}` + const skillhubNamespace = current.skill.namespace === 'global' + ? '' + : ` --namespace ${current.skill.namespace}` + + await expect(page.getByRole('tab', { name: 'ClawHub CLI' })).toHaveAttribute('aria-selected', 'true') + await expect(page.getByText(new RegExp(`npx clawhub install ${escapeRegExp(clawhubTarget)} --registry`))).toBeVisible() + await expect(page.getByRole('tab', { name: 'SkillHub CLI' })).toBeVisible() + + await page.getByRole('tab', { name: 'SkillHub CLI' }).click() + + await expect(page.getByRole('tab', { name: 'SkillHub CLI' })).toHaveAttribute('aria-selected', 'true') + await expect(page.getByText(new RegExp(`npx @astron-team/skillhub@latest install ${escapeRegExp(current.skill.slug)}${escapeRegExp(skillhubNamespace)} --registry`))).toBeVisible() await expect(page.getByRole('button', { name: 'Copy' }).first()).toBeVisible() }) }) diff --git a/web/e2e/reviews-pagination.spec.ts b/web/e2e/reviews-pagination.spec.ts index 2d18b30c..f9906a35 100644 --- a/web/e2e/reviews-pagination.spec.ts +++ b/web/e2e/reviews-pagination.spec.ts @@ -41,6 +41,7 @@ test.describe('Review Management Pagination (Real API)', () => { await page.goto('/dashboard/reviews') await expect(page.getByRole('heading', { name: 'Review Center' })).toBeVisible() + await expect(page.getByRole('tab', { name: 'Skill Reviews' })).toBeVisible() const tabMeta: Record = { PENDING: { tabLabel: 'Pending', summaryPrefix: 'Total' }, @@ -49,7 +50,7 @@ test.describe('Review Management Pagination (Real API)', () => { } for (const status of statuses) { - await page.getByRole('button', { name: tabMeta[status].tabLabel }).click() + await page.getByRole('tab', { name: tabMeta[status].tabLabel }).click() const meta = metaByStatus.get(status) if (!meta) { diff --git a/web/e2e/skill-detail-relative-links.spec.ts b/web/e2e/skill-detail-relative-links.spec.ts new file mode 100644 index 00000000..5f1f353c --- /dev/null +++ b/web/e2e/skill-detail-relative-links.spec.ts @@ -0,0 +1,55 @@ +import { expect, test } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' +import { registerSession } from './helpers/session' +import { E2eTestDataBuilder } from './helpers/test-data-builder' + +test.describe('Skill Detail Relative Links (Real API)', () => { + test.beforeEach(async ({ page }, testInfo) => { + await setEnglishLocale(page) + await registerSession(page, testInfo) + }) + + test('previews package files from overview relative links and reports missing files', async ({ page }, testInfo) => { + const builder = new E2eTestDataBuilder(page, testInfo) + await builder.init() + + try { + const namespace = await builder.ensureWritableNamespace() + const skillName = `relative-links-${Date.now().toString(36)}` + const skill = await builder.publishSkill(namespace.slug, { + name: skillName, + readmeBody: [ + `# ${skillName}`, + '', + '[Usage](docs/usage.md)', + '', + '[Missing](docs/missing.md)', + ].join('\n'), + extraFiles: [ + { + path: 'docs/usage.md', + content: '# Usage\n\nThis is linked documentation.', + }, + ], + }) + + await page.goto(`/space/${encodeURIComponent(namespace.slug)}/${encodeURIComponent(skill.slug)}`) + + await expect(page).toHaveURL(new RegExp(`/space/${namespace.slug}/${skill.slug}$`)) + await expect(page.getByRole('link', { name: 'Usage' })).toBeVisible() + await page.getByRole('link', { name: 'Usage' }).click() + await expect(page.getByRole('dialog')).toContainText('usage.md') + await expect(page.getByRole('dialog')).toContainText('This is linked documentation.') + + await page.getByRole('button', { name: 'Close' }).click() + await expect(page.getByRole('dialog')).toBeHidden() + + await page.getByRole('link', { name: 'Missing' }).click() + await expect(page).toHaveURL(new RegExp(`/space/${namespace.slug}/${skill.slug}$`)) + await expect(page.getByText('File not found')).toBeVisible() + await expect(page.getByText('not included in the current skill version')).toBeVisible() + } finally { + await builder.cleanup() + } + }) +}) diff --git a/web/e2e/skill-subscription.spec.ts b/web/e2e/skill-subscription.spec.ts index e14cda1d..c6d74d24 100644 --- a/web/e2e/skill-subscription.spec.ts +++ b/web/e2e/skill-subscription.spec.ts @@ -45,31 +45,14 @@ test.describe('Skill Subscription (Real API)', () => { const subscribeButton = page.getByRole('button', { name: /Subscribe/ }) await expect(subscribeButton).toBeVisible() - const initialCount = await subscribeButton.textContent() - const initialCountMatch = initialCount?.match(/\((\d+)\)/) - const initialCountValue = initialCountMatch ? Number.parseInt(initialCountMatch[1], 10) : 0 - await subscribeButton.click() await expect(page.getByRole('button', { name: /Subscribed/ })).toBeVisible() const subscribedButton = page.getByRole('button', { name: /Subscribed/ }) - const subscribedCount = await subscribedButton.textContent() - const subscribedCountMatch = subscribedCount?.match(/\((\d+)\)/) - const subscribedCountValue = subscribedCountMatch ? Number.parseInt(subscribedCountMatch[1], 10) : 0 - - expect(subscribedCountValue).toBe(initialCountValue + 1) - await subscribedButton.click() await expect(page.getByRole('button', { name: /Subscribe/ })).toBeVisible() - - const unsubscribedButton = page.getByRole('button', { name: /Subscribe/ }) - const unsubscribedCount = await unsubscribedButton.textContent() - const unsubscribedCountMatch = unsubscribedCount?.match(/\((\d+)\)/) - const unsubscribedCountValue = unsubscribedCountMatch ? Number.parseInt(unsubscribedCountMatch[1], 10) : 0 - - expect(unsubscribedCountValue).toBe(initialCountValue) } finally { await adminBuilder.cleanup() await adminContext.close() diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 3204d56a..16d2e7fe 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1024,13 +1024,19 @@ export const governanceApi = { } export const meApi = { - async getSkills(params?: { page?: number; size?: number; filter?: string }): Promise<{ items: SkillSummary[]; total: number; page: number; size: number }> { + async getSkills(params?: { page?: number; size?: number; filter?: string; q?: string; namespace?: string }): Promise<{ items: SkillSummary[]; total: number; page: number; size: number }> { const searchParams = new URLSearchParams() searchParams.set('page', String(params?.page ?? 0)) searchParams.set('size', String(params?.size ?? 10)) if (params?.filter) { searchParams.set('filter', params.filter) } + if (params?.q) { + searchParams.set('q', params.q) + } + if (params?.namespace) { + searchParams.set('namespace', params.namespace) + } return fetchJson<{ items: SkillSummary[]; total: number; page: number; size: number }>(`${WEB_API_PREFIX}/me/skills?${searchParams.toString()}`) }, diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 5f0a970a..9e056dff 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -916,6 +916,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/namespaces/{slug}/transfer-ownership": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["transferOwnership"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/namespaces/{slug}/transfer-ownership": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["transferOwnership_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/web/namespaces/{slug}/restore": { parameters: { query?: never; @@ -3685,6 +3717,21 @@ export interface components { /** Format: int64 */ targetNamespaceId?: number; }; + TransferOwnershipRequest: { + newOwnerId: string; + }; + ApiResponseMessageResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["MessageResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + MessageResponse: { + message?: string; + }; BatchMemberRequest: { members: components["schemas"]["MemberRequest"][]; }; @@ -3781,18 +3828,6 @@ export interface components { AuthorizeRequest: { userCode?: string; }; - ApiResponseMessageResponse: { - /** Format: int32 */ - code?: number; - msg?: string; - data?: components["schemas"]["MessageResponse"]; - /** Format: date-time */ - timestamp?: string; - requestId?: string; - }; - MessageResponse: { - message?: string; - }; SessionBootstrapRequest: { provider: string; }; @@ -3977,8 +4012,8 @@ export interface components { valid?: boolean; errors?: string[]; warnings?: string[]; - resolvedSlug?: string | null; - resolvedVersion?: string | null; + resolvedSlug?: string; + resolvedVersion?: string; }; UpdateProfileRequest: { displayName?: string; @@ -7035,6 +7070,58 @@ export interface operations { }; }; }; + transferOwnership: { + parameters: { + query?: never; + header?: never; + path: { + slug: string; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["TransferOwnershipRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseMessageResponse"]; + }; + }; + }; + }; + transferOwnership_1: { + parameters: { + query?: never; + header?: never; + path: { + slug: string; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["TransferOwnershipRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseMessageResponse"]; + }; + }; + }; + }; restoreNamespace: { parameters: { query?: never; diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index 8cabf0b3..d654e026 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -199,9 +199,10 @@ const searchRoute = createRoute({ getParentRoute: () => rootRoute, path: 'search', component: SearchPage, - validateSearch: (search: Record): { q: string; label?: string; sort: string; page: number; starredOnly: boolean } => { + validateSearch: (search: Record): { q: string; namespace?: string; label?: string; sort: string; page: number; starredOnly: boolean } => { return { q: normalizeSearchQuery(typeof search.q === 'string' ? search.q : ''), + namespace: typeof search.namespace === 'string' && search.namespace ? search.namespace.replace(/^@/, '') : undefined, label: typeof search.label === 'string' && search.label ? search.label : undefined, sort: (search.sort as string) || 'newest', page: Number(search.page) || 0, @@ -253,6 +254,12 @@ const dashboardSkillsRoute = createRoute({ getParentRoute: () => rootRoute, path: 'dashboard/skills', beforeLoad: requireAuth, + validateSearch: (search: Record): { page?: number; q?: string; namespace?: string; filter?: string } => ({ + page: typeof search.page === 'number' ? search.page : undefined, + q: typeof search.q === 'string' && search.q ? search.q : undefined, + namespace: typeof search.namespace === 'string' && search.namespace ? search.namespace : undefined, + filter: typeof search.filter === 'string' && search.filter ? search.filter : undefined, + }), component: MySkillsPage, }) diff --git a/web/src/docs/skill.md b/web/src/docs/skill.md index 18ea1e88..b8ed8bcf 100644 --- a/web/src/docs/skill.md +++ b/web/src/docs/skill.md @@ -138,7 +138,8 @@ If a request fails with `403`, check: ## Skill Package Contract -SkillHub expects OpenSkills-style packages with `SKILL.md` as the entry point. +SkillHub expects OpenSkills-style packages with canonical `SKILL.md` as the entry point. Uploads +accept filename case variants such as `skill.md` and normalize them to `SKILL.md`. ## Publishing Guidance diff --git a/web/src/features/search/search-bar.test.ts b/web/src/features/search/search-bar.test.ts index 199a607d..1ca7ec20 100644 --- a/web/src/features/search/search-bar.test.ts +++ b/web/src/features/search/search-bar.test.ts @@ -3,7 +3,7 @@ import * as mod from './search-bar' /** * search-bar.tsx exports the SearchBar component. The component delegates - * its max-length constraint to the shared MAX_SEARCH_QUERY_LENGTH constant + * its max-length constraint to the shared namespace-aware search input limit * (tested in search-query.test.ts). Controlled/uncontrolled mode logic and * submit/clear handlers are component-internal with no exported helpers. * diff --git a/web/src/features/search/search-bar.tsx b/web/src/features/search/search-bar.tsx index 94be3f25..4978a530 100644 --- a/web/src/features/search/search-bar.tsx +++ b/web/src/features/search/search-bar.tsx @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import { Loader2, Search, X } from 'lucide-react' -import { MAX_SEARCH_QUERY_LENGTH } from '@/shared/lib/search-query' +import { MAX_SEARCH_INPUT_LENGTH } from '@/shared/lib/search-query' import { Input } from '@/shared/ui/input' import { Button } from '@/shared/ui/button' @@ -59,7 +59,7 @@ export function SearchBar({ defaultValue = '', value, placeholder, isSearching = type="text" value={currentQuery} onChange={(e) => handleChange(e.target.value)} - maxLength={MAX_SEARCH_QUERY_LENGTH} + maxLength={MAX_SEARCH_INPUT_LENGTH} placeholder={placeholder || t('searchBar.placeholder')} className="pl-10 pr-10 border-0 bg-transparent focus-visible:ring-0 focus-visible:ring-offset-0 h-12" /> diff --git a/web/src/features/skill/install-command.test.ts b/web/src/features/skill/install-command.test.ts index 1f4daf50..60b8ee3a 100644 --- a/web/src/features/skill/install-command.test.ts +++ b/web/src/features/skill/install-command.test.ts @@ -1,7 +1,13 @@ import { createElement } from 'react' import { renderToStaticMarkup } from 'react-dom/server' import { afterEach, describe, expect, it, vi } from 'vitest' -import { InstallCommand, buildInstallCommand, buildInstallTarget, getBaseUrl } from './install-command' +import { + InstallCommand, + buildInstallCommand, + buildInstallTarget, + buildSkillhubInstallCommand, + getBaseUrl, +} from './install-command' vi.mock('react-i18next', () => ({ useTranslation: () => ({ @@ -62,6 +68,18 @@ describe('install-command', () => { ) }) + it('builds a one-line SkillHub npx command for the global namespace', () => { + expect(buildSkillhubInstallCommand('global', 'my-skill', 'https://skill.xfyun.cn')).toBe( + 'npx @astron-team/skillhub@latest install my-skill --registry https://skill.xfyun.cn', + ) + }) + + it('builds a one-line SkillHub npx command with namespace for team skills', () => { + expect(buildSkillhubInstallCommand('team-alpha', 'my-skill', 'https://skill.xfyun.cn')).toBe( + 'npx @astron-team/skillhub@latest install my-skill --namespace team-alpha --registry https://skill.xfyun.cn', + ) + }) + it('uses the runtime app base url when available', () => { setMockWindow('https://app.example.com') @@ -92,4 +110,33 @@ describe('install-command', () => { expect(html).toContain('leading-relaxed') expect(html).toContain('break-all') }) + + it('renders install method tabs with only a short active underline', () => { + setMockWindow('https://app.example.com') + + const html = renderToStaticMarkup(createElement(InstallCommand, { + namespace: 'global', + slug: 'meeting-minutes-generator', + })) + + expect(html).toContain('after:w-6') + expect(html).toContain('after:h-0.5') + expect(html).not.toContain('rounded-lg border bg-background/80 p-1') + expect(html).not.toContain('flex-1 rounded-md') + }) + + it('renders ClawHub CLI as the default install method', () => { + setMockWindow('https://app.example.com') + + const html = renderToStaticMarkup(createElement(InstallCommand, { + namespace: 'team-alpha', + slug: 'meeting-minutes-generator', + })) + + expect(html).toContain('skillDetail.installMethodClawhub') + expect(html).toContain('skillDetail.installMethodSkillhub') + expect(html).toContain('aria-selected="true"') + expect(html).toContain('npx clawhub install team-alpha--meeting-minutes-generator --registry https://app.example.com') + expect(html).not.toContain('npx @astron-team/skillhub@latest install meeting-minutes-generator --namespace team-alpha --registry https://app.example.com') + }) }) diff --git a/web/src/features/skill/install-command.tsx b/web/src/features/skill/install-command.tsx index f9989abb..3f409b9d 100644 --- a/web/src/features/skill/install-command.tsx +++ b/web/src/features/skill/install-command.tsx @@ -2,6 +2,7 @@ import { useMemo } from 'react' import { useTranslation } from 'react-i18next' import { Check, Copy } from 'lucide-react' import { Button } from '@/shared/ui/button' +import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/shared/ui/tabs' import { useCopyToClipboard } from '@/shared/lib/clipboard' interface InstallCommandProps { @@ -33,14 +34,22 @@ export function buildInstallCommand(namespace: string, slug: string, baseUrl: st return `npx clawhub install ${installTarget} --registry ${baseUrl}` } -export function InstallCommand({ namespace, slug }: InstallCommandProps) { +export function buildSkillhubInstallCommand(namespace: string, slug: string, baseUrl: string): string { + const namespaceArg = namespace === 'global' ? '' : ` --namespace ${namespace}` + return `npx @astron-team/skillhub@latest install ${slug}${namespaceArg} --registry ${baseUrl}` +} + +interface CommandBlockProps { + command: string +} + +const installMethodTabTriggerClass = + "relative border-b-0 px-1 py-2 text-xs after:absolute after:bottom-[-1px] after:left-1/2 after:h-0.5 after:w-6 after:-translate-x-1/2 after:rounded-full after:bg-transparent after:content-[''] data-[state=active]:after:bg-primary" + +function CommandBlock({ command }: CommandBlockProps) { const { t } = useTranslation() const [copied, copy] = useCopyToClipboard() - const baseUrl = useMemo(() => getBaseUrl(), []) - - const command = useMemo(() => buildInstallCommand(namespace, slug, baseUrl), [baseUrl, namespace, slug]) - const handleCopy = async () => { try { await copy(command) @@ -70,3 +79,29 @@ export function InstallCommand({ namespace, slug }: InstallCommandProps) { ) } + +export function InstallCommand({ namespace, slug }: InstallCommandProps) { + const { t } = useTranslation() + const baseUrl = useMemo(() => getBaseUrl(), []) + const clawhubCommand = useMemo(() => buildInstallCommand(namespace, slug, baseUrl), [baseUrl, namespace, slug]) + const skillhubCommand = useMemo(() => buildSkillhubInstallCommand(namespace, slug, baseUrl), [baseUrl, namespace, slug]) + + return ( + + + + {t('skillDetail.installMethodClawhub')} + + + {t('skillDetail.installMethodSkillhub')} + + + + + + + + + + ) +} diff --git a/web/src/features/skill/markdown-renderer.test.tsx b/web/src/features/skill/markdown-renderer.test.tsx index d7f39c4c..20fe3413 100644 --- a/web/src/features/skill/markdown-renderer.test.tsx +++ b/web/src/features/skill/markdown-renderer.test.tsx @@ -1,5 +1,10 @@ -import { describe, expect, it } from 'vitest' -import { MARKDOWN_IMAGE_CLASS_NAME } from './markdown-renderer' +/** @vitest-environment jsdom */ + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { MARKDOWN_IMAGE_CLASS_NAME, MarkdownRenderer } from './markdown-renderer' + +afterEach(() => cleanup()) describe('MARKDOWN_IMAGE_CLASS_NAME', () => { it('keeps markdown images at their intrinsic width while remaining responsive', () => { @@ -10,3 +15,21 @@ describe('MARKDOWN_IMAGE_CLASS_NAME', () => { expect(classNames).not.toContain('w-full') }) }) + +describe('MarkdownRenderer links', () => { + it('passes the raw markdown href to the optional link click handler', () => { + const onLinkClick = vi.fn() + + render() + fireEvent.click(screen.getByRole('link', { name: 'Usage' })) + + expect(onLinkClick).toHaveBeenCalledTimes(1) + expect(onLinkClick.mock.calls[0][0]).toBe('docs/usage.md') + }) + + it('keeps links renderable without a click handler', () => { + render() + + expect(screen.getByRole('link', { name: 'Usage' }).getAttribute('href')).toBe('docs/usage.md') + }) +}) diff --git a/web/src/features/skill/markdown-renderer.tsx b/web/src/features/skill/markdown-renderer.tsx index 1f787021..8bb0c198 100644 --- a/web/src/features/skill/markdown-renderer.tsx +++ b/web/src/features/skill/markdown-renderer.tsx @@ -1,4 +1,4 @@ -import { useMemo } from 'react' +import { useMemo, type MouseEvent } from 'react' import ReactMarkdown from 'react-markdown' import rehypeHighlight from 'rehype-highlight' import rehypeSanitize from 'rehype-sanitize' @@ -12,6 +12,7 @@ export const MARKDOWN_IMAGE_CLASS_NAME = 'h-auto max-w-full' interface MarkdownRendererProps { content: string className?: string + onLinkClick?: (href: string, event: MouseEvent) => void } /** @@ -20,7 +21,7 @@ interface MarkdownRendererProps { * dedicated UI sections and should not appear twice in the document body. * Memoized to prevent re-parsing on every render. */ -export function MarkdownRenderer({ content, className }: MarkdownRendererProps) { +export function MarkdownRenderer({ content, className, onLinkClick }: MarkdownRendererProps) { const containerClassName = [ className, 'max-w-none break-words text-sm text-foreground/90 [overflow-wrap:anywhere]', @@ -45,13 +46,15 @@ export function MarkdownRenderer({ content, className }: MarkdownRendererProps) {children}

), - a: ({ className: linkClassName, children, ...props }) => ( + a: ({ className: linkClassName, children, href, ...props }) => ( onLinkClick?.(href ?? '', event)} > {children} diff --git a/web/src/features/skill/package-relative-link.test.ts b/web/src/features/skill/package-relative-link.test.ts new file mode 100644 index 00000000..737be487 --- /dev/null +++ b/web/src/features/skill/package-relative-link.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from 'vitest' +import type { SkillFile } from '@/api/types' +import { resolvePackageRelativeLink } from './package-relative-link' + +function file(filePath: string): SkillFile { + return { + id: filePath.length, + filePath, + fileSize: 128, + contentType: 'text/markdown', + sha256: `sha-${filePath}`, + } +} + +const packageFiles = [ + file('README.md'), + file('docs/SKILL.md'), + file('docs/usage.md'), + file('shared.md'), + file('space name.md'), + file('使用.md'), +] + +describe('resolvePackageRelativeLink', () => { + it('matches same-directory and explicit current-directory links from the package root', () => { + expect(resolvePackageRelativeLink('docs/usage.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'docs/usage.md', + }) + expect(resolvePackageRelativeLink('./docs/usage.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'docs/usage.md', + }) + }) + + it('normalizes parent-directory links against the current documentation file', () => { + expect(resolvePackageRelativeLink('../shared.md', 'docs/SKILL.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'shared.md', + }) + }) + + it('keeps fragment information while matching the file path', () => { + expect(resolvePackageRelativeLink('docs/usage.md#intro', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'docs/usage.md', + fragment: 'intro', + }) + }) + + it('decodes encoded file paths before matching package files', () => { + expect(resolvePackageRelativeLink('space%20name.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'space name.md', + }) + expect(resolvePackageRelativeLink('%E4%BD%BF%E7%94%A8.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: '使用.md', + }) + }) + + it('ignores links that should keep native browser behavior', () => { + for (const href of ['https://example.com', 'mailto:team@example.com', '#intro', '/absolute/path.md', '']) { + expect(resolvePackageRelativeLink(href, 'README.md', packageFiles)).toMatchObject({ + status: 'ignored', + }) + } + }) + + it('returns missing for relative links that do not resolve to a package file', () => { + expect(resolvePackageRelativeLink('docs/missing.md', 'README.md', packageFiles)).toMatchObject({ + status: 'missing', + path: 'docs/missing.md', + }) + expect(resolvePackageRelativeLink('../../outside.md', 'docs/SKILL.md', packageFiles)).toMatchObject({ + status: 'missing', + path: null, + }) + }) +}) diff --git a/web/src/features/skill/package-relative-link.ts b/web/src/features/skill/package-relative-link.ts new file mode 100644 index 00000000..15c7bb25 --- /dev/null +++ b/web/src/features/skill/package-relative-link.ts @@ -0,0 +1,112 @@ +import type { SkillFile } from '@/api/types' + +export type PackageRelativeLinkResolution = + | { + status: 'ignored' + href: string + } + | { + status: 'matched' + href: string + path: string + fragment: string | null + file: SkillFile + } + | { + status: 'missing' + href: string + path: string | null + fragment: string | null + } + +function splitHref(href: string) { + const hashIndex = href.indexOf('#') + const beforeHash = hashIndex >= 0 ? href.slice(0, hashIndex) : href + const fragment = hashIndex >= 0 ? href.slice(hashIndex + 1) : null + const queryIndex = beforeHash.indexOf('?') + return { + path: queryIndex >= 0 ? beforeHash.slice(0, queryIndex) : beforeHash, + fragment, + } +} + +function decodePath(path: string) { + try { + return decodeURIComponent(path) + } catch { + return path + } +} + +function directoryOf(filePath?: string | null) { + if (!filePath) { + return '' + } + const normalized = filePath.replace(/^\/+/, '') + const lastSlash = normalized.lastIndexOf('/') + return lastSlash >= 0 ? normalized.slice(0, lastSlash) : '' +} + +function normalizePackagePath(baseDirectory: string, relativePath: string) { + const stack: string[] = [] + const rawParts = [...baseDirectory.split('/'), ...relativePath.split('/')] + + for (const part of rawParts) { + if (!part || part === '.') { + continue + } + if (part === '..') { + if (stack.length === 0) { + return null + } + stack.pop() + continue + } + stack.push(part) + } + + return stack.join('/') +} + +function shouldIgnoreLink(href: string, rawPath: string) { + if (!href.trim()) { + return true + } + if (!rawPath || href.startsWith('#')) { + return true + } + if (rawPath.startsWith('/') || rawPath.startsWith('//')) { + return true + } + return /^[a-z][a-z0-9+.-]*:/i.test(rawPath) +} + +export function resolvePackageRelativeLink( + href: string, + currentFilePath: string | null | undefined, + files: SkillFile[] | null | undefined, +): PackageRelativeLinkResolution { + const { path: rawPath, fragment } = splitHref(href) + + if (shouldIgnoreLink(href, rawPath)) { + return { status: 'ignored', href } + } + + const normalizedPath = normalizePackagePath(directoryOf(currentFilePath), decodePath(rawPath)) + if (!normalizedPath) { + return { status: 'missing', href, path: null, fragment } + } + + const matchedFile = (files ?? []).find((file) => file.filePath === normalizedPath) + if (!matchedFile) { + return { status: 'missing', href, path: normalizedPath, fragment } + } + + return { + status: 'matched', + href, + path: normalizedPath, + fragment, + file: matchedFile, + } +} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 3ff964a4..56039107 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -189,6 +189,7 @@ "noStarredResults": "No starred skills found", "noStarredResultsFor": "No starred skills match \"{{q}}\"", "noStarredSkills": "You have not starred any skills yet", + "namespaceFilter": "@{{namespace}}", "enterKeyword": "Please enter a search keyword", "results": "{{count}} skills found", "resultCount": "Found <1>{{count}} results", @@ -339,6 +340,12 @@ "mySkills": { "title": "My Skills", "subtitle": "Manage your published skills", + "searchPlaceholder": "Search by name, slug, or description", + "namespaceFilterLabel": "Filter by namespace", + "namespaceFilterAll": "All namespaces", + "clearSearch": "Clear filters", + "emptySearchTitle": "No matching skills", + "emptySearchDescription": "Try adjusting your keyword or switching namespace.", "filters": { "ALL": "All", "PENDING_REVIEW": "Pending Review", @@ -784,6 +791,8 @@ "documentationSource": "Source: {{path}}", "documentationUnavailableTitle": "Documentation is unavailable", "documentationUnavailable": "The documentation file could not be loaded. You can still inspect the package contents in the file list.", + "packageLinkMissingTitle": "File not found", + "packageLinkMissingDescription": "This link points to a file that is not included in the current skill version.", "authorLabel": "By {{name}}", "expandOverview": "Expand full overview", "collapseOverview": "Collapse content", @@ -801,6 +810,8 @@ "namespaceLabel": "Namespace", "loginToRate": "Login to star and rate", "install": "Install", + "installMethodClawhub": "ClawHub CLI", + "installMethodSkillhub": "SkillHub CLI", "download": "Download", "labelsSectionTitle": "Labels", "labelsSectionDescription": "Attach or remove recommended labels that help users filter and discover this skill.", @@ -1277,7 +1288,8 @@ "prev": "Previous", "next": "Next", "pagePrefix": "Page", - "pageSuffix": "" + "pageSuffix": "", + "goToPage": "Go to page {{page}}" }, "user": { "menu": { diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 85cdc73b..1920b158 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -189,6 +189,7 @@ "noStarredResults": "未找到已收藏技能", "noStarredResultsFor": "已收藏技能中没有与 \"{{q}}\" 相关的结果", "noStarredSkills": "你还没有收藏任何技能", + "namespaceFilter": "@{{namespace}}", "enterKeyword": "请输入搜索关键词", "results": "找到 {{count}} 个技能", "resultCount": "找到 <1>{{count}} 个结果", @@ -339,6 +340,12 @@ "mySkills": { "title": "我的技能", "subtitle": "管理你发布的技能", + "searchPlaceholder": "搜索技能名称、Slug 或描述", + "namespaceFilterLabel": "按命名空间过滤", + "namespaceFilterAll": "全部命名空间", + "clearSearch": "清除筛选", + "emptySearchTitle": "未找到匹配的技能", + "emptySearchDescription": "试试调整关键字或切换命名空间", "filters": { "ALL": "全部", "PENDING_REVIEW": "待审核", @@ -784,6 +791,8 @@ "documentationSource": "来源:{{path}}", "documentationUnavailableTitle": "文档暂时不可用", "documentationUnavailable": "当前无法读取这个技能版本的文档文件。你仍然可以在文件列表里查看包内容。", + "packageLinkMissingTitle": "文件未找到", + "packageLinkMissingDescription": "该链接指向的文件不在当前技能版本中。", "authorLabel": "作者 {{name}}", "expandOverview": "展开全文", "collapseOverview": "收起内容", @@ -801,6 +810,8 @@ "namespaceLabel": "命名空间", "loginToRate": "登录后可以收藏和评分", "install": "安装", + "installMethodClawhub": "ClawHub CLI", + "installMethodSkillhub": "SkillHub CLI", "download": "下载", "labelsSectionTitle": "标签管理", "labelsSectionDescription": "为这个技能挂载或移除推荐标签,帮助用户筛选和发现。", @@ -1278,7 +1289,8 @@ "prev": "上一页", "next": "下一页", "pagePrefix": "第", - "pageSuffix": "页" + "pageSuffix": "页", + "goToPage": "第 {{page}} 页" }, "user": { "menu": { diff --git a/web/src/i18n/skill-detail-locale.test.ts b/web/src/i18n/skill-detail-locale.test.ts index 6434f4d5..a237ef60 100644 --- a/web/src/i18n/skill-detail-locale.test.ts +++ b/web/src/i18n/skill-detail-locale.test.ts @@ -7,4 +7,11 @@ describe('skill detail lifecycle locales', () => { expect(zh.skillDetail.unarchiveSkill).toBe('恢复技能') expect(en.skillDetail.unarchiveSkill).toBe('Restore Skill') }) + + it('defines package relative link missing messages in both locales', () => { + expect(zh.skillDetail.packageLinkMissingTitle).toBe('文件未找到') + expect(zh.skillDetail.packageLinkMissingDescription).toBe('该链接指向的文件不在当前技能版本中。') + expect(en.skillDetail.packageLinkMissingTitle).toBe('File not found') + expect(en.skillDetail.packageLinkMissingDescription).toBe('This link points to a file that is not included in the current skill version.') + }) }) diff --git a/web/src/pages/dashboard/my-skills.test.ts b/web/src/pages/dashboard/my-skills.test.ts index f2c58112..c82faf93 100644 --- a/web/src/pages/dashboard/my-skills.test.ts +++ b/web/src/pages/dashboard/my-skills.test.ts @@ -8,6 +8,8 @@ const useMySkillsMock = vi.fn() vi.mock('@tanstack/react-router', () => ({ useNavigate: () => navigateMock, + useLocation: () => ({ pathname: '/dashboard/skills' }), + useSearch: () => ({}), })) vi.mock('react-i18next', async () => { @@ -69,6 +71,14 @@ vi.mock('@/shared/hooks/use-user-queries', () => ({ useSubmitPromotion: () => ({ mutateAsync: vi.fn(), isPending: false }), })) +vi.mock('@/shared/hooks/use-namespace-queries', () => ({ + useMyNamespaces: () => ({ data: [] }), +})) + +vi.mock('@/shared/hooks/use-debounce', () => ({ + useDebounce: (value: string) => value, +})) + vi.mock('@/shared/lib/skill-lifecycle', () => ({ getHeadlineVersion: () => ({ id: 11, version: '1.0.0', status: 'PUBLISHED' }), getPublishedVersion: () => ({ id: 11, version: '1.0.0', status: 'PUBLISHED' }), diff --git a/web/src/pages/dashboard/my-skills.tsx b/web/src/pages/dashboard/my-skills.tsx index 78192c32..4dbe4765 100644 --- a/web/src/pages/dashboard/my-skills.tsx +++ b/web/src/pages/dashboard/my-skills.tsx @@ -1,22 +1,28 @@ -import { useState } from 'react' -import { useNavigate } from '@tanstack/react-router' +import { useCallback, useEffect, useState } from 'react' +import { useLocation, useNavigate, useSearch } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useAuth } from '@/features/auth/use-auth' import { Button } from '@/shared/ui/button' import { Card } from '@/shared/ui/card' +import { Input } from '@/shared/ui/input' +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/shared/ui/select' import { EmptyState } from '@/shared/components/empty-state' import { ConfirmDialog } from '@/shared/components/confirm-dialog' import { DashboardPageHeader } from '@/shared/components/dashboard-page-header' import { Pagination } from '@/shared/components/pagination' import { useArchiveSkill, useUnarchiveSkill, useWithdrawSkillReview } from '@/shared/hooks/use-skill-queries' +import { useMyNamespaces } from '@/shared/hooks/use-namespace-queries' import { useMySkills, useSubmitPromotion } from '@/shared/hooks/use-user-queries' +import { useDebounce } from '@/shared/hooks/use-debounce' import { getHeadlineVersion, getPublishedVersion, getOwnerPreviewVersion, hasPendingOwnerPreview } from '@/shared/lib/skill-lifecycle' import { formatCompactCount } from '@/shared/lib/number-format' import { toast } from '@/shared/lib/toast' +import { buildReturnTo } from '@/shared/lib/auth-route' import { ApiError } from '@/api/client' import { getMySkillEmptyStateKey, getMySkillFilters, type MySkillFilter } from './my-skill-filters' const PAGE_SIZE = 10 +const ALL_NAMESPACES_VALUE = '__all_namespaces__' /** * Dashboard page for skills owned by the current user. @@ -36,18 +42,61 @@ function getPromotionConflictKey(error: ApiError): 'promotion.duplicate_pending' export function MySkillsPage() { const navigate = useNavigate() + const location = useLocation() + const search = useSearch({ from: '/dashboard/skills' }) const { t } = useTranslation() const { hasRole } = useAuth() - const [page, setPage] = useState(0) - const [filter, setFilter] = useState('ALL') + + // The URL is the source of truth for page / filter / namespace / keyword so the + // search context survives navigating into a skill and back via the returnTo link. + const page = search.page ?? 0 + const filter = (search.filter as MySkillFilter) ?? 'ALL' + const namespaceFilter = search.namespace ?? '' + const keyword = search.q ?? '' + + // Keep an instant-feedback copy of the keyword input, debounced before it is + // pushed to the URL so each keystroke does not create a history entry or query. + const [keywordInput, setKeywordInput] = useState(keyword) + const debouncedKeyword = useDebounce(keywordInput.trim(), 300) + const [archiveTarget, setArchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [unarchiveTarget, setUnarchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [withdrawTarget, setWithdrawTarget] = useState<{ namespace: string; slug: string; name: string; version: string } | null>(null) const [promotionTarget, setPromotionTarget] = useState<{ skillId: number; versionId: number; name: string; version: string } | null>(null) - const { data: skillPage, isLoading } = useMySkills({ page, size: PAGE_SIZE, filter: filter === 'ALL' ? undefined : filter }) + + const updateSearch = useCallback((next: Partial, options?: { replace?: boolean }) => { + navigate({ + to: '/dashboard/skills', + search: (prev) => ({ ...prev, ...next }), + replace: options?.replace, + }) + }, [navigate]) + + // Push the debounced keyword to the URL (reset page to 0 when search changes) + useEffect(() => { + if (debouncedKeyword !== keyword) { + updateSearch({ q: debouncedKeyword || undefined, page: 0 }, { replace: true }) + } + }, [debouncedKeyword, keyword, updateSearch]) + + // Sync keywordInput when navigating back via returnTo + useEffect(() => { + setKeywordInput(keyword) + }, [keyword]) + + const { data: skillPage, isLoading } = useMySkills({ + page, + size: PAGE_SIZE, + filter: filter === 'ALL' ? undefined : filter, + q: keyword || undefined, + namespace: namespaceFilter || undefined, + }) + const { data: namespaceOptions } = useMyNamespaces() + const skills = skillPage?.items ?? [] const totalPages = skillPage ? Math.max(Math.ceil(skillPage.total / skillPage.size), 1) : 1 const availableFilters = getMySkillFilters(hasRole('SUPER_ADMIN')) + const hasActiveSearch = keyword.trim() !== '' || namespaceFilter !== '' const emptyStateKey = getMySkillEmptyStateKey(filter) const archiveMutation = useArchiveSkill() const unarchiveMutation = useUnarchiveSkill() @@ -57,10 +106,15 @@ export function MySkillsPage() { const handleSkillClick = (namespace: string, slug: string) => { navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}`, - search: { returnTo: '/dashboard/skills' }, + search: { returnTo: buildReturnTo(location) }, }) } + const handleClearSearch = () => { + setKeywordInput('') + updateSearch({ q: undefined, namespace: undefined, page: 0 }) + } + const handleUpdateSkill = (namespace: string, visibility?: string) => { navigate({ to: '/dashboard/publish', @@ -238,21 +292,61 @@ export function MySkillsPage() { )} /> -
- {availableFilters.map((option) => ( - + ) : null} +
+ +
+ {availableFilters.map((option) => ( + + ))} +
{skillPage && skillPage.total > 0 ? ( @@ -400,17 +494,23 @@ export function MySkillsPage() { {skillPage.total > PAGE_SIZE ? ( - + updateSearch({ page: next })} /> ) : null} ) : ( navigate({ to: '/dashboard/publish' })}> - {t('mySkills.publishSkill')} - + hasActiveSearch ? ( + + ) : ( + + ) } /> )} diff --git a/web/src/pages/namespace.test.tsx b/web/src/pages/namespace.test.tsx index 8fcfd410..c950fb55 100644 --- a/web/src/pages/namespace.test.tsx +++ b/web/src/pages/namespace.test.tsx @@ -1,4 +1,7 @@ -import { describe, expect, it, vi } from 'vitest' +import type { ReactNode } from 'react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const buttonRecords: Array<{ label: string }> = [] vi.mock('@tanstack/react-router', () => ({ useNavigate: () => vi.fn(), @@ -23,6 +26,14 @@ vi.mock('@/features/skill/skill-card', () => ({ SkillCard: () => null, })) +vi.mock('@/shared/ui/button', () => ({ + Button: ({ children }: { children?: ReactNode }) => { + const label = Array.isArray(children) ? children.join('') : String(children ?? '') + buttonRecords.push({ label }) + return + }, +})) + vi.mock('@/shared/components/skeleton-loader', () => ({ SkeletonList: () => null, })) @@ -38,7 +49,26 @@ vi.mock('@/shared/hooks/use-namespace-queries', () => ({ vi.mock('@/shared/hooks/use-skill-queries', () => ({ useSearchSkills: () => ({ - data: { items: [] }, + data: { + items: [ + { + id: 1, + displayName: 'Demo Skill', + summary: 'summary', + namespace: 'global', + slug: 'demo', + downloadCount: 1, + starCount: 1, + ratingCount: 0, + updatedAt: '2026-03-20T00:00:00Z', + canSubmitPromotion: false, + publishedVersion: { id: 10, version: '1.0.0', status: 'PUBLISHED' }, + }, + ], + total: 1, + page: 0, + size: 20, + }, isLoading: false, }), })) @@ -47,6 +77,14 @@ import { renderToStaticMarkup } from 'react-dom/server' import { NamespacePage } from './namespace' describe('NamespacePage', () => { + beforeEach(() => { + buttonRecords.length = 0 + useNamespaceDetailMock.mockReturnValue({ + data: { id: 1, slug: 'global', displayName: 'Global', type: 'GLOBAL', status: 'ACTIVE' }, + isLoading: false, + }) + }) + it('exports a named component function', () => { expect(typeof NamespacePage).toBe('function') }) @@ -60,4 +98,11 @@ describe('NamespacePage', () => { const html = renderToStaticMarkup() expect(html).toContain('namespace.notFound') }) + + it('does not render namespace distribution controls when skills are available', () => { + const html = renderToStaticMarkup() + + expect(buttonRecords).toHaveLength(0) + expect(html).not.toContain('type="checkbox"') + }) }) diff --git a/web/src/pages/namespace.tsx b/web/src/pages/namespace.tsx index 69ac0127..275f7ba5 100644 --- a/web/src/pages/namespace.tsx +++ b/web/src/pages/namespace.tsx @@ -63,7 +63,7 @@ export function NamespacePage() { <>
{skillsData.items.map((skill, idx) => ( -
+
handleSkillClick(skill.slug)} diff --git a/web/src/pages/search.test.tsx b/web/src/pages/search.test.tsx index a921d5d3..aac629a0 100644 --- a/web/src/pages/search.test.tsx +++ b/web/src/pages/search.test.tsx @@ -6,6 +6,8 @@ const navigateMock = vi.fn() const useSearchMock = vi.fn() const buttonRecords: Array<{ label: string; variant?: string | null; onClick?: (() => void) | undefined }> = [] const paginationProps: Array<{ onPageChange: (page: number) => void }> = [] +const searchBarProps: Array<{ value?: string; onSearch?: (query: string) => void }> = [] +const searchSkillParams: Array> = [] vi.mock('@tanstack/react-router', () => ({ useNavigate: () => navigateMock, @@ -34,7 +36,10 @@ vi.mock('@/features/auth/use-auth', () => ({ })) vi.mock('@/features/search/search-bar', () => ({ - SearchBar: () =>
search-bar
, + SearchBar: (props: { value?: string; onSearch?: (query: string) => void }) => { + searchBarProps.push(props) + return
search-bar
+ }, })) vi.mock('@/features/skill/skill-card', () => ({ @@ -85,7 +90,10 @@ vi.mock('@/app/page-shell-style', () => ({ const useSearchSkillsMock = vi.fn() vi.mock('@/shared/hooks/use-skill-queries', () => ({ - useSearchSkills: () => useSearchSkillsMock(), + useSearchSkills: (params: Record) => { + searchSkillParams.push(params) + return useSearchSkillsMock() + }, })) vi.mock('@/shared/hooks/use-label-queries', () => ({ @@ -120,8 +128,11 @@ describe('SearchPage', () => { navigateMock.mockReset() buttonRecords.length = 0 paginationProps.length = 0 + searchBarProps.length = 0 + searchSkillParams.length = 0 useSearchMock.mockReturnValue({ q: 'agent', + namespace: 'team-ai', label: 'code-generation', sort: 'downloads', page: 1, @@ -156,6 +167,7 @@ describe('SearchPage', () => { to: '/search', search: { q: 'agent', + namespace: 'team-ai', label: '', sort: 'downloads', page: 0, @@ -173,6 +185,7 @@ describe('SearchPage', () => { to: '/search', search: { q: 'agent', + namespace: 'team-ai', label: 'code-generation', sort: 'newest', page: 0, @@ -191,6 +204,7 @@ describe('SearchPage', () => { to: '/search', search: { q: 'agent', + namespace: 'team-ai', label: 'code-generation', sort: 'downloads', page: 2, @@ -201,6 +215,7 @@ describe('SearchPage', () => { to: '/search', search: { q: 'agent', + namespace: 'team-ai', label: 'code-generation', sort: 'downloads', page: 0, @@ -209,6 +224,38 @@ describe('SearchPage', () => { }) }) + it('passes the namespace URL state into skill search', () => { + renderToStaticMarkup() + + expect(searchSkillParams[0]).toMatchObject({ + q: 'agent', + namespace: 'team-ai', + label: 'code-generation', + sort: 'downloads', + page: 1, + size: 12, + }) + }) + + it('extracts a leading namespace token from the search input', () => { + renderToStaticMarkup() + + searchBarProps[0]?.onSearch?.('@product-team onboarding') + + expect(navigateMock).toHaveBeenCalledWith({ + to: '/search', + search: { + q: 'onboarding', + namespace: 'product-team', + label: 'code-generation', + sort: 'downloads', + page: 0, + starredOnly: false, + }, + replace: true, + }) + }) + it('renders the default skill list when the empty query still returns items', () => { useSearchMock.mockReturnValue({ q: '', diff --git a/web/src/pages/search.tsx b/web/src/pages/search.tsx index 58c421db..ec23d319 100644 --- a/web/src/pages/search.tsx +++ b/web/src/pages/search.tsx @@ -12,7 +12,7 @@ import { Pagination } from '@/shared/components/pagination' import { useSearchSkills } from '@/shared/hooks/use-skill-queries' import { useVisibleLabels } from '@/shared/hooks/use-label-queries' import { useMyStars } from '@/shared/hooks/use-user-queries' -import { normalizeSearchQuery } from '@/shared/lib/search-query' +import { formatNamespaceSearchInput, normalizeSearchQuery, parseNamespaceSearchInput } from '@/shared/lib/search-query' import { Button } from '@/shared/ui/button' import { APP_SHELL_PAGE_CLASS_NAME } from '@/app/page-shell-style' @@ -55,17 +55,22 @@ function scrollToTopOnPageChange() { * Search text, sorting, pagination, and the starred-only filter are mirrored into router search * params so the page can be shared, restored, and revisited without losing state. */ -function filterStarredSkills(skills: SkillSummary[], query: string): SkillSummary[] { +function filterStarredSkills(skills: SkillSummary[], query: string, namespace: string): SkillSummary[] { const normalizedQuery = query.trim().toLowerCase() - if (!normalizedQuery) { - return skills - } + const normalizedNamespace = namespace.trim().toLowerCase() - return skills.filter((skill) => - [skill.displayName, skill.summary, skill.namespace, skill.slug] - .filter(Boolean) - .some((value) => value!.toLowerCase().includes(normalizedQuery)) - ) + return skills.filter((skill) => { + const matchesNamespace = !normalizedNamespace || skill.namespace.toLowerCase() === normalizedNamespace + if (!matchesNamespace) { + return false + } + if (!normalizedQuery) { + return true + } + return [skill.displayName, skill.summary, skill.namespace, skill.slug] + .filter(Boolean) + .some((value) => value!.toLowerCase().includes(normalizedQuery)) + }) } function sortStarredSkills(skills: SkillSummary[], sort: string): SkillSummary[] { @@ -86,16 +91,17 @@ export function SearchPage() { const { isAuthenticated } = useAuth() const q = normalizeSearchQuery(searchParams.q || '') + const namespace = (searchParams.namespace || '').replace(/^@/, '') const selectedLabel = searchParams.label || '' const sort = searchParams.sort || 'newest' const page = searchParams.page ?? 0 const starredOnly = searchParams.starredOnly ?? false - const [queryInput, setQueryInput] = useState(q) + const [queryInput, setQueryInput] = useState(formatNamespaceSearchInput(namespace, q)) const previousPageRef = useRef(page) useEffect(() => { - setQueryInput(q) - }, [q]) + setQueryInput(formatNamespaceSearchInput(namespace, q)) + }, [namespace, q]) useEffect(() => { if (previousPageRef.current !== page) { @@ -113,6 +119,7 @@ export function SearchPage() { const { data, isLoading, isFetching } = useSearchSkills({ q, + namespace: namespace || undefined, label: selectedLabel || undefined, sort, page, @@ -128,47 +135,51 @@ export function SearchPage() { useEffect(() => { // Debounce URL updates while the user is typing so query state stays shareable without // triggering a navigation on every keystroke. - const normalizedQuery = normalizeSearchQuery(queryInput) - if (normalizedQuery === q) { + const parsedInput = parseNamespaceSearchInput(queryInput) + if (parsedInput.query === q && parsedInput.namespace === namespace) { return } - if (!normalizedQuery) { + if (!parsedInput.query && !parsedInput.namespace) { startTransition(() => { - navigate({ to: '/search', search: { q: '', label: selectedLabel, sort, page: 0, starredOnly }, replace: page === 0 }) + navigate({ to: '/search', search: { q: '', namespace: '', label: selectedLabel, sort, page: 0, starredOnly }, replace: page === 0 }) }) return } const timeoutId = window.setTimeout(() => { startTransition(() => { - navigate({ to: '/search', search: { q: normalizedQuery, label: selectedLabel, sort, page: 0, starredOnly }, replace: true }) + navigate({ to: '/search', search: { q: parsedInput.query, namespace: parsedInput.namespace, label: selectedLabel, sort, page: 0, starredOnly }, replace: true }) }) }, 250) return () => window.clearTimeout(timeoutId) - }, [navigate, page, q, queryInput, selectedLabel, sort, starredOnly]) + }, [navigate, namespace, page, q, queryInput, selectedLabel, sort, starredOnly]) const handleSearch = (query: string) => { - const normalizedQuery = normalizeSearchQuery(query) + const parsedInput = parseNamespaceSearchInput(query) setQueryInput(query) startTransition(() => { - navigate({ to: '/search', search: { q: normalizedQuery, label: selectedLabel, sort, page: 0, starredOnly }, replace: true }) + navigate({ to: '/search', search: { q: parsedInput.query, namespace: parsedInput.namespace, label: selectedLabel, sort, page: 0, starredOnly }, replace: true }) }) } const handleSortChange = (newSort: string) => { - navigate({ to: '/search', search: { q, label: selectedLabel, sort: newSort, page: 0, starredOnly } }) + navigate({ to: '/search', search: { q, namespace, label: selectedLabel, sort: newSort, page: 0, starredOnly } }) } const handlePageChange = (newPage: number) => { blurActiveElement() - navigate({ to: '/search', search: { q, label: selectedLabel, sort, page: newPage, starredOnly } }) + navigate({ to: '/search', search: { q, namespace, label: selectedLabel, sort, page: newPage, starredOnly } }) } const handleLabelToggle = (label: string) => { const nextLabel = selectedLabel === label ? '' : label - navigate({ to: '/search', search: { q, label: nextLabel, sort, page: 0, starredOnly } }) + navigate({ to: '/search', search: { q, namespace, label: nextLabel, sort, page: 0, starredOnly } }) + } + + const handleNamespaceClear = () => { + navigate({ to: '/search', search: { q, namespace: '', label: selectedLabel, sort, page: 0, starredOnly } }) } const handleStarredToggle = () => { @@ -182,15 +193,15 @@ export function SearchPage() { return } - navigate({ to: '/search', search: { q, label: selectedLabel, sort, page: 0, starredOnly: !starredOnly } }) + navigate({ to: '/search', search: { q, namespace, label: selectedLabel, sort, page: 0, starredOnly: !starredOnly } }) } const handleSkillClick = (namespace: string, slug: string) => { - navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}` }) + navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}`, search: { returnTo: `${window.location.pathname}${window.location.search}` } }) } const filteredStarredSkills = starredOnly - ? sortStarredSkills(filterStarredSkills(starredSkills ?? [], q), sort) + ? sortStarredSkills(filterStarredSkills(starredSkills ?? [], q, namespace), sort) : [] const starredPageItems = starredOnly ? filteredStarredSkills.slice(page * PAGE_SIZE, (page + 1) * PAGE_SIZE) @@ -280,6 +291,15 @@ export function SearchPage() { {label.displayName} ))} + {namespace ? ( + + ) : null}
diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx index 87bdcb48..1c6374ce 100644 --- a/web/src/pages/skill-detail.test.tsx +++ b/web/src/pages/skill-detail.test.tsx @@ -1,11 +1,24 @@ +/** @vitest-environment jsdom */ + import { renderToStaticMarkup } from 'react-dom/server' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { MouseEvent } from 'react' +import type { SkillFile } from '@/api/types' + +const toastMocks = vi.hoisted(() => ({ + success: vi.fn(), + error: vi.fn(), +})) const navigateMock = vi.fn() const hasRoleMock = vi.fn<(role: string) => boolean>((role: string) => role === 'USER') const useSkillDetailMock = vi.fn() const useSkillLabelsMock = vi.fn() const useSkillVersionsMock = vi.fn() +const useSkillFilesMock = vi.fn() +const useSkillReadmeMock = vi.fn() +const useSkillFileMock = vi.fn() let authState: { user: { userId: string; platformRoles: string[] } | null hasRole: (role: string) => boolean @@ -47,7 +60,7 @@ vi.mock('@/features/report/use-skill-reports', () => ({ })) vi.mock('@/shared/lib/toast', () => ({ - toast: { success: vi.fn(), error: vi.fn() }, + toast: { success: toastMocks.success, error: toastMocks.error }, })) vi.mock('@/api/client', () => ({ @@ -76,7 +89,47 @@ vi.mock('@/shared/lib/number-format', () => ({ })) vi.mock('@/features/skill/markdown-renderer', () => ({ - MarkdownRenderer: () =>
markdown
, + MarkdownRenderer: ({ + content, + onLinkClick, + }: { + content: string + onLinkClick?: (href: string, event: MouseEvent) => void + }) => ( + + ), +})) + +vi.mock('@/features/skill/file-preview-dialog', () => ({ + FilePreviewDialog: ({ open, node }: { open: boolean; node: { path: string } | null }) => ( + open && node ?
preview:{node.path}
: null + ), })) vi.mock('@/features/skill/file-tree', () => ({ @@ -107,9 +160,9 @@ vi.mock('@/shared/hooks/use-skill-queries', () => ({ useDetachSkillLabel: () => ({ mutate: vi.fn(), isPending: false }), useSkillVersions: (...args: unknown[]) => useSkillVersionsMock(...args), useSkillVersionDetail: () => ({ data: undefined }), - useSkillFiles: () => ({ data: [] }), - useSkillReadme: () => ({ data: '# Demo', error: null }), - useSkillFile: () => ({ data: null, isLoading: false, error: null }), + useSkillFiles: (...args: unknown[]) => useSkillFilesMock(...args), + useSkillReadme: (...args: unknown[]) => useSkillReadmeMock(...args), + useSkillFile: (...args: unknown[]) => useSkillFileMock(...args), useArchiveSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), useDeleteSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), useDeleteSkillVersion: () => ({ mutateAsync: vi.fn(), isPending: false }), @@ -165,9 +218,26 @@ function createSkill(overrides: Record = {}) { } } +function createSkillFile(filePath: string): SkillFile { + return { + id: filePath.length, + filePath, + fileSize: 128, + contentType: 'text/markdown', + sha256: `sha-${filePath}`, + } +} + describe('SkillDetailPage', () => { + afterEach(() => cleanup()) + beforeEach(() => { navigateMock.mockReset() + useSkillFilesMock.mockReset() + useSkillReadmeMock.mockReset() + useSkillFileMock.mockReset() + toastMocks.success.mockReset() + toastMocks.error.mockReset() hasRoleMock.mockImplementation((role: string) => role === 'USER') authState = { user: { userId: 'owner-1', platformRoles: ['USER'] }, @@ -196,6 +266,9 @@ describe('SkillDetailPage', () => { useSkillLabelsMock.mockReturnValue({ data: undefined, }) + useSkillFilesMock.mockReturnValue({ data: [] }) + useSkillReadmeMock.mockReturnValue({ data: '# Demo', error: null }) + useSkillFileMock.mockReturnValue({ data: null, isLoading: false, error: null }) }) it('shows hard delete action for the skill owner', () => { @@ -401,4 +474,53 @@ describe('SkillDetailPage', () => { expect(html).toContain('break-all') expect(html).toContain('leading-snug') }) + + it('opens a file preview when overview markdown relative link matches a package file', () => { + useSkillFilesMock.mockReturnValue({ + data: [ + createSkillFile('README.md'), + createSkillFile('docs/usage.md'), + ], + }) + + render() + fireEvent.click(screen.getByRole('link', { name: 'Usage' })) + + expect(screen.getByRole('dialog').textContent).toContain('preview:docs/usage.md') + expect(toastMocks.error).not.toHaveBeenCalled() + }) + + it('keeps the viewer on the detail page and shows a toast for missing package files', () => { + useSkillFilesMock.mockReturnValue({ + data: [ + createSkillFile('README.md'), + createSkillFile('docs/usage.md'), + ], + }) + + render() + fireEvent.click(screen.getByRole('link', { name: 'Missing' })) + + expect(screen.queryByRole('dialog')).toBeNull() + expect(toastMocks.error).toHaveBeenCalledWith( + 'skillDetail.packageLinkMissingTitle', + 'skillDetail.packageLinkMissingDescription', + ) + }) + + it('leaves external links and same-document anchors alone', () => { + useSkillFilesMock.mockReturnValue({ + data: [ + createSkillFile('README.md'), + createSkillFile('docs/usage.md'), + ], + }) + + render() + fireEvent.click(screen.getByRole('link', { name: 'External' })) + fireEvent.click(screen.getByRole('link', { name: 'Anchor' })) + + expect(screen.queryByRole('dialog')).toBeNull() + expect(toastMocks.error).not.toHaveBeenCalled() + }) }) diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index 086f85a0..77b0c291 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -1,12 +1,14 @@ -import { useEffect, useRef, useState } from 'react' +import { useEffect, useRef, useState, type MouseEvent } from 'react' import { useTranslation } from 'react-i18next' import { useParams, useNavigate, useRouterState, useSearch } from '@tanstack/react-router' import { useMutation, useQueryClient } from '@tanstack/react-query' import { ArrowLeft, ArrowUpCircle, ChevronDown, ChevronUp, Clock, Folder, Globe, Lock, RefreshCw, ShieldCheck, Terminal, User, Users } from 'lucide-react' import { MarkdownRenderer } from '@/features/skill/markdown-renderer' +import { resolvePackageRelativeLink } from '@/features/skill/package-relative-link' import { FileTree } from '@/features/skill/file-tree' import { FilePreviewDialog } from '@/features/skill/file-preview-dialog' import type { FileTreeNode } from '@/features/skill/file-tree-builder' +import type { SkillFile } from '@/api/types' import { InstallCommand } from '@/features/skill/install-command' import { ShareButton } from '@/features/skill/share-button' import { SkillLabelPanel } from '@/features/skill/skill-label-panel' @@ -87,6 +89,20 @@ function parseMetadataJson(parsed?: string) { } } +function createPackageFilePreviewNode(file: SkillFile): FileTreeNode { + const pathParts = file.filePath.split('/').filter(Boolean) + const name = pathParts[pathParts.length - 1] ?? file.filePath + + return { + id: file.filePath, + name, + path: file.filePath, + type: 'file', + file, + depth: Math.max(pathParts.length - 1, 0), + } +} + function getPromotionConflictKey(error: ApiError): 'promotion.duplicate_pending' | 'promotion.already_promoted' | null { if (error.serverMessageKey === 'promotion.duplicate_pending') { return 'promotion.duplicate_pending' @@ -172,7 +188,6 @@ export function SkillDetailPage() { && ['PENDING_REVIEW', 'SCANNING', 'SCAN_FAILED'].includes(headlineVersion?.status ?? '') const hasPendingOwnerPreview = ownerPreviewVersion?.status === 'PENDING_REVIEW' const hasRejectedOwnerPreview = ownerPreviewVersion?.status === 'REJECTED' - const hasRejectedVersion = versions?.some((v) => v.status === 'REJECTED') ?? false const hasPublishedPendingReview = Boolean(publishedVersion && hasPendingOwnerPreview) const canInteract = skill?.canInteract ?? true const canReport = skill?.canReport ?? true @@ -285,6 +300,24 @@ export function SkillDetailPage() { setPreviewDialogOpen(true) } + const handleOverviewLinkClick = (href: string, event: MouseEvent) => { + const resolution = resolvePackageRelativeLink(href, documentationPath, files) + + if (resolution.status === 'ignored') { + return + } + + event.preventDefault() + + if (resolution.status === 'matched') { + setPreviewNode(createPackageFilePreviewNode(resolution.file)) + setPreviewDialogOpen(true) + return + } + + toast.error(t('skillDetail.packageLinkMissingTitle'), t('skillDetail.packageLinkMissingDescription')) + } + // Download a single file from the skill version const handleDownloadFile = () => { const isAnonymousAllowed = namespace === 'global' && skill?.visibility === 'PUBLIC' @@ -762,7 +795,7 @@ export function SkillDetailPage() { {t('skillDetail.versionStatusPendingReview')} )} - {!isPendingPreview && (isRejectedPreview || hasRejectedOwnerPreview || hasRejectedVersion) && skill.canManageLifecycle && ( + {!isPendingPreview && (isRejectedPreview || hasRejectedOwnerPreview) && skill.canManageLifecycle && ( {t('skillDetail.rejectedBadge')} @@ -845,7 +878,7 @@ export function SkillDetailPage() { style={!isOverviewExpanded && isOverviewCollapsible ? { maxHeight: `${overviewMaxHeight}px` } : undefined} >
- +
{!isOverviewExpanded && isOverviewCollapsible ? (
diff --git a/web/src/shared/components/confirm-dialog.tsx b/web/src/shared/components/confirm-dialog.tsx index b760ac45..aee74103 100644 --- a/web/src/shared/components/confirm-dialog.tsx +++ b/web/src/shared/components/confirm-dialog.tsx @@ -45,7 +45,7 @@ export function ConfirmDialog({ return ( - + {title} {description && {description}} diff --git a/web/src/shared/components/pagination.tsx b/web/src/shared/components/pagination.tsx index 5f54d904..069dbd85 100644 --- a/web/src/shared/components/pagination.tsx +++ b/web/src/shared/components/pagination.tsx @@ -7,8 +7,43 @@ interface PaginationProps { onPageChange: (page: number) => void } +type PageItem = number | 'ellipsis' + +/** + * Builds the list of page slots to render. Always shows the first and last page, + * the current page, and one neighbour on each side, collapsing the rest into + * ellipsis markers. Pages are 0-indexed internally; labels are 1-indexed. + */ +function buildPageItems(current: number, totalPages: number): PageItem[] { + if (totalPages <= 7) { + return Array.from({ length: totalPages }, (_, i) => i) + } + + const items: PageItem[] = [] + const first = 0 + const last = totalPages - 1 + const start = Math.max(first + 1, current - 1) + const end = Math.min(last - 1, current + 1) + + items.push(first) + if (start > first + 1) { + items.push('ellipsis') + } + for (let i = start; i <= end; i += 1) { + items.push(i) + } + if (end < last - 1) { + items.push('ellipsis') + } + items.push(last) + + return items +} + export function Pagination({ page, totalPages, onPageChange }: PaginationProps) { const { t } = useTranslation() + const pageItems = buildPageItems(page, totalPages) + return (
-
- {t('pagination.pagePrefix')} - {page + 1} - / - {totalPages} - {t('pagination.pageSuffix') && {t('pagination.pageSuffix')}} + +
+ {pageItems.map((item, index) => + item === 'ellipsis' ? ( + + ) : ( + + ), + )}
+