From 7e50acace76433ad601a36e31d2c8675042eac0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Sat, 31 Oct 2026 00:21:06 +0800 Subject: [PATCH] fix: tolerate extended metadata snapshots during presentation inheritance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 千乘妍 (Xiaoyaner) --- .../add-skill-presentation-fields/tasks.md | 2 +- .../skill/service/SkillPublishService.java | 4 +- .../domain/review/PromotionServiceTest.java | 9 +++- .../domain/review/ReviewServiceTest.java | 7 ++- .../service/SkillGovernanceServiceTest.java | 6 ++- .../service/SkillPublishServiceTest.java | 29 +++++++++++-- .../dashboard/publish-display-name.test.tsx | 43 ++++++++++++++++++- web/src/pages/skill-detail.test.tsx | 18 +++++++- 8 files changed, 104 insertions(+), 14 deletions(-) diff --git a/openspec/changes/add-skill-presentation-fields/tasks.md b/openspec/changes/add-skill-presentation-fields/tasks.md index 10fc21b23..b67fbb4f8 100644 --- a/openspec/changes/add-skill-presentation-fields/tasks.md +++ b/openspec/changes/add-skill-presentation-fields/tasks.md @@ -5,6 +5,6 @@ - [x] Snapshot presentation fields in parsed metadata and update effective published summary. - [x] Preserve original description in search text and keep pending updates isolated. - [x] Add web short-description and Markdown-overview inputs. -- [ ] Add full inheritance/restore-default/rollback acceptance coverage (partial: inheritance snapshot and parser reset; end-to-end restore/rollback still needed). +- [x] Add full inheritance/restore-default/rollback acceptance coverage (published-only frozen inheritance, reset/default-not-copied, yank rollback, approval, promotion, and rerelease). - [x] Regenerate OpenAPI types and update CLI usage documentation. - [x] Add detail-page overview projection and View SKILL.md entry. 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 4c494e13e..bb78dec59 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 @@ -705,7 +705,9 @@ public class SkillPublishService { .orElse(null); if (published != null && published.getParsedMetadataJson() != null) { try { - SkillMetadata previous = objectMapper.readValue(published.getParsedMetadataJson(), SkillMetadata.class); + SkillMetadata previous = objectMapper.readerFor(SkillMetadata.class) + .without(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .readValue(published.getParsedMetadataJson()); if (previous.shortDescriptionOverride() != null || previous.overviewOverride() != null) { metadata = metadata.withPresentation(previous.shortDescriptionOverride(), previous.overviewOverride(), published.getVersion()); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java index 79c5dc6f2..ce9dfd1be 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java @@ -672,7 +672,9 @@ class PromotionServiceTest { Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); sourceSkill.setLatestVersionId(999L); - sourceVersion.setParsedMetadataJson(selectedVersionMetadata(hasDisplayName)); + sourceVersion.setParsedMetadataJson(selectedVersionMetadata(hasDisplayName) + .replace("\"description\":\"Old version summary\"", + "\"description\":\"Old version summary\",\"shortDescriptionOverride\":\"Selected custom summary\",\"overviewOverride\":\"# Selected overview\"")); when(promotionRequestRepository.findById(PROMOTION_ID)) .thenReturn(Optional.of(pr), Optional.of(approvedRequest)); @@ -699,7 +701,10 @@ class PromotionServiceTest { verify(skillRepository, times(2)).save(skillCaptor.capture()); Skill newSkill = skillCaptor.getAllValues().get(0); assertEquals(hasDisplayName ? "旧版标题" : "包内标题", newSkill.getDisplayName()); - assertEquals("Old version summary", newSkill.getSummary()); + assertEquals("Selected custom summary", newSkill.getSummary()); + ArgumentCaptor versionCaptor = ArgumentCaptor.forClass(SkillVersion.class); + verify(skillVersionRepository).save(versionCaptor.capture()); + assertEquals(sourceVersion.getParsedMetadataJson(), versionCaptor.getValue().getParsedMetadataJson()); assertEquals(NEW_VERSION_ID, newSkill.getLatestVersionId()); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java index bf7721578..8406410b5 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java @@ -304,7 +304,8 @@ class ReviewServiceTest { assertDoesNotThrow(() -> sv.setParsedMetadataJson(objectMapper.writeValueAsString( new SkillMetadata("Approved Name", "Approved Summary", "1.0.0", "Body", hasDisplayName ? Map.of("metadata", Map.of("display-name", "包内标题")) : Map.of(), - hasDisplayName ? "已审核的中文标题" : null) + hasDisplayName ? "已审核的中文标题" : null, + "Approved custom summary", "# Approved overview", null) ))); when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task)); @@ -333,7 +334,9 @@ class ReviewServiceTest { assertEquals(SKILL_VERSION_ID, skill.getLatestVersionId()); assertEquals(hasDisplayName ? "已审核的中文标题" : "Approved Name", skill.getDisplayName()); assertEquals("my-skill", skill.getSlug()); - assertEquals("Approved Summary", skill.getSummary()); + assertEquals("Approved custom summary", skill.getSummary()); + assertEquals("# Approved overview", assertDoesNotThrow(() -> + objectMapper.readTree(sv.getParsedMetadataJson())).path("overviewOverride").asText()); assertEquals(REVIEWER_ID, skill.getUpdatedBy()); verify(eventPublisher).publishEvent(any(SkillPublishedEvent.class)); verify(governanceNotificationService).notifyUser(eq(USER_ID), eq("REVIEW"), eq("REVIEW_TASK"), eq(REVIEW_TASK_ID), eq("Review approved"), any()); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java index 7af64be84..390006c4b 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java @@ -297,6 +297,8 @@ class SkillGovernanceServiceTest { fallback.setParsedMetadataJson(new ObjectMapper().writeValueAsString(Map.of( "name", "example-skill", "description", "Published summary from v1", + "shortDescriptionOverride", "Restored custom summary", + "overviewOverride", "# Restored overview", "displayNameOverride", displayNameCase.equals("override") ? "网页旧版标题" : "", "frontmatter", switch (displayNameCase) { case "custom" -> Map.of("metadata", Map.of("display-name", "示例技能一")); @@ -321,7 +323,9 @@ class SkillGovernanceServiceTest { assertThat(skill.getLatestVersionId()).isEqualTo(11L); assertThat(skill.getDisplayName()).isEqualTo(displayNameCase.equals("override") ? "网页旧版标题" : displayNameCase.equals("custom") ? "示例技能一" : "example-skill"); - assertThat(skill.getSummary()).isEqualTo("Published summary from v1"); + assertThat(skill.getSummary()).isEqualTo("Restored custom summary"); + assertThat(new ObjectMapper().readTree(fallback.getParsedMetadataJson()).path("overviewOverride").asText()) + .isEqualTo("# Restored overview"); verify(skillRepository).save(skill); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java index 1b0853d2a..1f25a2c37 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java @@ -133,9 +133,11 @@ class SkillPublishServiceTest { SkillVersion previous = new SkillVersion(1L, "1.0.0", "owner"); setId(previous, 11L); previous.setStatus(SkillVersionStatus.PUBLISHED); - previous.setParsedMetadataJson(objectMapper.writeValueAsString( - new SkillMetadata("test-skill", "Old default", "1.0.0", "Body", Map.of(), null, - "Published custom", "# Custom overview", null))); + previous.setParsedMetadataJson(objectMapper.writeValueAsString(Map.of( + "name", "test-skill", "description", "Old default", "version", "1.0.0", + "body", "Body", "frontmatter", Map.of(), + "shortDescriptionOverride", "Published custom", "overviewOverride", "# Custom overview", + "complianceSnapshot", Map.of("status", "PASS")))); when(namespaceRepository.findBySlug("test-ns")).thenReturn(Optional.of(namespace)); when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "owner")) .thenReturn(Optional.of(mock(NamespaceMember.class))); @@ -166,6 +168,11 @@ class SkillPublishServiceTest { assertTrue(resetSnapshot.path("shortDescriptionOverride").isNull()); assertTrue(resetSnapshot.path("overviewOverride").isNull()); assertTrue(resetSnapshot.path("presentationInheritedFrom").isNull()); + SkillMetadata restored = objectMapper.readerFor(SkillMetadata.class) + .without(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .readValue(reset.getParsedMetadataJson()); + assertEquals("New default", restored.shortDescription()); + assertEquals("Body", restored.overview()); assertEquals("Published custom", skill.getSummary()); previous.setParsedMetadataJson(objectMapper.writeValueAsString( @@ -175,6 +182,14 @@ class SkillPublishServiceTest { JsonNode defaultSnapshot = objectMapper.readTree(defaults.getParsedMetadataJson()); assertTrue(defaultSnapshot.path("shortDescriptionOverride").isNull()); assertTrue(defaultSnapshot.path("presentationInheritedFrom").isNull()); + + SkillVersion publishedReset = service.publishFromEntries("test-ns", entries, "owner", SkillVisibility.PUBLIC, + Set.of("SUPER_ADMIN"), true, null, null, null, true).version(); + assertEquals(SkillVersionStatus.PUBLISHED, publishedReset.getStatus()); + assertEquals("New default", skill.getSummary()); + assertEquals("Body", objectMapper.readerFor(SkillMetadata.class) + .without(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .readValue(publishedReset.getParsedMetadataJson()).overview()); } @Test @@ -1084,13 +1099,14 @@ class SkillPublishServiceTest { setId(skill, 11L); skill.setDisplayName("Demo Skill"); skill.setSummary("Original summary"); + skill.setLatestVersionId(21L); Namespace namespace = new Namespace("global", "Global", "owner"); setId(namespace, 1L); SkillVersion sourceVersion = new SkillVersion(skill.getId(), "1.2.3", publisherId); setId(sourceVersion, 21L); sourceVersion.setStatus(SkillVersionStatus.PUBLISHED); - sourceVersion.setParsedMetadataJson("{\"displayNameOverride\":\"原网页标题\"}"); + sourceVersion.setParsedMetadataJson("{\"displayNameOverride\":\"原网页标题\",\"shortDescriptionOverride\":\"Custom summary\",\"overviewOverride\":\"# Custom overview\"}"); sourceVersion.setPublishedAt(Instant.parse("2026-03-15T10:00:00Z")); String sourceSkillMd = """ @@ -1121,6 +1137,8 @@ class SkillPublishServiceTest { when(skillFileRepository.findByVersionId(sourceVersion.getId())).thenReturn(List.of(skillMdFile, readmeFile)); when(objectStorageService.getObject(skillMdFile.getStorageKey())).thenReturn(new java.io.ByteArrayInputStream(sourceSkillMd.getBytes(StandardCharsets.UTF_8))); when(objectStorageService.getObject(readmeFile.getStorageKey())).thenReturn(new java.io.ByteArrayInputStream(readmeBytes)); + when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(1L, "demo-skill", publisherId)) + .thenReturn(Optional.of(skill)); when(skillPackageValidator.validate(anyList())).thenReturn(ValidationResult.pass()); when(skillMetadataParser.parse(anyString())).thenReturn(rereleaseMetadata); when(prePublishValidator.validate(any())).thenReturn(ValidationResult.pass()); @@ -1144,6 +1162,9 @@ class SkillPublishServiceTest { assertEquals("1.2.4", result.version().getVersion()); assertEquals("原网页标题", objectMapper.readTree(result.version().getParsedMetadataJson()).path("displayNameOverride").asText()); + assertEquals("Custom summary", objectMapper.readTree(result.version().getParsedMetadataJson()).path("shortDescriptionOverride").asText()); + assertEquals("# Custom overview", objectMapper.readTree(result.version().getParsedMetadataJson()).path("overviewOverride").asText()); + assertEquals("Original summary", skill.getSummary(), "Pending rerelease must not change the public summary"); // Rerelease for PUBLIC skill should go to PENDING_REVIEW (respecting visibility rules) assertEquals(SkillVersionStatus.PENDING_REVIEW, result.version().getStatus()); // Review task should be created for PUBLIC skill diff --git a/web/src/pages/dashboard/publish-display-name.test.tsx b/web/src/pages/dashboard/publish-display-name.test.tsx index cb243493d..da36e23cd 100644 --- a/web/src/pages/dashboard/publish-display-name.test.tsx +++ b/web/src/pages/dashboard/publish-display-name.test.tsx @@ -5,13 +5,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const preview = vi.hoisted(() => vi.fn()) const publish = vi.hoisted(() => vi.fn()) -vi.mock('@tanstack/react-router', () => ({ useNavigate: () => vi.fn(), useSearch: () => ({ namespace: 'team-ai' }) })) +const searchParams = vi.hoisted(() => ({ value: { namespace: 'team-ai', updateSkill: undefined as string | undefined } })) +vi.mock('@tanstack/react-router', () => ({ useNavigate: () => vi.fn(), useSearch: () => searchParams.value })) vi.mock('react-i18next', async () => ({ ...await vi.importActual('react-i18next'), useTranslation: () => ({ t: (key: string) => key }) })) vi.mock('@/shared/hooks/use-skill-queries', () => ({ useSkillDetail: () => ({ data: undefined }), previewSkillPackage: preview, usePublishSkill: () => ({ mutateAsync: publish, isPending: false }) })) vi.mock('@/shared/hooks/use-namespace-queries', () => ({ useMyNamespaces: () => ({ data: [{ id: 1, slug: 'team-ai', displayName: 'Team' }], isLoading: false }) })) vi.mock('@/features/publish/upload-zone', () => ({ UploadZone: ({ onFileSelect }: { onFileSelect: (file: File) => void }) => { if (e.target.files?.[0]) onFileSelect(e.target.files[0]) }} /> })) vi.mock('@/features/skill/skill-avatar-editor', () => ({ KEEP_AVATAR: { action: 'KEEP', ready: true }, SkillAvatarEditor: () => null })) vi.mock('@/features/skill/skill-avatar-manager', () => ({ SkillAvatarManager: () => null })) +vi.mock('@/api/client', async (importOriginal) => ({ ...await importOriginal(), skillAvatarApi: { status: vi.fn().mockResolvedValue({ action: 'KEEP' }) } })) vi.mock('@/shared/lib/navigate-after-overlays', () => ({ navigateAfterOverlays: (action: () => void) => action() })) vi.mock('@/shared/lib/toast', () => ({ toast: { success: vi.fn(), error: vi.fn() } })) import { PublishPage } from './publish' @@ -27,7 +29,44 @@ function select(name: string) { const metadata = (name: string) => ({ name: 'stable-slug', displayName: name, version: '1.0.0', description: 'summary' }) afterEach(cleanup) -beforeEach(() => { preview.mockReset(); publish.mockReset(); publish.mockResolvedValue({ namespace: 'team-ai', slug: 'stable-slug', version: '1.0.0', status: 'PENDING_REVIEW' }) }) +beforeEach(() => { searchParams.value.updateSkill = undefined; preview.mockReset(); publish.mockReset(); publish.mockResolvedValue({ namespace: 'team-ai', slug: 'stable-slug', version: '1.0.0', status: 'PENDING_REVIEW' }) }) + +describe('web presentation publishing', () => { + it('prefills package presentation without sending unedited defaults', async () => { + preview.mockResolvedValue({ ...metadata('Package title'), shortDescription: 'Package short', overview: '# Package overview' }) + mount(); select('presentation.zip') + await waitFor(() => expect((screen.getByLabelText('publish.shortDescription') as HTMLInputElement).value).toBe('Package short')) + expect((screen.getByLabelText('publish.overview') as HTMLTextAreaElement).value).toBe('# Package overview') + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(publish).toHaveBeenCalledWith(expect.objectContaining({ + shortDescription: undefined, overview: undefined, resetPresentation: false, + }))) + }) + + it('sends edited presentation and reset intent independently of form defaults', async () => { + preview.mockResolvedValue({ ...metadata('Package title'), shortDescription: 'Package short', overview: '# Package overview' }) + mount(); select('presentation.zip') + await waitFor(() => expect((screen.getByLabelText('publish.shortDescription') as HTMLInputElement).value).toBe('Package short')) + fireEvent.change(screen.getByLabelText('publish.shortDescription'), { target: { value: 'Web short' } }) + fireEvent.change(screen.getByLabelText('publish.overview'), { target: { value: '# Web overview' } }) + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(publish).toHaveBeenCalledWith(expect.objectContaining({ + shortDescription: 'Web short', overview: '# Web overview', resetPresentation: false, + }))) + }) + it('sends reset without re-sending edited fields', async () => { + searchParams.value.updateSkill = 'stable-slug' + preview.mockResolvedValue({ ...metadata('Package title'), shortDescription: 'Package short', overview: '# Package overview' }) + mount(); select('presentation.zip') + await waitFor(() => expect((screen.getByLabelText('publish.shortDescription') as HTMLInputElement).value).toBe('Package short')) + fireEvent.change(screen.getByLabelText('publish.shortDescription'), { target: { value: 'Discard me' } }) + fireEvent.click(screen.getByRole('checkbox', { name: 'publish.resetPresentation' })) + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(publish).toHaveBeenCalledWith(expect.objectContaining({ + shortDescription: undefined, overview: undefined, resetPresentation: true, + }))) + }) +}) describe('web display name publishing', () => { it('prefills from the package and sends the explicit edited name with the original file', async () => { diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx index ce6b940ac..982ad5aed 100644 --- a/web/src/pages/skill-detail.test.tsx +++ b/web/src/pages/skill-detail.test.tsx @@ -19,6 +19,7 @@ const useSkillVersionsMock = vi.fn() const useSkillFilesMock = vi.fn() const useSkillReadmeMock = vi.fn() const useSkillFileMock = vi.fn() +const useSkillVersionDetailMock = vi.fn() const usePromotionSourceStateMock = vi.fn() const submitPromotionMock = vi.fn() const submitRevocationMock = vi.fn() @@ -208,7 +209,7 @@ vi.mock('@/shared/hooks/use-skill-queries', () => ({ useAttachSkillLabel: () => ({ mutate: vi.fn(), isPending: false }), useDetachSkillLabel: () => ({ mutate: vi.fn(), isPending: false }), useSkillVersions: (...args: unknown[]) => useSkillVersionsMock(...args), - useSkillVersionDetail: () => ({ data: undefined }), + useSkillVersionDetail: (...args: unknown[]) => useSkillVersionDetailMock(...args), useSkillFiles: (...args: unknown[]) => useSkillFilesMock(...args), useSkillReadme: (...args: unknown[]) => useSkillReadmeMock(...args), useSkillFile: (...args: unknown[]) => useSkillFileMock(...args), @@ -301,6 +302,8 @@ describe('SkillDetailPage', () => { useSkillFilesMock.mockReset() useSkillReadmeMock.mockReset() useSkillFileMock.mockReset() + useSkillVersionDetailMock.mockReset() + useSkillVersionDetailMock.mockReturnValue({ data: undefined }) toastMocks.success.mockReset() toastMocks.error.mockReset() hasRoleMock.mockImplementation((role: string) => role === 'USER') @@ -1013,6 +1016,19 @@ describe('SkillDetailPage', () => { expect(html).toContain('leading-snug') }) + it('renders the selected snapshot overview and opens the packaged SKILL.md separately', () => { + useSkillVersionDetailMock.mockReturnValue({ data: { parsedMetadataJson: JSON.stringify({ + name: 'demo-skill', description: 'Original description', shortDescriptionOverride: 'Selected summary', + overviewOverride: '# Selected overview', + }) } }) + useSkillFilesMock.mockReturnValue({ data: [createSkillFile('README.md'), createSkillFile('SKILL.md')] }) + render() + expect(screen.getByText('markdown:# Selected overview')).toBeTruthy() + expect(screen.queryByText('markdown:# Demo')).toBeNull() + fireEvent.click(screen.getByRole('button', { name: 'skillDetail.viewSkillMd' })) + expect(screen.getByRole('dialog').textContent).toContain('preview:SKILL.md') + }) + it('opens a file preview when overview markdown relative link matches a package file', () => { useSkillFilesMock.mockReturnValue({ data: [