From 38b1a406bb73aadc5ee4b47bfc95cfb5360565aa Mon Sep 17 00:00:00 2001 From: fatelei Date: Fri, 31 Jul 2026 18:23:06 +0800 Subject: [PATCH] fix: fix publish update button ui issue --- api/services/skill_management_service.py | 14 ++- .../services/test_skill_management_service.py | 6 +- .../skills/__tests__/detail-page.spec.tsx | 48 +++++++---- web/features/skills/detail/file-editor.tsx | 80 +++++++++++------ web/features/skills/detail/shared.tsx | 85 +++++++++++++------ web/features/skills/detail/skill-metadata.tsx | 2 +- 6 files changed, 159 insertions(+), 76 deletions(-) diff --git a/api/services/skill_management_service.py b/api/services/skill_management_service.py index 3fe2962001c..4199e76733e 100644 --- a/api/services/skill_management_service.py +++ b/api/services/skill_management_service.py @@ -92,6 +92,7 @@ _MAX_SKILL_DESCRIPTION_LENGTH = 1024 _UNTITLED_DISPLAY_NAME = "Untitled skill" _UNTITLED_SKILL_NAME_PREFIX = "untitled-skill" _UNTITLED_SKILL_DESCRIPTION = "Describe what this Skill does and when an Agent should use it." +_EMPTY_SKILL_DRAFT_CONTENT = "\n" _UNTITLED_SKILL_MD_BODY = """# Untitled skill Describe what this Skill does, when an Agent should use it, and any step-by-step instructions it must follow. @@ -422,7 +423,7 @@ class SkillManagementService: Creating a Skill is intentionally a database write even before publication: the editor needs a stable ``skill_id`` for the side panel and draft file edits. A no-name create request produces a unique internal name, an - ``Untitled skill`` display name, and a placeholder ``SKILL.md`` draft; it + ``Untitled skill`` display name, and an empty ``SKILL.md`` draft; it does not create a published version. """ @@ -434,7 +435,9 @@ class SkillManagementService: self._enforce_workspace_skill_limit(session, tenant_id=tenant_id) skill_name = payload.name or self._generate_untitled_skill_name(session, tenant_id=tenant_id) display_name = payload.display_name or (_UNTITLED_DISPLAY_NAME if payload.name is None else skill_name) - description = payload.description or _UNTITLED_SKILL_DESCRIPTION + description = payload.description or ( + "" if payload.name is None else _UNTITLED_SKILL_DESCRIPTION + ) skill = Skill( tenant_id=tenant_id, name=skill_name, @@ -2937,12 +2940,15 @@ class SkillManagementService: file.hash = hashlib.sha256(file.content_text.encode("utf-8")).hexdigest() def _build_initial_skill_md(self, *, skill: Skill) -> str: - body = _UNTITLED_SKILL_MD_BODY if skill.display_name == _UNTITLED_DISPLAY_NAME else "" + is_untitled_draft = not skill.name_manually_edited and skill.display_name == _UNTITLED_DISPLAY_NAME + if is_untitled_draft: + return _EMPTY_SKILL_DRAFT_CONTENT + return self._build_skill_md( name=skill.name, description=skill.description, display_name=skill.display_name, - body=body, + body="", ) @staticmethod diff --git a/api/tests/unit_tests/services/test_skill_management_service.py b/api/tests/unit_tests/services/test_skill_management_service.py index c0ad98d6b2c..d31dd45bb5e 100644 --- a/api/tests/unit_tests/services/test_skill_management_service.py +++ b/api/tests/unit_tests/services/test_skill_management_service.py @@ -283,7 +283,7 @@ def test_create_skill_without_name_initializes_untitled_draft() -> None: assert created["name"].startswith("untitled-skill-") assert created["display_name"] == "Untitled skill" - assert created["description"] == "Describe what this Skill does and when an Agent should use it." + assert created["description"] == "" assert created["created_by_name"] == "Li Wei" assert created["updated_by_name"] == "Li Wei" assert created["latest_published_version_id"] is None @@ -292,9 +292,7 @@ def test_create_skill_without_name_initializes_untitled_draft() -> None: assert skill_md["path"] == "SKILL.md" assert skill_md["kind"] == "file" assert skill_md["storage"] == "text" - assert f"name: {created['name']}" in skill_md["content"] - assert "description: Describe what this Skill does and when an Agent should use it." in skill_md["content"] - assert "# Untitled skill" in skill_md["content"] + assert skill_md["content"] == "\n" assert service.list_versions(tenant_id=TENANT, skill_id=created["id"]) == {"data": []} diff --git a/web/features/skills/__tests__/detail-page.spec.tsx b/web/features/skills/__tests__/detail-page.spec.tsx index ca4d95a0439..aa18597af4e 100644 --- a/web/features/skills/__tests__/detail-page.spec.tsx +++ b/web/features/skills/__tests__/detail-page.spec.tsx @@ -909,21 +909,7 @@ describe('SkillDetailPage', () => { }) it('shows Skill manifest placeholders for an empty draft', async () => { - mocks.skillDetail = createDefaultSkillDraftDetail({ - files: [ - { - id: 'file-1', - path: 'SKILL.md', - kind: 'file', - storage: 'text', - mime_type: 'text/markdown', - content: '---\nname: \ndescription: \nmetadata:\n display-name: Untitled skill\n---\n\n', - tool_file_id: null, - size: 72, - hash: 'hash-1', - }, - ], - }) + mocks.skillDetail = createDefaultSkillDraftDetail() renderSkillDetailPage() @@ -934,8 +920,38 @@ describe('SkillDetailPage', () => { screen.getByPlaceholderText('skill.skillManagement.detail.skillDescriptionPlaceholder'), ).toBeInTheDocument() expect( - screen.getByText('skill.skillManagement.detail.referenceFiles.livePlaceholder'), + screen.queryByText( + 'Describe what this Skill does, when an Agent should use it, and any step-by-step instructions it must follow.', + ), + ).not.toBeInTheDocument() + }) + + it('treats a newly created empty Skill draft as Builder creation mode', async () => { + mocks.skillDetail = createDefaultSkillDraftDetail({ + description: '', + files: [ + { + id: 'file-1', + path: 'SKILL.md', + kind: 'file', + storage: 'text', + mime_type: 'text/markdown', + content: '\n', + tool_file_id: null, + size: 32, + hash: 'hash-1', + }, + ], + }) + + renderSkillDetailPage() + + expect( + await screen.findByText('skill.skillManagement.detail.builder.promptTitle'), ).toBeInTheDocument() + expect( + screen.queryByText('skill.skillManagement.detail.builder.editIntro'), + ).not.toBeInTheDocument() }) it('does not render the code editor when external file content fails to load', async () => { diff --git a/web/features/skills/detail/file-editor.tsx b/web/features/skills/detail/file-editor.tsx index bff454d584d..549cd207c19 100644 --- a/web/features/skills/detail/file-editor.tsx +++ b/web/features/skills/detail/file-editor.tsx @@ -64,6 +64,7 @@ import { isTextFile, metadataKeyInputClassName, metadataValueInputClassName, + normalizeSkillDraftContentForEditing, parseCsvRows, parseMarkdownContent, refreshSkillDetailAfterConflict, @@ -129,7 +130,8 @@ export function FileEditor({ const { t } = useTranslation('skill') const queryClient = useQueryClient() const { formatTimeFromNow } = useFormatTimeFromNow() - const initialContent = file && isTextFile(file) ? (file.content ?? '') : '' + const initialContent = + file && isTextFile(file) ? normalizeSkillDraftContentForEditing(file.content ?? '') : '' const initialSavedAt = detail?.updated_at ? detail.updated_at * 1000 : undefined const [draftContent, setDraftContent] = useState(initialContent) const [markdownMode, setMarkdownMode] = useState<'live' | 'source'>('live') @@ -170,20 +172,24 @@ export function FileEditor({ const isMarkdown = isMarkdownFile(file) const isSkillManifestFile = filePath === 'SKILL.md' const isCsv = isCsvFile(file) + const editableDraftContent = useMemo( + () => normalizeSkillDraftContentForEditing(draftContent), + [draftContent], + ) const markdownContent = useMemo( () => isSkillManifestFile - ? parseMarkdownContent(draftContent) + ? parseMarkdownContent(editableDraftContent) : { - body: stripSkillFrontmatterForDisplay(draftContent), + body: stripSkillFrontmatterForDisplay(editableDraftContent), description: '', displayName: '', metadata: [], name: '', }, - [draftContent, isSkillManifestFile], + [editableDraftContent, isSkillManifestFile], ) - const csvRows = useMemo(() => parseCsvRows(draftContent), [draftContent]) + const csvRows = useMemo(() => parseCsvRows(editableDraftContent), [editableDraftContent]) const hasPublishedVersion = !!detail?.latest_published_version_id const latestPublishedAt = detail?.latest_published_at const hasUnpublishedChanges = @@ -237,6 +243,19 @@ export function FileEditor({ useEffect(() => { setDisplayNameDraft(markdownContent.displayName) }, [markdownContent.displayName]) + + useEffect(() => { + if (editableDraftContent === draftContent) return + + draftContentRef.current = editableDraftContent + if (lastSavedContentRef.current === draftContent) + lastSavedContentRef.current = editableDraftContent + if (saveConflictContentRef.current === draftContent) + saveConflictContentRef.current = editableDraftContent + setDraftContent(editableDraftContent) + setSaveStatus(editableDraftContent === lastSavedContentRef.current ? 'saved' : 'dirty') + setExternalContentRevision((revision) => revision + 1) + }, [draftContent, editableDraftContent]) const shouldFetchTextFileContent = !!file && isTextFile(file) && file.content == null const textContentQuery = useQuery({ queryKey: ['skill-file-text-content', skillId, selectedVersionId, filePath, fileHash], @@ -372,8 +391,9 @@ export function FileEditor({ ? findFileByPath(refetchedDetail.files ?? [], currentFile.path) : undefined if (latestFile && isTextFile(latestFile) && latestFile.content != null) - lastSavedContentRef.current = latestFile.content - else if (currentFileContent != null) lastSavedContentRef.current = currentFileContent + lastSavedContentRef.current = normalizeSkillDraftContentForEditing(latestFile.content) + else if (currentFileContent != null) + lastSavedContentRef.current = normalizeSkillDraftContentForEditing(currentFileContent) if (refetchedDetail) { detailRef.current = refetchedDetail fileMutationCoordinator.latestDetail = refetchedDetail @@ -437,7 +457,10 @@ export function FileEditor({ useEffect(() => { const currentFile = fileRef.current - const nextContent = currentFile && isTextFile(currentFile) ? (currentFile.content ?? '') : '' + const nextContent = + currentFile && isTextFile(currentFile) + ? normalizeSkillDraftContentForEditing(currentFile.content ?? '') + : '' draftContentRef.current = nextContent lastSavedContentRef.current = nextContent @@ -455,13 +478,14 @@ export function FileEditor({ useEffect(() => { if (!file || !isTextFile(file) || file.content == null) return if (draftContentRef.current !== lastSavedContentRef.current) return - if (file.content === lastSavedContentRef.current) return + const nextContent = normalizeSkillDraftContentForEditing(file.content) + if (nextContent === lastSavedContentRef.current) return - draftContentRef.current = file.content - lastSavedContentRef.current = file.content + draftContentRef.current = nextContent + lastSavedContentRef.current = nextContent saveConflictContentRef.current = null setHasSaveConflict(false) - setDraftContent(file.content) + setDraftContent(nextContent) setSavedAt(detail?.updated_at ? detail.updated_at * 1000 : undefined) setSaveStatus('saved') setExternalContentRevision((revision) => revision + 1) @@ -470,12 +494,13 @@ export function FileEditor({ useEffect(() => { if (!shouldFetchTextFileContent || textContentQuery.data == null) return if (draftContentRef.current !== lastSavedContentRef.current) return + const nextContent = normalizeSkillDraftContentForEditing(textContentQuery.data) - draftContentRef.current = textContentQuery.data - lastSavedContentRef.current = textContentQuery.data + draftContentRef.current = nextContent + lastSavedContentRef.current = nextContent saveConflictContentRef.current = null setHasSaveConflict(false) - setDraftContent(textContentQuery.data) + setDraftContent(nextContent) setSaveStatus('saved') setExternalContentRevision((revision) => revision + 1) }, [shouldFetchTextFileContent, textContentQuery.data]) @@ -942,7 +967,7 @@ export function FileEditor({ } return ( -
+
-
+
{isTextContentPending ? (
) : isTextContentError ? ( @@ -1181,7 +1206,7 @@ export function FileEditor({ key={editorRenderKey} editorRef={sourceTextareaRef} readOnly={readonly} - value={draftContent} + value={editableDraftContent} placeholder={t(($) => $['skillManagement.detail.referenceFiles.livePlaceholder'])} onChange={handleContentChange} onKeyDown={(event) => handleTextEditorKeyDown(event)} @@ -1231,7 +1256,7 @@ export function FileEditor({ ref={sourceTextareaRef} key={editorRenderKey} readOnly={readonly} - value={draftContent} + value={editableDraftContent} spellCheck={false} className={cn( 'h-full w-full resize-none rounded-xl border border-divider-regular bg-background-default p-4 font-mono text-[13px]/[20px] text-text-secondary outline-hidden read-only:bg-background-section', @@ -1314,13 +1339,8 @@ export function FileEditor({ )}
{!readonly && ( -
- +
+
setPublishConfirmOpen(false)} @@ -1332,7 +1352,13 @@ export function FileEditor({ referenceCount={detail?.reference_count ?? 0} skillId={skillId} /> - + +
)} {readonly && selectedVersion && ( diff --git a/web/features/skills/detail/shared.tsx b/web/features/skills/detail/shared.tsx index d2788105f55..47ce98eb0c4 100644 --- a/web/features/skills/detail/shared.tsx +++ b/web/features/skills/detail/shared.tsx @@ -180,6 +180,33 @@ const defaultSkillDescription = 'Describe what this Skill does and when an Agent const defaultSkillBody = '# Untitled skill\n\nDescribe what this Skill does, when an Agent should use it, and any step-by-step instructions it must follow.' +export const emptySkillDraftContentPlaceholder = '' + +function isLegacyUntitledSkillDraftContent(content: string) { + const normalizedContent = content.replace(/\r\n?/g, '\n').trim() + + return ( + normalizedContent.startsWith('---\n') && + /\n---\n/.test(normalizedContent) && + /^name:\s*untitled-skill-[a-z0-9-]+\s*$/m.test(normalizedContent) && + new RegExp( + `^description:\\s*${defaultSkillDescription.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*$`, + 'm', + ).test(normalizedContent) && + /^# Untitled skill\s*$/m.test(normalizedContent) && + normalizedContent.includes( + 'Describe what this Skill does, when an Agent should use it, and any step-by-step instructions it must follow.', + ) + ) +} + +export function normalizeSkillDraftContentForEditing(content: string) { + if (content.trim() === emptySkillDraftContentPlaceholder) return '' + if (isLegacyUntitledSkillDraftContent(content)) return '' + + return content +} + type SkillUploadStatus = 'failed' | 'saving' | 'uploaded' | 'uploading' export type SkillUploadQueueItem = { @@ -373,9 +400,11 @@ export function isEditableMetadataKey(key: string) { } export function parseMarkdownContent(content: string): ParsedMarkdownContent { - if (!content.startsWith('---')) { + const normalizedContent = normalizeSkillDraftContentForEditing(content) + + if (!normalizedContent.startsWith('---')) { return { - body: content, + body: normalizedContent, description: '', displayName: '', metadata: [], @@ -383,7 +412,7 @@ export function parseMarkdownContent(content: string): ParsedMarkdownContent { } } - const lines = content.split(/\r?\n/) + const lines = normalizedContent.split(/\r?\n/) const closingIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---') if (closingIndex === -1) { return { @@ -466,11 +495,12 @@ export function parseMarkdownContent(content: string): ParsedMarkdownContent { } export function stripSkillFrontmatterForDisplay(content: string) { - if (!content.startsWith('---')) return content + const normalizedContent = normalizeSkillDraftContentForEditing(content) + if (!normalizedContent.startsWith('---')) return normalizedContent - const lines = content.split(/\r?\n/) + const lines = normalizedContent.split(/\r?\n/) const closingIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---') - if (closingIndex === -1) return content + if (closingIndex === -1) return normalizedContent const frontmatterLines = lines.slice(1, closingIndex) const hasSkillFrontmatter = frontmatterLines.some((line) => { @@ -481,7 +511,7 @@ export function stripSkillFrontmatterForDisplay(content: string) { trimmedLine === 'metadata:' ) }) - if (!hasSkillFrontmatter) return content + if (!hasSkillFrontmatter) return normalizedContent return lines .slice(closingIndex + 1) @@ -549,7 +579,8 @@ function stringifyYamlKey(key: string) { } export function addMarkdownMetadata(content: string, key: string, value: string) { - const nextContent = removeMarkdownMetadata(content, key) + const normalizedContent = normalizeSkillDraftContentForEditing(content) + const nextContent = removeMarkdownMetadata(normalizedContent, key) const metadataLine = ` ${stringifyYamlKey(key)}: ${stringifyYamlValue(value)}` if (!nextContent.startsWith('---')) { @@ -559,7 +590,7 @@ export function addMarkdownMetadata(content: string, key: string, value: string) const lines = nextContent.split(/\r?\n/) const closingIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---') if (closingIndex === -1) { - return `---\nmetadata:\n${metadataLine}\n---\n\n${content}` + return `---\nmetadata:\n${metadataLine}\n---\n\n${normalizedContent}` } const metadataIndex = lines.findIndex( @@ -617,16 +648,17 @@ export function setMarkdownFrontmatterField( key: 'description' | 'name', value: string, ) { + const normalizedContent = normalizeSkillDraftContentForEditing(content) const fieldLine = `${key}: ${stringifyYamlValue(value)}` - if (!content.startsWith('---')) { - return `---\n${fieldLine}\n---\n\n${content}` + if (!normalizedContent.startsWith('---')) { + return `---\n${fieldLine}\n---\n\n${normalizedContent}` } - const lines = content.split(/\r?\n/) + const lines = normalizedContent.split(/\r?\n/) const closingIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---') if (closingIndex === -1) { - return `---\n${fieldLine}\n---\n\n${content}` + return `---\n${fieldLine}\n---\n\n${normalizedContent}` } for (let index = 1; index < closingIndex; index += 1) { @@ -645,14 +677,15 @@ export function setMarkdownFrontmatterField( } export function removeMarkdownMetadata(content: string, key: string) { + const normalizedContent = normalizeSkillDraftContentForEditing(content) const trimmedKey = key.trim() if (!isEditableMetadataKey(trimmedKey) || isProtectedMarkdownMetadataKey(trimmedKey)) - return content - if (!content.startsWith('---')) return content + return normalizedContent + if (!normalizedContent.startsWith('---')) return normalizedContent - const lines = content.split(/\r?\n/) + const lines = normalizedContent.split(/\r?\n/) const closingIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---') - if (closingIndex === -1) return content + if (closingIndex === -1) return normalizedContent const removeLineIndexes = new Set() let insideMetadata = false @@ -706,11 +739,12 @@ export function removeMarkdownMetadata(content: string, key: string) { } function removeMarkdownDisplayName(content: string) { - if (!content.startsWith('---')) return content + const normalizedContent = normalizeSkillDraftContentForEditing(content) + if (!normalizedContent.startsWith('---')) return normalizedContent - const lines = content.split(/\r?\n/) + const lines = normalizedContent.split(/\r?\n/) const closingIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---') - if (closingIndex === -1) return content + if (closingIndex === -1) return normalizedContent const removeLineIndexes = new Set() let insideMetadata = false @@ -892,9 +926,10 @@ export function getReferenceText(file: SkillFileResponse) { } export function getMarkdownBodyPrefix(content: string) { - if (!content.startsWith('---')) return '' + const normalizedContent = normalizeSkillDraftContentForEditing(content) + if (!normalizedContent.startsWith('---')) return '' - const lines = content.split(/\r?\n/) + const lines = normalizedContent.split(/\r?\n/) const closingIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---') if (closingIndex === -1) return '' @@ -1271,13 +1306,15 @@ export function isDefaultSkillBuilderDraft(detail: SkillDetailResponse) { const skillMd = findFileByPath(detail.files ?? [], 'SKILL.md') const skillMdContent = skillMd && isTextFile(skillMd) && skillMd.content ? parseMarkdownContent(skillMd.content) : null + const description = detail.description.trim() + const skillMdBody = skillMdContent?.body.trim() ?? '' return ( detail.latest_published_version_id == null && detail.name.startsWith('untitled-skill') && detail.display_name === 'Untitled skill' && - detail.description === defaultSkillDescription && - skillMdContent?.body.trim() === defaultSkillBody + (description === '' || description === defaultSkillDescription) && + (skillMdBody === '' || skillMdBody === defaultSkillBody) ) } diff --git a/web/features/skills/detail/skill-metadata.tsx b/web/features/skills/detail/skill-metadata.tsx index b343e6c1211..750ab12ac8f 100644 --- a/web/features/skills/detail/skill-metadata.tsx +++ b/web/features/skills/detail/skill-metadata.tsx @@ -402,7 +402,7 @@ export function SkillPublishConfirmPanel({