Skip to content

Commit 863ce1d

Browse files
authored
fix(skills): fix import, tags, duplication, model selection, and TXT icons (#42653)
1 parent 9089ed4 commit 863ce1d

38 files changed

Lines changed: 1278 additions & 83 deletions

‎web/features/agent-v2/agent-detail/configure/components/orchestrate/files/__tests__/file-icon.spec.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,17 @@ describe('agent file icon helpers', () => {
3939
expect(getFileIconType('archive.zip')).toBe('archive')
4040
expect(getFileIconType('script.ts')).toBe('code')
4141
})
42+
43+
it.each([
44+
['references/notes.txt', undefined],
45+
['references/NOTES.TXT', undefined],
46+
['references/notes.txt', 'text/plain'],
47+
['references/notes.txt', 'application/octet-stream'],
48+
])('should identify %s as text regardless of MIME metadata (%s)', (fileName, mimeType) => {
49+
expect(getFileIconType(fileName, mimeType)).toBe('text')
50+
})
51+
52+
it('should keep unrecognized binary files as generic files', () => {
53+
expect(getFileIconType('data.bin', 'application/octet-stream')).toBe('file')
54+
})
4255
})

‎web/features/agent-v2/agent-detail/configure/components/orchestrate/files/file-icon.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export function getFileIconType(fileName: string, mimeType?: string | null): Fil
6363
if (tableFileExtensions.has(extension)) return 'table'
6464
if (archiveFileExtensions.has(extension)) return 'archive'
6565
if (codeFileExtensions.has(extension)) return 'code'
66-
if (mimeType?.startsWith('text/')) return 'text'
66+
if (extension === 'txt' || mimeType?.startsWith('text/')) return 'text'
6767

6868
return 'file'
6969
}

‎web/features/skills/__tests__/detail-page-builder.spec.tsx‎

Lines changed: 208 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { SkillDetailResponse } from '@dify/contracts/api/console/workspaces/types.gen'
2-
import { act, fireEvent, screen, waitFor } from '@testing-library/react'
2+
import { act, fireEvent, screen, waitFor, within } from '@testing-library/react'
33
import userEvent from '@testing-library/user-event'
44
import { beforeEach, describe, expect, it, vi } from 'vite-plus/test'
55
import {
@@ -447,6 +447,213 @@ describe('SkillDetailPage builder', () => {
447447
expect(sendButton).toBeDisabled()
448448
}, 15000)
449449

450+
it('uses an available model instead of a default model that no longer exists', async () => {
451+
const user = userEvent.setup()
452+
mocks.defaultTextGenerationModel = {
453+
provider: { provider: 'langgenius/openai/openai' },
454+
model: 'gpt-5-chat-latest',
455+
}
456+
457+
renderSkillDetailPage()
458+
459+
const promptInput = await screen.findByPlaceholderText(
460+
'skill.skillManagement.detail.builder.modifyPlaceholder',
461+
)
462+
await user.type(promptInput, 'Update the skill{Enter}')
463+
464+
expect(mocks.sendSkillAssistMessage).toHaveBeenCalledWith(
465+
expect.objectContaining({
466+
model: expect.objectContaining({
467+
provider: 'langgenius/openai/openai',
468+
model: 'gpt-5.5',
469+
}),
470+
}),
471+
)
472+
expect(mocks.modelParameterRulesFetch).toHaveBeenCalled()
473+
const requestedModels = mocks.modelParameterRulesFetch.mock.calls.map(([input]) => {
474+
const url = input instanceof Request ? input.url : String(input)
475+
return new URL(url).searchParams.get('model')
476+
})
477+
expect(requestedModels).toEqual(['gpt-5.5'])
478+
})
479+
480+
it('does not request parameters or send with an unavailable default and no usable model', async () => {
481+
const user = userEvent.setup()
482+
mocks.defaultTextGenerationModel = {
483+
provider: { provider: 'langgenius/openai/openai' },
484+
model: 'gpt-5-chat-latest',
485+
}
486+
mocks.textGenerationModelList = []
487+
mocks.skillDetail = createDefaultSkillDraftDetail()
488+
489+
renderSkillDetailPage()
490+
491+
const promptInput = await screen.findByPlaceholderText(
492+
'skill.skillManagement.detail.builder.placeholder',
493+
)
494+
expect(
495+
screen.getByRole('button', {
496+
name: 'skill.skillManagement.detail.builder.send',
497+
}),
498+
).toBeDisabled()
499+
expect(
500+
screen.getByRole('button', {
501+
name: 'skill.skillManagement.detail.builder.exampleIssueTriage',
502+
}),
503+
).toBeDisabled()
504+
await user.type(promptInput, 'Create a skill{Enter}')
505+
506+
expect(mocks.sendSkillAssistMessage).not.toHaveBeenCalled()
507+
expect(mocks.modelParameterRulesFetch).not.toHaveBeenCalled()
508+
})
509+
510+
it('keeps an available default model instead of replacing it with the first model', async () => {
511+
const user = userEvent.setup()
512+
mocks.textGenerationModelList[0]!.models.unshift({
513+
model: 'gpt-4.1',
514+
status: 'active',
515+
})
516+
517+
renderSkillDetailPage()
518+
const promptInput = await screen.findByPlaceholderText(
519+
'skill.skillManagement.detail.builder.modifyPlaceholder',
520+
)
521+
await user.type(promptInput, 'Update the skill{Enter}')
522+
523+
expect(mocks.sendSkillAssistMessage).toHaveBeenCalledWith(
524+
expect.objectContaining({
525+
model: expect.objectContaining({ model: 'gpt-5.5' }),
526+
}),
527+
)
528+
expect(mocks.toastError).not.toHaveBeenCalled()
529+
})
530+
531+
it.each([
532+
{ target: 'provider', status: 'disabled' },
533+
{ target: 'model', status: 'disabled' },
534+
{ target: 'model', status: 'credential-removed' },
535+
])('does not use a default whose $target has status $status', async ({ target, status }) => {
536+
const user = userEvent.setup()
537+
const defaultProvider = mocks.textGenerationModelList[0]!
538+
if (target === 'provider') defaultProvider.status = status
539+
else defaultProvider.models[0]!.status = status
540+
mocks.textGenerationModelList.push({
541+
provider: 'langgenius/anthropic/anthropic',
542+
status: 'active',
543+
models: [{ model: 'claude-sonnet-4-6', status: 'active' }],
544+
})
545+
546+
renderSkillDetailPage()
547+
const promptInput = await screen.findByPlaceholderText(
548+
'skill.skillManagement.detail.builder.modifyPlaceholder',
549+
)
550+
await user.type(promptInput, 'Update the skill{Enter}')
551+
552+
expect(mocks.sendSkillAssistMessage).toHaveBeenCalledWith(
553+
expect.objectContaining({
554+
model: expect.objectContaining({
555+
provider: 'langgenius/anthropic/anthropic',
556+
model: 'claude-sonnet-4-6',
557+
}),
558+
}),
559+
)
560+
expect(mocks.modelParameterRulesFetch).toHaveBeenCalledTimes(1)
561+
const request = mocks.modelParameterRulesFetch.mock.calls[0]![0]
562+
const url = new URL(request instanceof Request ? request.url : String(request))
563+
expect(url.pathname).toContain('/model-providers/langgenius/anthropic/anthropic/')
564+
expect(url.searchParams.get('model')).toBe('claude-sonnet-4-6')
565+
})
566+
567+
it('waits for available models before requesting parameters or allowing Builder sends', async () => {
568+
const user = userEvent.setup()
569+
let resolveModels: (response: { data: typeof mocks.textGenerationModelList }) => void = () => {}
570+
const modelListResponse = new Promise<{ data: typeof mocks.textGenerationModelList }>(
571+
(resolve) => {
572+
resolveModels = resolve
573+
},
574+
)
575+
mocks.textGenerationModelListGet.mockReturnValue(modelListResponse)
576+
mocks.skillDetail = createDefaultSkillDraftDetail()
577+
578+
renderSkillDetailPage()
579+
const promptInput = await screen.findByPlaceholderText(
580+
'skill.skillManagement.detail.builder.placeholder',
581+
)
582+
const sendButton = screen.getByRole('button', {
583+
name: 'skill.skillManagement.detail.builder.send',
584+
})
585+
const suggestion = screen.getByRole('button', {
586+
name: 'skill.skillManagement.detail.builder.exampleIssueTriage',
587+
})
588+
await user.type(promptInput, 'Create a skill{Enter}')
589+
590+
expect(sendButton).toBeDisabled()
591+
expect(suggestion).toBeDisabled()
592+
expect(mocks.sendSkillAssistMessage).not.toHaveBeenCalled()
593+
expect(mocks.modelParameterRulesFetch).not.toHaveBeenCalled()
594+
595+
await act(async () => {
596+
resolveModels({ data: mocks.textGenerationModelList })
597+
await modelListResponse
598+
})
599+
await waitFor(() => expect(sendButton).toBeEnabled())
600+
expect(suggestion).toBeEnabled()
601+
await user.keyboard('{Enter}')
602+
603+
expect(mocks.sendSkillAssistMessage).toHaveBeenCalledWith(
604+
expect.objectContaining({ model: expect.objectContaining({ model: 'gpt-5.5' }) }),
605+
)
606+
expect(mocks.modelParameterRulesFetch).toHaveBeenCalledTimes(1)
607+
})
608+
609+
it('preserves user-edited parameters for an available Builder model', async () => {
610+
const user = userEvent.setup()
611+
mocks.modelParameterRulesFetch.mockImplementation(async () =>
612+
Response.json({
613+
data: [
614+
{
615+
name: 'temperature',
616+
label: { en_US: 'Temperature' },
617+
help: { en_US: 'Control randomness' },
618+
type: 'float',
619+
min: 0,
620+
max: 2,
621+
default: 0.7,
622+
precision: 1,
623+
},
624+
],
625+
}),
626+
)
627+
628+
renderSkillDetailPage()
629+
await user.click(
630+
await screen.findByRole('button', {
631+
name: 'common.modelProvider.modelSettings',
632+
}),
633+
)
634+
const dialog = await screen.findByRole('dialog', {
635+
name: 'common.modelProvider.modelSettings',
636+
})
637+
const temperature = await within(dialog).findByRole('spinbutton', { name: 'Temperature' })
638+
await user.clear(temperature)
639+
await user.type(temperature, '1.2')
640+
await user.click(within(dialog).getByRole('button', { name: 'common.operation.close' }))
641+
await user.type(
642+
screen.getByPlaceholderText('skill.skillManagement.detail.builder.modifyPlaceholder'),
643+
'Update the skill{Enter}',
644+
)
645+
646+
expect(mocks.sendSkillAssistMessage).toHaveBeenCalledWith(
647+
expect.objectContaining({
648+
model: {
649+
provider: 'langgenius/openai/openai',
650+
model: 'gpt-5.5',
651+
model_settings: { temperature: 1.2 },
652+
},
653+
}),
654+
)
655+
})
656+
450657
it('uploads image attachments in Skill Builder', async () => {
451658
const user = userEvent.setup({ applyAccept: false })
452659
mocks.uploadSkillFile.mockResolvedValue({

0 commit comments

Comments
 (0)