Skip to content

[ISSUE #15201] Fix skill meta description sync on upload#15204

Merged
KomachiSion merged 1 commit into
alibaba:developfrom
Jholly2008:develop-issue-ai-skill-description-sync
May 21, 2026
Merged

[ISSUE #15201] Fix skill meta description sync on upload#15204
KomachiSion merged 1 commit into
alibaba:developfrom
Jholly2008:develop-issue-ai-skill-description-sync

Conversation

@Jholly2008

Copy link
Copy Markdown
Contributor

Please do not create a Pull Request without creating an issue first.

What is the purpose of the change

Fixes #15201.

This PR fixes inconsistent AI skill descriptions between the skill list page and the skill detail/version page after uploading a new ZIP version for an existing skill.

Previously, the uploaded SKILL.md frontmatter description was saved into the version data, but the existing skill meta row only updated editingVersion. The meta description used by the list page could remain stale while the detail/version page showed the new uploaded description.

Brief changelog

  • Synchronize a non-blank draft description into the existing skill meta row when uploading a ZIP draft for an existing skill.
  • Preserve the existing meta description when the incoming draft description is blank.
  • Add a regression unit test for existing skill ZIP upload description synchronization.

Verifying this change

  • mvn -B clean package apache-rat:check spotbugs:check -DskipTests
  • mvn -pl ai -am "-Dtest=SkillOperationServiceImplTest#testUploadSkillFromZipShouldSyncMetaDescriptionForExistingSkill" "-Dsurefire.failIfNoSpecifiedTests=false" test
  • mvn -pl ai -am "-Dtest=SkillOperationServiceImplTest" "-Dsurefire.failIfNoSpecifiedTests=false" test
  • mvn -pl ai -am "-Dtest=AiResourceManagerTest" "-Dsurefire.failIfNoSpecifiedTests=false" test
  • mvn -pl ai -am "-Dtest=SkillOperationServiceImplTest,AiResourceManagerTest" "-Dsurefire.failIfNoSpecifiedTests=false" test
  • git diff --check

mvn clean install was also attempted locally, but it failed in existing nacos-common tests under JDK 21.0.1 with IllegalAccessException: static final field has no write access, unrelated to this change.

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean package apache-rat:check spotbugs:check -DskipTests to make sure basic checks pass. Run mvn clean install to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

@github-actions

Copy link
Copy Markdown

Thanks for your this PR. 🙏
Please check again for your PR changes whether contains any usage/api/configuration change such as Add new API , Add new configuration, Change default value of configuration.
If so, please add or update documents(markdown type) in docs/next/ for repository nacos-group/nacos-group.github.io


感谢您提交的PR。 🙏
请再次查看您的PR内容,确认是否包含任何使用方式/API/配置参数的变更,如:新增API新增配置参数修改默认配置等操作。
如果是,请确保在提交之前,在仓库nacos-group/nacos-group.github.io中的docs/next/目录下添加或更新文档(markdown格式)。

@Jholly2008

Copy link
Copy Markdown
Contributor Author

This PR only fixes internal metadata synchronization for existing AI skill ZIP uploads. It does not add or change any API, configuration, default value, or user-facing usage. Therefore no docs update is required.

@KomachiSion KomachiSion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR 主路径修复是对的,但 description 为空时的 CAS retry 语义确实有问题。第一次 CAS 失败后,如果并发请求已经把 ai_resource.c_desc 更新成新值,当前代码第二次重试仍会用最初 existedMeta.getDesc() 里的旧描述写回去,覆盖掉最新描述。

建议把 description 是否非空作为判断:

boolean syncDescription = StringUtils.isNotBlank(description);
newValue.setDesc(syncDescription ? description : existedMeta.getDesc());

...
if (!syncDescription) {
    nv.setDesc(latest.getDesc());
}

这样非空描述表示“本次操作要主动同步描述”,空描述表示“本次只更新 versionInfo,desc 应跟随 latest 保留”。

影响范围:skill 和 agentspec 都受这个通用方法影响;prompt 现有 meta 创建草稿路径没走这个 existing-meta 分支,基本不受本 PR 影响。建议补一个 AiResourceManagerTest:空 description、第一次 CAS 失败、retry find 返回 latest.desc = new-desc,最终第二次 updateMetaCas 捕获到的 desc 应该是 new-desc。

@Jholly2008
Jholly2008 force-pushed the develop-issue-ai-skill-description-sync branch from 0225224 to e9d06cc Compare May 21, 2026 07:52
@Jholly2008

Copy link
Copy Markdown
Contributor Author

Fixed the CAS retry behavior from the review comment.

Changes:

  • Non-blank description now means the draft upload should actively sync the meta description.
  • Blank description now means the operation only updates versionInfo; on CAS retry, desc is refreshed from latest meta to avoid overwriting concurrent description updates.
  • Added AiResourceManagerTest#initOrUpdateMetaForDraftShouldPreserveLatestDescriptionWhenDescriptionBlankOnRetry.

Verified with:

  • mvn -pl ai -am "-Dtest=AiResourceManagerTest#initOrUpdateMetaForDraftShouldPreserveLatestDescriptionWhenDescriptionBlankOnRetry" "-Dsurefire.failIfNoSpecifiedTests=false" test
  • mvn -pl ai -am "-Dtest=AiResourceManagerTest,SkillOperationServiceImplTest" "-Dsurefire.failIfNoSpecifiedTests=false" test
  • git diff --check

KomachiSion
KomachiSion previously approved these changes May 21, 2026
@Jholly2008
Jholly2008 force-pushed the develop-issue-ai-skill-description-sync branch from e9d06cc to 53f6b32 Compare May 21, 2026 10:37
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...a/nacos/ai/service/resource/AiResourceManager.java 85.00% 1 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@KomachiSion
KomachiSion merged commit b0aa560 into alibaba:develop May 21, 2026
6 checks passed
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.

Skill description is inconsistent between list and detail after uploading new version

3 participants