fix: harden runtime skill registration

This commit is contained in:
Yichen Jiang
2026-07-06 10:21:00 +08:00
parent 545b2775db
commit b95595f0c7
4 changed files with 62 additions and 11 deletions

View File

@@ -71,7 +71,7 @@ interface SkillLookupOptions {
}
```
The service can be pointed at alternate user roots in tests or deployments. `installSystemSkills` controls whether bundled system skills are materialized under `<dshHome>/skills/.system` on startup.
The service can be pointed at alternate user roots in tests or deployments. `installSystemSkills` controls whether bundled system skills are materialized under `<dshHome>/skills/.system` on startup. `promptFieldMaxLength` must be at least `3`, matching the `...` truncation suffix reserved in rendered prompt fields.
```ts type-equiv
interface Config {

View File

@@ -8,7 +8,7 @@ Agent skill discovery and model-facing skill guidance.
- `ctx.skills.list({ cwd? })` Returns model-invocable skill summaries for the current workspace.
- `ctx.skills.get(name, { cwd? })` Returns the full skill, including disabled-for-model skills.
- `ctx.skills.register(skill): () => void` Registers a runtime skill, disposed with the calling fiber.
- `ctx.skills.register(skill): () => void` Registers a runtime skill, disposed with the calling fiber. Same-name runtime registrations are first-wins: a duplicate logs a warning and gets a no-op disposer.
### Config
@@ -18,7 +18,7 @@ Agent skill discovery and model-facing skill guidance.
| `agentsHome` | `$DSH_AGENTS_HOME` or `~/.agents` | Shared agent config root scanned for compatible skills. |
| `extraRoots` | `[]` | Additional skill roots scanned after user roots and before system skills. |
| `installSystemSkills` | `true` | Whether startup materializes bundled system skills under `dshHome`. |
| `promptFieldMaxLength` | `500` | Maximum rendered `description` / `whenToUse` length in the prompt listing. |
| `promptFieldMaxLength` | `500` | Maximum rendered `description` / `whenToUse` length in the prompt listing; must be at least `3` because truncated fields reserve `...`. |
| `collectCacheMaxEntries` | `128` | Maximum cwd/root discovery promises kept in memory. |
### Discovery
@@ -39,7 +39,7 @@ The project root is the nearest ancestor containing `.git`; without one, the cur
When `ctx.fs` is available, discovery lists roots through `ctx.fs.listDir`, reads skill files through `ctx.fs.readText`, and installs system skills through `ctx.fs.writeText`. Without a filesystem service, the package falls back to Node filesystem I/O so the service can still run in minimal test contexts. Missing, unreadable, or malformed skill files warn and skip instead of failing the whole request.
Discovery is memoized per resolved root set and runtime-skill revision. Runtime `register()` and disposer calls invalidate the cache; disk-only changes are picked up on the next invalidation or process restart.
Discovery is memoized per resolved root set and runtime-skill revision. Runtime `register()` and active disposer calls invalidate the cache; duplicate runtime registrations do not alter the active set. Disk-only changes are picked up on the next invalidation or process restart.
## Skill Format

View File

@@ -75,7 +75,7 @@ export interface Config {
extraRoots?: string[]
/** Ensure bundled system skills exist under `<dshHome>/skills/.system`. Defaults true. */
installSystemSkills?: boolean
/** Maximum rendered description/whenToUse length in the prompt listing. */
/** Maximum rendered description/whenToUse length in the prompt listing; minimum 3. */
promptFieldMaxLength?: number
/** Maximum number of cwd/root discovery promises kept in the in-memory cache. */
collectCacheMaxEntries?: number
@@ -159,7 +159,7 @@ export class SkillService extends Service {
this.installSystemSkills = config.installSystemSkills ?? true
this.promptFieldMaxLength = config.promptFieldMaxLength ?? DEFAULT_PROMPT_FIELD_LENGTH
this.collectCacheMaxEntries = config.collectCacheMaxEntries ?? DEFAULT_COLLECT_CACHE_ENTRIES
assertPositiveInteger('promptFieldMaxLength', this.promptFieldMaxLength)
assertPositiveInteger('promptFieldMaxLength', this.promptFieldMaxLength, 3)
assertPositiveInteger('collectCacheMaxEntries', this.collectCacheMaxEntries)
if (this.installSystemSkills) {
const systemRoot = join(this.dshHome, 'skills/.system')
@@ -178,11 +178,18 @@ export class SkillService extends Service {
/**
* Register a runtime skill contribution.
* Same-name runtime registrations are first-wins: a duplicate logs a warning
* and returns a no-op disposer so it cannot remove the active contribution.
* @param skill - the complete skill definition to expose for discovery.
* @returns a disposer that removes the runtime skill and invalidates caches.
* @returns a disposer that removes this runtime contribution and invalidates caches.
*/
register(skill: SkillRegistration): () => void {
const normalized = normalizeSkill(skill)
const existing = this.runtime.get(normalized.name)
if (existing !== undefined) {
this.ctx.logger.warn(`runtime skill "${normalized.name}" from ${normalized.source} ignored because it is already registered from ${existing.source}`)
return () => {}
}
const dispose = this.ctx.effect(function* (this: SkillService) {
this.runtime.set(normalized.name, normalized)
this.invalidateCache()
@@ -587,9 +594,9 @@ function promptLine(value: string, maxLength: number): string {
return escapeText(truncated)
}
function assertPositiveInteger(name: string, value: number): void {
if (!Number.isInteger(value) || value < 1) {
throw new Error(`skill: ${name} must be a positive integer`)
function assertPositiveInteger(name: string, value: number, minimum = 1): void {
if (!Number.isInteger(value) || value < minimum) {
throw new Error(`skill: ${name} must be an integer greater than or equal to ${minimum}`)
}
}

View File

@@ -1,4 +1,4 @@
import { describe, expect, it } from 'vitest'
import { describe, expect, it, vi } from 'vitest'
import { mkdir, readdir, readFile, stat, symlink, writeFile } from 'node:fs/promises'
import { dirname, join } from 'node:path'
import { tmpdir } from 'node:os'
@@ -331,6 +331,12 @@ describe('SkillService', () => {
installSystemSkills: false,
promptFieldMaxLength: 0,
})).rejects.toThrow('promptFieldMaxLength')
await expect(ctx.plugin(SkillService, {
dshHome: join(home, '.dsh'),
agentsHome: join(home, '.agents'),
installSystemSkills: false,
promptFieldMaxLength: 2,
})).rejects.toThrow('greater than or equal to 3')
await expect(ctx.plugin(SkillService, {
dshHome: join(home, '.dsh'),
agentsHome: join(home, '.agents'),
@@ -661,6 +667,44 @@ describe('SkillService', () => {
expect(await ctx.skills.list()).toEqual([])
})
it('keeps the first runtime skill when a duplicate name is registered', async () => {
const home = await tempDir('skill-runtime-duplicate')
const ctx = new Context()
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => undefined)
await ctx.plugin(SkillService, { dshHome: join(home, '.dsh'), agentsHome: join(home, '.agents'), installSystemSkills: false })
const firstDispose = ctx.skills.register({
name: 'same-runtime',
description: 'first',
content: 'First body.',
directory: 'memory://first',
source: 'runtime',
})
const duplicateDispose = ctx.skills.register({
name: 'same-runtime',
description: 'second',
content: 'Second body.',
directory: 'memory://second',
source: 'runtime',
})
await expect(ctx.skills.get('same-runtime')).resolves.toMatchObject({
description: 'first',
content: 'First body.',
directory: 'memory://first',
})
expect(warn).toHaveBeenCalledWith(expect.stringContaining('runtime skill "same-runtime"'))
duplicateDispose()
await expect(ctx.skills.get('same-runtime')).resolves.toMatchObject({
description: 'first',
content: 'First body.',
})
firstDispose()
await expect(ctx.skills.get('same-runtime')).resolves.toBeUndefined()
})
it('rejects invalid runtime skill registrations', async () => {
const home = await tempDir('skill-runtime-invalid')
const ctx = new Context()