Skip to content

fix(cli): align anonymous installability rules (ISSUE-39)#515

Closed
dongmucat wants to merge 2 commits into
mainfrom
fix/issue-39-installable-public-rules
Closed

fix(cli): align anonymous installability rules (ISSUE-39)#515
dongmucat wants to merge 2 commits into
mainfrom
fix/issue-39-installable-public-rules

Conversation

@dongmucat

Copy link
Copy Markdown
Collaborator

概述

Align CLI anonymous search, resolve, and download with the same installable public version contract.

变更内容

后端实现

  • Added SkillInstallability as the shared version-level installability rule: PUBLISHED + downloadReady + not yanked.
  • Updated search published-version projection so download-unavailable/yanked versions are not exposed as installable published summaries.
  • Updated CLI search mapping to omit results without an installable published version and return a filtered total.
  • Updated resolve/download flows to reject non-installable versions before returning download URLs or falling back to per-file bundles.
  • DB migration: none.
  • API contract/schema: no Controller or OpenAPI schema change.

前端实现

  • No frontend changes.

测试覆盖

  • Backend unit: SkillQueryServiceTest covers downloadReady=false resolve rejection and yanked download availability.
  • Backend unit: SkillDownloadServiceTest covers downloadReady=false and yanked download rejection before bundle fallback.
  • App unit: SkillSearchAppServiceTest covers search projection excluding download-unavailable published versions.
  • App unit: CliSkillAppServiceTest covers CLI search filtering of results without installable published versions.
  • E2E tests: not applicable, no frontend changes.

质量门禁

  • Focused backend verification passed: CliSkillAppServiceTest, SkillSearchAppServiceTest, SkillQueryServiceTest, SkillDownloadServiceTest.
  • make test-backend-app passed: 528 tests, 0 failures, 0 errors.
  • git diff --check passed.
  • make generate-api not required: no Controller/API schema change.
  • Frontend typecheck/lint/unit/E2E not run: no frontend files changed.

安全考虑

  • Anonymous CLI search no longer advertises versions that install/resolve/download will reject.
  • Resolve/download reject domain-level unavailable or yanked versions before touching storage fallback logic.
  • No authentication, authorization, or secret-handling changes in this PR.

相关 Issue

Closes ISSUE-39
Related to ISSUE-36

测试说明

本地验证步骤

  1. Run make test-backend-app.
  2. Confirm Maven reports BUILD SUCCESS and Tests run: 528, Failures: 0, Errors: 0, Skipped: 0 for skillhub-app.

回归测试范围

  • Anonymous CLI search result filtering.
  • CLI resolve/download behavior for downloadReady=false and yanked versions.
  • Published version summary projection used by search.
  • Bundle fallback behavior for storage-object missing cases remains separate from domain installability.

Signed-off-by: dongmucat <1127093059@qq.com>
@dongmucat

Copy link
Copy Markdown
Collaborator Author

Ready for technical review.

Scope:

  • Aligns CLI anonymous search/resolve/download with the installable version contract: PUBLISHED + downloadReady + not yanked.
  • Keeps storage bundle fallback separate from domain installability, so downloadReady=false and yanked versions fail before object-storage fallback.
  • Does not change Controller/OpenAPI, DB migration, frontend, publish/delete/whoami auth behavior, or main branch state.

Verification:

  • Focused tests passed: CliSkillAppServiceTest, SkillSearchAppServiceTest, SkillQueryServiceTest, SkillDownloadServiceTest.
  • make test-backend-app passed: 528 tests, 0 failures, 0 errors.
  • git diff --check passed.

@dongmucat

Copy link
Copy Markdown
Collaborator Author

技术审查结论:Changes requested,当前不建议合并。说明:当前 GitHub 凭证被判定为 PR 作者,平台不允许我对自己的 PR 设置 Request changes review,因此本结论以评论形式记录;我没有合并 main。

阻塞问题

  1. 匿名 CLI search 仍可能返回 install/resolve 会拒绝的 skill,search/resolve/download 的“latest 可安装”契约没有真正一致。

server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java:93-107 对 latest 缺失或 latest 不可安装的 skill 会进入 fallback,再从 findBySkillIdInAndStatus(..., PUBLISHED) 中选择任意可安装的旧 published version。随后 server/skillhub-app/src/main/java/com/iflytek/skillhub/service/cli/CliSkillAppService.java:58-68 只要 publishedVersion != null 就把它返回给 CLI。

server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java:779-786 的隐式 latest resolve 仍要求 latestVersionId 存在并指向 published version,resolveVersion 又会在 :562 校验 installability。结果是:

  • latestVersionId == null 但存在旧的 published/downloadReady/not-yanked 版本时,search 可能返回,install/resolve latest 会拒绝;
  • latest 指向 downloadReady=false 或 yanked,但存在旧的可安装 published 版本时,search 可能返回旧版本,install/resolve latest 会拒绝。

这违反了 ISSUE-39 的“匿名 search 只返回有可安装 latest version 的 skill”和正式规格中的 no latest / downloadReady=false / yanked 负例要求。建议为 CLI anonymous/public search 使用专门的“installable latest”投影,要求 latestVersionId 指向 SkillInstallability.isInstallableVersion(version),不要在 CLI search 路径 fallback 到旧 published version;如果门户搜索仍需要旧 fallback 行为,应把两条投影路径拆开。

  1. 负例矩阵覆盖不足,当前还不能证明 hidden/private/archived/unpublished/no latest/downloadReady=false/yanked 在 search/resolve/download 上都按规格收敛。

本 PR 新增覆盖主要是:search 的 downloadReady=false published summary、CLI search 过滤 null publishedVersion、resolve latest 的 downloadReady=false、download 的 downloadReady=false 和 yanked。缺口包括:

  • search:no latest、latest yanked、latest 不可安装但存在旧可安装 published version;
  • resolve:no latest、yanked、显式/标签解析到不可安装 published version;
  • download latest:no latest、latest yanked;
  • hidden/private/archived/unpublished:已有一些 visibility/detail 侧测试,但缺少针对 CLI anonymous resolve/download/search 这组验收矩阵的聚合回归。

优化建议

  • SkillDownloadService.downloadVersion 现在对所有调用者都要求 PUBLISHED + downloadReady + not yanked。如果产品仍允许 owner/admin 通过非 CLI 公共下载路径拿 UPLOADED/PENDING_REVIEW 包,需要保留或测试替代路径;如果 downloadReviewVersion 已覆盖,则建议补一条说明或回归测试。
  • CliSearchResult.total = items.size() 是分页后过滤结果数量,不是全局 filtered total;对 CLI 可能可接受,但 final QA 应确认展示语义。

CI 状态

当前 DCO、Docs、Web、Server Unit Tests 已通过;E2E (Real Services) 仍是 pending。即使上述代码问题修复,也应等待 E2E 通过后再进入最终 QA/合并门禁。本次我未重跑本地测试,审查基于 PR diff、现有测试覆盖和 GitHub checks 当前状态。

Signed-off-by: dongmucat <1127093059@qq.com>
@dongmucat

Copy link
Copy Markdown
Collaborator Author

Updated PR #515 with commit 7f8d2aa5.

Fixed:

  • Anonymous/public search summaries now only surface the exact latestVersionId when that version is installable; no fallback to older published/download-ready versions.
  • Download latest now rejects archived namespaces for non-members, matching the public resolve/search visibility contract.
  • Added negative coverage for search no-latest/latest-yanked/latest-downloadReady=false with old-version fallback, resolve no-latest/yanked/explicit/tag not-downloadable, download latest no-latest/yanked/hidden/private/archived/unpublished, and anonymous search SQL visibility filters.

Verification:

  • Focused matrix passed: SkillSearchAppServiceTest, SkillQueryServiceTest, SkillDownloadServiceTest, PostgresFullTextQueryServiceTest (86 tests, 0 failures).
  • make test-backend-app passed: 530 tests, 0 failures, 0 errors.
  • git diff --check passed.
  • GitHub checks are all green: DCO, Docs Build, Server Unit Tests, Web Build And Test, E2E (Real Services), license/cla.

No main merge performed.

@dongmucat

Copy link
Copy Markdown
Collaborator Author

复审结论:Approved,上一轮阻塞项已关闭。说明:当前 GitHub 凭证被判定为 PR 作者,平台不允许我设置正式 Approved review,因此本结论以评论形式记录;我没有合并 main。

确认点:

  • search 不再 fallback 旧可安装 published version:projectPublishedSummaries 只接受 latestVersionId 指向且满足 SkillInstallability.isInstallableVersion 的版本,不再查询旧 published 版本补位。
  • search/resolve/download latest 的可安装判断已收敛到 latest 版本级条件:PUBLISHED + downloadReady + not yanked;resolve 和 download 在返回 URL/包之前都会拒绝不可安装版本。
  • 负例矩阵已补齐到本轮要求:search 覆盖 no latest、latest yanked、latest downloadReady=false、旧版本 fallback;resolve 覆盖 no latest、latest yanked、downloadReady=false、显式版本/tag 指向不可安装版本、hidden/private/archived/unpublished;download latest 覆盖 no latest、latest yanked、archived、hidden/private/unpublished,并保留 bundle fallback 区分。
  • 未看到对 publish/delete/whoami 的认证行为改动。

验证:GitHub checks 当前全部通过(DCO、Docs Build、Server Unit Tests、Web Build And Test、E2E Real Services、license/cla);本地 git diff --check origin/main..origin/pr/515 通过。未重跑本地 backend 全量测试,复审基于当前 PR diff、补充测试和 GitHub CI。

@dongmucat

Copy link
Copy Markdown
Collaborator Author

Superseded by #523, the single replacement PR for ISSUE-36 / ISSUE-42. Closing this PR so there is one active delivery path. No main merge was performed.

@dongmucat dongmucat closed this Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant