fix(skills): propagate discovery cancellation

This commit is contained in:
Brennan Benson
2026-08-05 18:50:14 -07:00
parent 9ab5c9b070
commit 0edc792c72
10 changed files with 103 additions and 20 deletions
+18 -1
View File
@@ -7,7 +7,10 @@ vi.mock('../../../skills/skill-discovery-target', () => ({
}))
import { SKILL_METHODS } from './skills'
import { resolveSkillDiscoveryTarget } from '../../../skills/skill-discovery-target'
import {
discoverSkillsOnTarget,
resolveSkillDiscoveryTarget
} from '../../../skills/skill-discovery-target'
const WSL_RUNTIME = {
status: 'resolved',
@@ -65,4 +68,18 @@ describe('skills.discover RPC', () => {
expect.objectContaining({ projectRuntime: WSL_RUNTIME })
)
})
it('propagates request cancellation to native and WSL discovery targets', async () => {
const signal = new AbortController().signal
const context = makeContext({})
context.signal = signal
await discoverMethod().handler({ cwd: '/repo' }, context)
expect(vi.mocked(discoverSkillsOnTarget)).toHaveBeenLastCalledWith(
expect.objectContaining({ kind: 'native-host', cwd: '/repo' }),
[],
signal
)
})
})
+6 -2
View File
@@ -12,7 +12,7 @@ export const SKILL_METHODS: RpcMethod[] = [
defineMethod({
name: 'skills.discover',
params: SkillDiscoveryTargetSchema.default({}),
handler: async (params, { runtime }) => {
handler: async (params, { runtime, signal }) => {
// Why: the executing runtime owns WSL project preferences. Remote callers
// send worktree identity only; trusting their projectRuntime absence
// would scan this host's native filesystem for a WSL-configured project.
@@ -22,7 +22,11 @@ export const SKILL_METHODS: RpcMethod[] = [
...params,
projectRuntime: runtime.resolveProjectRuntimeForWorktree(params.worktreeId)
}
return discoverSkillsOnTarget(resolveSkillDiscoveryTarget(target), runtime.listRepos())
return discoverSkillsOnTarget(
resolveSkillDiscoveryTarget(target),
runtime.listRepos(),
signal
)
}
}),
// Why: a separate method, not a field on skills.discover — old runtimes strip
@@ -56,7 +56,11 @@ export function parseWslClaudePluginMetadataOutput(
return contents
}
function executeWslMetadataRead(distro: string, command: string): Promise<string> {
function executeWslMetadataRead(
distro: string,
command: string,
signal?: AbortSignal
): Promise<string> {
return new Promise((resolve, reject) => {
execFile(
'wsl.exe',
@@ -65,7 +69,8 @@ function executeWslMetadataRead(distro: string, command: string): Promise<string
encoding: 'utf8',
maxBuffer: WSL_METADATA_MAX_BUFFER_BYTES,
timeout: WSL_METADATA_TIMEOUT_MS,
windowsHide: true
windowsHide: true,
signal
},
(error, stdout) => {
if (error) {
@@ -82,15 +87,19 @@ export async function discoverClaudePluginSkillSourcesInWsl(args: {
distro: string
homeDir: string
cwd: string
signal?: AbortSignal
}): Promise<SkillScanRoot[]> {
args.signal?.throwIfAborted()
const paths = getClaudePluginMetadataPaths(args.homeDir, args.cwd, pathPosix)
const orderedPaths = [paths.installedPlugins, ...paths.settings]
// Why: plugin enablement and install paths belong to the distro just like
// SKILL.md identity; reading them through UNC could apply Windows semantics.
const output = await executeWslMetadataRead(
args.distro,
buildWslClaudePluginMetadataCommand(orderedPaths)
buildWslClaudePluginMetadataCommand(orderedPaths),
args.signal
)
args.signal?.throwIfAborted()
const [installedPlugins, ...settings] = parseWslClaudePluginMetadataOutput(
output,
orderedPaths.length
+4 -2
View File
@@ -81,12 +81,14 @@ describe('discoverPaneSkills', () => {
mocks.resolveSkillDiscoveryTarget.mockReturnValue(resolved)
mocks.discoverSkillsOnTarget.mockResolvedValue(EMPTY_RESULT)
const projectRuntime = { status: 'resolved' } as never
const signal = new AbortController().signal
const response = await discoverPaneSkills({
worktreeId: 'wt-1',
cwd: '/local/repo',
projectRuntime,
repos: []
repos: [],
signal
})
expect(response).toEqual({ status: 'ok', result: EMPTY_RESULT })
@@ -95,6 +97,6 @@ describe('discoverPaneSkills', () => {
worktreeId: 'wt-1',
projectRuntime
})
expect(mocks.discoverSkillsOnTarget).toHaveBeenCalledWith(resolved, [])
expect(mocks.discoverSkillsOnTarget).toHaveBeenCalledWith(resolved, [], signal)
})
})
+4 -1
View File
@@ -37,5 +37,8 @@ export async function discoverPaneSkills(args: {
worktreeId: args.worktreeId,
...(args.projectRuntime ? { projectRuntime: args.projectRuntime } : {})
})
return { status: 'ok', result: await discoverSkillsOnTarget(target, args.repos) }
return {
status: 'ok',
result: await discoverSkillsOnTarget(target, args.repos, args.signal)
}
}
+6 -4
View File
@@ -55,16 +55,18 @@ export function resolveSkillDiscoveryTarget(
export async function discoverSkillsOnTarget(
target: ResolvedSkillDiscoveryTarget,
repos: readonly Repo[]
repos: readonly Repo[],
signal?: AbortSignal
): Promise<SkillDiscoveryResult> {
if (target.kind === 'wsl') {
return discoverSkillsInWsl({
distro: target.distro,
homeDir: target.homeDir,
cwd: target.cwd
cwd: target.cwd,
signal
})
}
return target.cwd
? discoverSkills({ repos: [], cwd: target.cwd })
: discoverSkills({ repos: [...repos] })
? discoverSkills({ repos: [], cwd: target.cwd, signal })
: discoverSkills({ repos: [...repos], signal })
}
@@ -22,6 +22,21 @@ function completeExecFileCall(callIndex: number, stdout: string): void {
describe('WSL Claude plugin skill discovery', () => {
beforeEach(() => execFileMock.mockReset())
it('does not start a distro process when discovery is already aborted', async () => {
const controller = new AbortController()
controller.abort()
await expect(
discoverSkillsInWsl({
distro: 'Ubuntu',
homeDir: '/home/alice',
cwd: '/work/orca',
signal: controller.signal
})
).rejects.toMatchObject({ name: 'AbortError' })
expect(execFileMock).not.toHaveBeenCalled()
})
it('reads enabled plugin metadata and scans the selected install inside the distro', async () => {
const homeDir = '/home/alice'
const cwd = '/work/orca'
@@ -59,10 +74,13 @@ describe('WSL Claude plugin skill discovery', () => {
execFileMock.mockImplementationOnce((..._args: unknown[]) => {
queueMicrotask(() => completeExecFileCall(1, scanOutput))
})
const signal = new AbortController().signal
const result = await discoverSkillsInWsl({ distro: 'Ubuntu', homeDir, cwd })
const result = await discoverSkillsInWsl({ distro: 'Ubuntu', homeDir, cwd, signal })
expect(execFileMock).toHaveBeenCalledTimes(2)
expect(execFileMock.mock.calls[0]?.[2]).toEqual(expect.objectContaining({ signal }))
expect(execFileMock.mock.calls[1]?.[2]).toEqual(expect.objectContaining({ signal }))
const scanArgs = execFileMock.mock.calls[1]?.[1] as string[]
const encoded = /printf %s '([^']+)'/.exec(scanArgs[5] ?? '')?.[1]
const scanScript = Buffer.from(encoded ?? '', 'base64').toString('utf8')
+17 -3
View File
@@ -58,7 +58,11 @@ export function buildWslSkillDiscoveryCommand(roots: readonly SkillScanRoot[]):
return buildEncodedWslBashCommand(lines.join('\n'))
}
function executeWslSkillDiscovery(distro: string, command: string): Promise<string> {
function executeWslSkillDiscovery(
distro: string,
command: string,
signal?: AbortSignal
): Promise<string> {
return new Promise((resolve, reject) => {
execFile(
'wsl.exe',
@@ -67,7 +71,8 @@ function executeWslSkillDiscovery(distro: string, command: string): Promise<stri
encoding: 'utf8',
maxBuffer: WSL_SCAN_MAX_BUFFER_BYTES,
timeout: WSL_SCAN_TIMEOUT_MS,
windowsHide: true
windowsHide: true,
signal
},
(error, stdout) => {
if (error) {
@@ -180,7 +185,9 @@ export async function discoverSkillsInWsl(args: {
distro: string
homeDir: string
cwd: string
signal?: AbortSignal
}): Promise<SkillDiscoveryResult> {
args.signal?.throwIfAborted()
// Plugin roots are resolved (in JS) from metadata this first wsl.exe call
// reads, then fed to the scan's own wsl.exe call below — two sequential
// process boots. That is a deliberate one-time-per-pane cost (the renderer
@@ -194,6 +201,7 @@ export async function discoverSkillsInWsl(args: {
try {
pluginRoots = await discoverClaudePluginSkillSourcesInWsl(args)
} catch {
args.signal?.throwIfAborted()
pluginRoots = []
}
const roots = [
@@ -207,6 +215,12 @@ export async function discoverSkillsInWsl(args: {
]
// Why: UNC traversal applies Windows casing and symlink rules. The distro
// must own enumeration, metadata reads, and canonical path identity.
const output = await executeWslSkillDiscovery(args.distro, buildWslSkillDiscoveryCommand(roots))
args.signal?.throwIfAborted()
const output = await executeWslSkillDiscovery(
args.distro,
buildWslSkillDiscoveryCommand(roots),
args.signal
)
args.signal?.throwIfAborted()
return parseWslSkillDiscoveryOutput(output, roots)
}
@@ -52,10 +52,10 @@ function setup(autocomplete: ComposerAutocomplete = picker(), composing = false)
return { handler: hook.result.current, callbacks }
}
function keyEvent(key: string, isComposing = false) {
function keyEvent(key: string, isComposing = false, shiftKey = false) {
return {
key,
shiftKey: false,
shiftKey,
keyCode: isComposing ? 229 : 0,
nativeEvent: { isComposing },
preventDefault: vi.fn()
@@ -75,6 +75,17 @@ describe('useNativeChatComposerKeyDown', () => {
expect(tab.callbacks.dispatchPickerCommand).not.toHaveBeenCalled()
})
it('leaves Shift+Tab available for keyboard focus navigation', () => {
const { handler, callbacks } = setup()
const event = keyEvent('Tab', false, true)
handler(event as never)
expect(event.preventDefault).not.toHaveBeenCalled()
expect(callbacks.completePickerItem).not.toHaveBeenCalled()
expect(callbacks.dispatchPickerCommand).not.toHaveBeenCalled()
})
it('falls through to composer send when the open picker has no options', () => {
const { handler, callbacks } = setup(picker([]))
handler(keyEvent('Enter') as never)
@@ -63,7 +63,10 @@ export function useNativeChatComposerKeyDown({
setActiveSuggestion((index) => (index - 1 + items.length) % items.length)
return
}
if ((event.key === 'Enter' || event.key === 'Tab') && items.length > 0) {
if (
(event.key === 'Enter' || (event.key === 'Tab' && !event.shiftKey)) &&
items.length > 0
) {
event.preventDefault()
const item = items[activeSuggestion] ?? items[0]
if (event.key === 'Enter' && item.kind === 'command') {