fix(skill): forward cancellation to local reads
This commit is contained in:
@@ -30,7 +30,7 @@ Default roots are resolved in this provider's rank order:
|
||||
|
||||
The project root is the nearest ancestor containing `.git`; without one, the current cwd is used. The user DSH root skips its `.system` child so system-owned directories are not accidentally treated as normal user skills. DeepSeek Harness no longer ships built-in system skills from this provider; additional built-ins can be supplied later by another provider.
|
||||
|
||||
When `ctx.fs` is available, discovery lists roots through `ctx.fs.listDir`, reads skill files through `ctx.fs.readText`, and probes `.git` through the filesystem service. Without a filesystem service, the provider falls back to Node filesystem I/O so minimal local contexts can still load skills. Missing, unreadable, or malformed skill files warn and skip instead of failing the whole request.
|
||||
When `ctx.fs` is available, discovery lists roots through `ctx.fs.listDir`, reads skill files through `ctx.fs.readText`, and probes `.git` through the filesystem service. Full skill loads forward the lookup abort signal to filesystem metadata and content reads. Without a filesystem service, the provider falls back to abortable Node filesystem I/O so minimal local contexts can still load skills. Missing, unreadable, or malformed skill files warn and skip instead of failing the whole request.
|
||||
|
||||
## Skill Format
|
||||
|
||||
|
||||
@@ -116,11 +116,12 @@ export class LocalSkillProvider implements SkillProvider {
|
||||
/**
|
||||
* Load a complete local skill body from the candidate's file locator.
|
||||
* @param candidate - the winning candidate returned by this provider.
|
||||
* @param options - lookup options whose signal cancels filesystem reads.
|
||||
* @returns the full local skill, or `undefined` if the file disappeared.
|
||||
*/
|
||||
async get(candidate: SkillCandidate): Promise<SkillDefinition | undefined> {
|
||||
async get(candidate: SkillCandidate, options: SkillLookupOptions): Promise<SkillDefinition | undefined> {
|
||||
const locator = candidate.locator as LocalLocator
|
||||
const parsed = await parseSkillFile(locator.path, this.ctx)
|
||||
const parsed = await parseSkillFile(locator.path, this.ctx, options.signal)
|
||||
if (parsed === undefined) return undefined
|
||||
return {
|
||||
name: parsed.name,
|
||||
@@ -223,8 +224,9 @@ async function listSkillRootEntriesFromNode(root: SkillRoot, ctx: Context): Prom
|
||||
return result
|
||||
}
|
||||
|
||||
async function parseSkillFile(path: string, ctx: Context): Promise<ParsedSkill | undefined> {
|
||||
const raw = await readSkillText(ctx, path)
|
||||
async function parseSkillFile(path: string, ctx: Context, signal?: AbortSignal): Promise<ParsedSkill | undefined> {
|
||||
const raw = await readSkillText(ctx, path, signal)
|
||||
signal?.throwIfAborted()
|
||||
if (raw === undefined) {
|
||||
return undefined
|
||||
}
|
||||
@@ -263,30 +265,39 @@ function optionalFileSystem(ctx: Context): FileSystem | undefined {
|
||||
return ctx.get('fs')
|
||||
}
|
||||
|
||||
async function readSkillText(ctx: Context, path: string): Promise<string | undefined> {
|
||||
async function readSkillText(ctx: Context, path: string, signal?: AbortSignal): Promise<string | undefined> {
|
||||
signal?.throwIfAborted()
|
||||
const fs = optionalFileSystem(ctx)
|
||||
if (fs !== undefined) {
|
||||
return await readSkillTextFromFileSystem(ctx, fs, path)
|
||||
return await readSkillTextFromFileSystem(ctx, fs, path, signal)
|
||||
}
|
||||
try {
|
||||
return await readFile(path, 'utf8')
|
||||
return await readFile(path, { encoding: 'utf8', signal })
|
||||
} catch {
|
||||
signal?.throwIfAborted()
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
async function readSkillTextFromFileSystem(ctx: Context, fs: FileSystem, path: string): Promise<string | undefined> {
|
||||
async function readSkillTextFromFileSystem(ctx: Context, fs: FileSystem, path: string, signal?: AbortSignal): Promise<string | undefined> {
|
||||
// A missing or temporarily inaccessible skill file is not fatal to discovery.
|
||||
signal?.throwIfAborted()
|
||||
const target = await fs.resolve(path).catch(() => undefined)
|
||||
signal?.throwIfAborted()
|
||||
if (target === undefined) return undefined
|
||||
const info = await fs.stat(target).catch((error: unknown) => {
|
||||
let info
|
||||
try {
|
||||
info = await fs.stat(target, signal)
|
||||
} catch (error) {
|
||||
signal?.throwIfAborted()
|
||||
ctx.logger.warn(`skill file ${path} ignored: failed to stat through filesystem service: ${errorMessage(error)}`)
|
||||
return undefined
|
||||
})
|
||||
}
|
||||
if (info === undefined || info.type !== 'file') return undefined
|
||||
try {
|
||||
return await fs.readText(target)
|
||||
return await fs.readText(target, signal)
|
||||
} catch (error) {
|
||||
signal?.throwIfAborted()
|
||||
ctx.logger.warn(`skill file ${path} ignored: ${fsReadErrorMessage(target, error)}`)
|
||||
return undefined
|
||||
}
|
||||
|
||||
@@ -27,13 +27,17 @@ class TestFileSystem extends FileSystem {
|
||||
failResolvePaths = new Set<string>()
|
||||
failStatPaths = new Set<string>()
|
||||
statOverrides = new Map<string, FsInfo | undefined>()
|
||||
statSignals: Array<AbortSignal | undefined> = []
|
||||
readTextSignals: Array<AbortSignal | undefined> = []
|
||||
readTextOverride?: (target: FsTarget, signal?: AbortSignal) => Promise<string>
|
||||
|
||||
override async resolve(path: string): Promise<FsTarget> {
|
||||
if (this.failResolvePaths.has(path)) throw new Error('resolve failed')
|
||||
return { targetKey: path as never, displayPath: path }
|
||||
}
|
||||
|
||||
override async stat(target: FsTarget): Promise<FsInfo | undefined> {
|
||||
override async stat(target: FsTarget, signal?: AbortSignal): Promise<FsInfo | undefined> {
|
||||
this.statSignals.push(signal)
|
||||
if (this.failStatPaths.has(target.displayPath)) throw new Error('stat failed')
|
||||
if (this.statOverrides.has(target.displayPath)) return this.statOverrides.get(target.displayPath)
|
||||
try {
|
||||
@@ -49,7 +53,9 @@ class TestFileSystem extends FileSystem {
|
||||
}
|
||||
}
|
||||
|
||||
override async readText(target: FsTarget): Promise<string> {
|
||||
override async readText(target: FsTarget, signal?: AbortSignal): Promise<string> {
|
||||
this.readTextSignals.push(signal)
|
||||
if (this.readTextOverride !== undefined) return await this.readTextOverride(target, signal)
|
||||
const text = await readFile(target.displayPath, 'utf8')
|
||||
if (text.includes('\uFFFD')) throw new Error('not text')
|
||||
return text
|
||||
@@ -317,6 +323,41 @@ describe('LocalSkillProvider', () => {
|
||||
expect(await ctx.skills.get('binary-skill')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('forwards cancellation to filesystem reads while loading a skill', async () => {
|
||||
const home = await tempDir('skill-read-abort')
|
||||
await writeSkill(join(home, '.dsh/skills'), 'abortable-skill', 'Abortable skill')
|
||||
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(TestFileSystem)
|
||||
const fs = ctx.fs as TestFileSystem
|
||||
await ctx.plugin(SkillService)
|
||||
await ctx.plugin(SkillLocal, { dshHome: join(home, '.dsh'), agentsHome: join(home, '.agents') })
|
||||
expect((await ctx.skills.list()).map(skill => skill.name)).toEqual(['abortable-skill'])
|
||||
|
||||
fs.statSignals = []
|
||||
fs.readTextSignals = []
|
||||
const started = Promise.withResolvers<undefined>()
|
||||
fs.readTextOverride = async (_target, signal) => {
|
||||
if (signal === undefined) throw new Error('expected the skill lookup signal')
|
||||
started.resolve(undefined)
|
||||
return await new Promise<string>((_resolve, reject) => {
|
||||
signal.addEventListener('abort', () => {
|
||||
const abortReason = signal.reason as unknown
|
||||
reject(abortReason instanceof Error ? abortReason : new Error(String(abortReason)))
|
||||
}, { once: true })
|
||||
})
|
||||
}
|
||||
const controller = new AbortController()
|
||||
const reason = new Error('turn cancelled')
|
||||
const loading = ctx.skills.get('abortable-skill', { signal: controller.signal })
|
||||
await started.promise
|
||||
controller.abort(reason)
|
||||
|
||||
await expect(loading).rejects.toBe(reason)
|
||||
expect(fs.statSignals).toEqual([controller.signal])
|
||||
expect(fs.readTextSignals).toEqual([controller.signal])
|
||||
})
|
||||
|
||||
it('uses default home root resolution without exposing builtin skills', async () => {
|
||||
const previousDshHome = process.env.DSH_HOME
|
||||
const previousAgentsHome = process.env.DSH_AGENTS_HOME
|
||||
|
||||
Reference in New Issue
Block a user