Skip to content

[ISSUE #15345] Return skill front matter in list response#15508

Open
jay666mnj wants to merge 4 commits into
alibaba:developfrom
jay666mnj:fix-skill-list-frontmatter
Open

[ISSUE #15345] Return skill front matter in list response#15508
jay666mnj wants to merge 4 commits into
alibaba:developfrom
jay666mnj:fix-skill-list-frontmatter

Conversation

@jay666mnj

Copy link
Copy Markdown
Contributor

What is the purpose of the change

Fixes #15345

Support returning front matter parsed from SKILL.md in the skill list response, so the console can display metadata such as alias and other custom fields.

Brief changelog

  • Add frontMatter to SkillSummary.
  • Load and parse SKILL.md front matter for the display version in skill list API.
  • Add unit test coverage for list response front matter.

Verifying this change

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

Result: Tests run: 1, Failures: 0, Errors: 0, Skipped: 0.

@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格式)。

@jay666mnj

jay666mnj commented Jul 14, 2026 via email

Copy link
Copy Markdown
Contributor Author

@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.

I don't think current implementation is good design.

If we query for a big page, the original list query only query the resource table and handle it.

But after the change, each skill will query two more: one version and query all skill data to get the frontmatter. which cost so many and get a small frontmatter.

I suggest to redesign for this before you do coding.

@jay666mnj
jay666mnj force-pushed the fix-skill-list-frontmatter branch from be14f5a to 94ae27a Compare July 15, 2026 14:43
@jay666mnj

jay666mnj commented Jul 15, 2026 via email

Copy link
Copy Markdown
Contributor Author

@jay666mnj
jay666mnj requested a review from KomachiSion July 15, 2026 15:15
@KomachiSion

Copy link
Copy Markdown
Collaborator

Thanks for redesigning the implementation. Reading front matter directly from ai_resource.ext avoids the N+1 version/storage queries on the list path, which is the right performance direction.

However, the cached data lifecycle is not complete yet:

  1. ext.frontMatter is overwritten when a draft is created or updated, while the display-version logic previously preferred latest. If v1 is online and v2 is being edited, the list may expose v2's unpublished front matter. Deleting the draft also does not restore the cache, so stale metadata may remain.

  2. How will existing skills be updated with this design? Their ai_resource.ext does not contain frontMatter, so the list API may continue returning frontMatter = null for them. Please clarify the migration or backfill strategy, and avoid lazy per-item database/storage queries in the list path.

  3. bumpMetaDescriptionAndExt ignores the final CAS result. If retries are exhausted, the content update succeeds but the list cache remains stale. Since the list no longer has a read-through fallback, this cache update should be reliable or repairable.

I suggest storing the corresponding frontMatterVersion in ext and updating the cached display metadata at lifecycle transitions such as publish, draft deletion, redraft, and bootstrap. The list path should continue to use only the single paginated ai_resource query.

Please also add lifecycle and large-page tests that verify:

  • latest plus editing draft returns the intended version's front matter;
  • deleting/publishing a draft updates the cache;
  • legacy and bootstrap skills are covered;
  • listing multiple skills performs no version-table or storage queries.

Additionally, the current PR fails spotless:check in SkillSummary.java, and the Admin/Console OpenAPI IT scenarios and Skill spec should be updated for the new response contract.

@jay666mnj

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I have updated the implementation according to your suggestions.

The list API still reads frontMatter only from ai_resource.ext and does not perform per-item version-table or storage queries. I added frontMatterVersion into ext and refreshed the cached display metadata during lifecycle transitions, including publish, force publish, draft deletion, redraft, and bootstrap.

Draft create/update with an existing latest version no longer exposes unpublished front matter. For legacy skills without cached frontMatter, the list API may return frontMatter = null until a later lifecycle transition or bootstrap repair, and it will not do lazy per-item backfill in the list path.

I also changed bumpMetaDescriptionAndExt to fail explicitly when CAS retries are exhausted, added lifecycle/large-page/legacy/bootstrap tests, updated the skill spec and Admin/Console OpenAPI IT scenarios, and fixed the SkillSummary spotless issue.

@KomachiSion

Copy link
Copy Markdown
Collaborator

Thanks for the update. The list hot path now avoids per-item version/storage queries, and the display-version lifecycle is improved. However, there are still several blockers:

  1. Version-level online/offline transitions are missing from the cache lifecycle. toggleVersionOnlineStatus may change or remove the latest label, but changeOnlineStatus does not refresh the cached front matter. Taking the current latest version offline, or bringing a newer version online, can therefore leave the list response with stale metadata.

  2. The cache update is not race-safe. refreshDisplayMetadataCache resolves the display version from one meta snapshot, then fetches the meta again before updating it. If the display version changes between those reads, the new versionInfo may be combined with front matter from the old version. The CAS conflict callback also refreshes versionInfo without recomputing ext. Although frontMatterVersion is stored, it is never validated or read.

Please restart the whole display-version calculation on CAS conflict, or update the lifecycle state and corresponding cache atomically. The list path can also compare frontMatterVersion with the resolved display version and return null on mismatch without introducing extra queries.

  1. Existing skills are still not actually backfilled. SkillDataBootstrapInitializer.buildBootstrapPlan only imports missing packages and skips existing built-in skills before bootstrapSkillFromZip is called. Non-built-in legacy skills also have no migration path. The new legacy test only confirms that they continue returning null. Please provide a bounded asynchronous/offline repair strategy rather than lazy list-time queries.

  2. Draft updates now introduce repeated version/storage work. When latest=v1 and editing=v2, every draft update calls refreshDisplayMetadataCache, queries v1, loads all files from storage, and performs another meta CAS update even when frontMatterVersion is already v1. Please use frontMatterVersion to short-circuit when the cache is current, and read only SKILL.md when a rebuild is actually required.

I also ran the complete related unit tests:

./mvnw -pl api,ai -am -Dtest=SkillSummaryTest,SkillOperationServiceImplTest -Dsurefire.failIfNoSpecifiedTests=false -DskipITs test

SkillSummaryTest passed, but SkillOperationServiceImplTest had 2 failures and 4 errors out of 79 tests. spotless:check passes.

Please also update the Admin/Console API scenario matrices and coverage registry for the changed response contract, and clean up the unrelated whitespace churn and corrupted 鈫? Javadoc characters.

So I prefer to discuss full solution before you do PR. We can discuss in issue first.

Can you submit one design plan spec into issue first, we disscuss it first.

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.md中frontmatter

2 participants