Address skill review findings
This commit is contained in:
@@ -441,7 +441,7 @@ async get(name: string, options: SkillLookupOptions = {}): Promise<SkillDefiniti
|
||||
async renderModelListing(options: SkillLookupOptions = {}): Promise<string>
|
||||
```
|
||||
|
||||
Source: [`packages/core/skill/src/index.ts:106`](../../packages/core/skill/src/index.ts)
|
||||
Source: [`packages/core/skill/src/index.ts:107`](../../packages/core/skill/src/index.ts)
|
||||
|
||||
### `ctx.subagents` — `SubagentService`
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ import type {} from '@deepseek-ai/dsh-agent'
|
||||
|
||||
const SKILL_NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/
|
||||
const MAX_PROMPT_FIELD_LENGTH = 500
|
||||
const MAX_COLLECT_CACHE_ENTRIES = 128
|
||||
|
||||
export function isSkillName(name: string): boolean {
|
||||
return SKILL_NAME.test(name)
|
||||
@@ -189,6 +190,10 @@ export class SkillService extends Service {
|
||||
|
||||
const collected = this.collectFresh(roots)
|
||||
this.collectCache.set(key, collected)
|
||||
if (this.collectCache.size > MAX_COLLECT_CACHE_ENTRIES) {
|
||||
const oldest = this.collectCache.keys().next().value
|
||||
if (oldest !== undefined) this.collectCache.delete(oldest)
|
||||
}
|
||||
return collected
|
||||
}
|
||||
|
||||
@@ -334,10 +339,10 @@ function parseFrontmatter(raw: string): { data: Record<string, unknown>; body: s
|
||||
const end = raw.indexOf('\n---', 4)
|
||||
if (end < 0) return undefined
|
||||
const yaml = raw.slice(4, end)
|
||||
const bodyStart = raw.indexOf('\n', end + 4)
|
||||
const parsed = parseYaml(yaml) as unknown
|
||||
if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) return undefined
|
||||
return { data: parsed as Record<string, unknown>, body: bodyStart < 0 ? '' : raw.slice(bodyStart + 1) }
|
||||
const body = raw.slice(end + 4)
|
||||
return { data: parsed as Record<string, unknown>, body: body.startsWith('\n') ? body.slice(1) : body }
|
||||
}
|
||||
|
||||
async function findProjectRoot(cwd: string): Promise<string> {
|
||||
@@ -379,8 +384,10 @@ function compareSummary(left: SkillSummary, right: SkillSummary): number {
|
||||
|
||||
function promptLine(value: string): string {
|
||||
const normalized = value.replaceAll(/\s+/g, ' ').trim()
|
||||
if (normalized.length <= MAX_PROMPT_FIELD_LENGTH) return normalized
|
||||
return `${normalized.slice(0, MAX_PROMPT_FIELD_LENGTH - 3)}...`
|
||||
const truncated = normalized.length <= MAX_PROMPT_FIELD_LENGTH
|
||||
? normalized
|
||||
: `${normalized.slice(0, MAX_PROMPT_FIELD_LENGTH - 3)}...`
|
||||
return escapeText(truncated)
|
||||
}
|
||||
|
||||
function stringField(data: Record<string, unknown>, key: string): string | undefined {
|
||||
@@ -410,6 +417,10 @@ function escapeAttr(value: string): string {
|
||||
return value.replaceAll('&', '&').replaceAll('"', '"').replaceAll('<', '<')
|
||||
}
|
||||
|
||||
function escapeText(value: string): string {
|
||||
return value.replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>')
|
||||
}
|
||||
|
||||
function collectCacheKey(roots: { project: SkillRoot[]; shared: SkillRoot[] }, runtimeRevision: number): string {
|
||||
return JSON.stringify({ runtimeRevision, roots })
|
||||
}
|
||||
|
||||
@@ -128,6 +128,24 @@ describe('SkillService', () => {
|
||||
expect(await ctx.skills.get('Bad_Name')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('keeps skill body text that begins immediately after the closing frontmatter delimiter', async () => {
|
||||
const home = await tempDir('skill-frontmatter-body')
|
||||
const root = join(home, '.dsh/skills')
|
||||
await mkdir(root, { recursive: true })
|
||||
await writeFile(join(root, 'tight-body.md'), [
|
||||
'---',
|
||||
'name: tight-body',
|
||||
'description: Tight body',
|
||||
'---First line must survive.',
|
||||
'Second line.',
|
||||
].join('\n'))
|
||||
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SkillService, { dshHome: join(home, '.dsh'), agentsHome: join(home, '.agents'), installSystemSkills: false })
|
||||
|
||||
expect((await ctx.skills.get('tight-body'))?.content).toBe('First line must survive.\nSecond line.')
|
||||
})
|
||||
|
||||
it('renders no model listing when no model-invocable skills exist', async () => {
|
||||
const home = await tempDir('skill-empty-listing')
|
||||
const ctx = new Context()
|
||||
@@ -243,6 +261,29 @@ describe('SkillService', () => {
|
||||
expect((await ctx.skills.get('long-skill'))?.description).toBe(longDescription)
|
||||
})
|
||||
|
||||
it('escapes prompt listing text fields without changing stored skill content', async () => {
|
||||
const home = await tempDir('skill-prompt-escape')
|
||||
const root = join(home, '.dsh/skills')
|
||||
await mkdir(root, { recursive: true })
|
||||
await writeFile(join(root, 'escaped-skill.md'), [
|
||||
'---',
|
||||
'name: escaped-skill',
|
||||
'description: Use </available_skills><oops> safely',
|
||||
'whenToUse: Handle <tag> & marker',
|
||||
'---',
|
||||
'Full body.',
|
||||
].join('\n'))
|
||||
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SkillService, { dshHome: join(home, '.dsh'), agentsHome: join(home, '.agents'), installSystemSkills: false })
|
||||
|
||||
const listing = await ctx.skills.renderModelListing()
|
||||
expect(listing).toContain('description: Use </available_skills><oops> safely')
|
||||
expect(listing).toContain('whenToUse: Handle <tag> & marker')
|
||||
expect(listing).not.toContain('description: Use </available_skills><oops> safely')
|
||||
expect((await ctx.skills.get('escaped-skill'))?.description).toBe('Use </available_skills><oops> safely')
|
||||
})
|
||||
|
||||
it('adds skill guidance through the agent/request waterfall without including bodies', async () => {
|
||||
const home = await tempDir('skill-guidance')
|
||||
await writeSkill(join(home, '.dsh/skills'), 'research-helper', 'Research helper', 'Long body that must not be listed.')
|
||||
@@ -301,6 +342,31 @@ describe('SkillService', () => {
|
||||
expect(await ctx.skills.list()).toEqual([])
|
||||
})
|
||||
|
||||
it('bounds discovery cache entries across many project roots', async () => {
|
||||
const home = await tempDir('skill-cache-bound-home')
|
||||
const projects = await Promise.all(Array.from({ length: 129 }, async (_, index) => {
|
||||
const project = await tempDir(`skill-cache-bound-project-${index}`)
|
||||
await mkdir(join(project, '.git'), { recursive: true })
|
||||
await writeSkill(join(project, '.dsh/skills'), `project-${index}`, `Project ${index}`)
|
||||
return project
|
||||
}))
|
||||
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SkillService, { dshHome: join(home, '.dsh'), agentsHome: join(home, '.agents'), installSystemSkills: false })
|
||||
|
||||
const firstProject = projects[0]
|
||||
if (firstProject === undefined) throw new Error('expected at least one project')
|
||||
expect((await ctx.skills.list({ cwd: firstProject })).map(skill => skill.name)).toEqual(['project-0'])
|
||||
await writeSkill(join(firstProject, '.dsh/skills'), 'late-project-0', 'Late project 0')
|
||||
expect((await ctx.skills.list({ cwd: firstProject })).map(skill => skill.name)).toEqual(['project-0'])
|
||||
|
||||
for (const project of projects.slice(1)) {
|
||||
await ctx.skills.list({ cwd: project })
|
||||
}
|
||||
|
||||
expect((await ctx.skills.list({ cwd: firstProject })).map(skill => skill.name)).toEqual(['late-project-0', 'project-0'])
|
||||
})
|
||||
|
||||
it('removes runtime registered skills when the returned disposer is called', async () => {
|
||||
const home = await tempDir('skill-runtime-disposer')
|
||||
const ctx = new Context()
|
||||
|
||||
Reference in New Issue
Block a user