Skip to content

Commit 0acf1f5

Browse files
committed
fix: harden skills registry commands
1 parent d9015c5 commit 0acf1f5

2 files changed

Lines changed: 39 additions & 8 deletions

File tree

src/server/skillsRoutes.ts

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { homedir, tmpdir } from 'node:os'
66
import { join } from 'node:path'
77
import { writeFile } from 'node:fs/promises'
88
import { resolvePythonCommand, resolveSkillInstallerScriptPath } from '../commandResolution.js'
9+
import { getSpawnInvocation } from '../utils/commandInvocation.js'
910

1011
type AppServerLike = {
1112
rpc(method: string, params: unknown): Promise<unknown>
@@ -114,11 +115,14 @@ function getSkillsInstallDir(): string {
114115
}
115116

116117
const DEFAULT_COMMAND_TIMEOUT_MS = 120_000
118+
const SKILL_SEARCH_METADATA_LIMIT = 20
119+
const SKILL_SEARCH_METADATA_CONCURRENCY = 4
117120

118121
async function runCommand(command: string, args: string[], options: { cwd?: string; timeoutMs?: number } = {}): Promise<void> {
119122
const timeout = options.timeoutMs ?? DEFAULT_COMMAND_TIMEOUT_MS
120123
await new Promise<void>((resolve, reject) => {
121-
const proc = spawn(command, args, {
124+
const invocation = getSpawnInvocation(command, args)
125+
const proc = spawn(invocation.command, invocation.args, {
122126
cwd: options.cwd,
123127
env: process.env,
124128
stdio: ['ignore', 'pipe', 'pipe'],
@@ -158,7 +162,8 @@ async function runCommand(command: string, args: string[], options: { cwd?: stri
158162
async function runCommandWithOutput(command: string, args: string[], options: { cwd?: string; timeoutMs?: number } = {}): Promise<string> {
159163
const timeout = options.timeoutMs ?? DEFAULT_COMMAND_TIMEOUT_MS
160164
return await new Promise<string>((resolve, reject) => {
161-
const proc = spawn(command, args, {
165+
const invocation = getSpawnInvocation(command, args)
166+
const proc = spawn(invocation.command, invocation.args, {
162167
cwd: options.cwd,
163168
env: process.env,
164169
stdio: ['ignore', 'pipe', 'pipe'],
@@ -401,16 +406,39 @@ async function fetchGithubSkillMetadata(source: string): Promise<Partial<Pick<Sk
401406
return { avatarUrl: getGithubOwnerAvatarUrl(source) }
402407
}
403408

409+
async function mapWithConcurrency<T, R>(
410+
items: T[],
411+
concurrency: number,
412+
mapper: (item: T, index: number) => Promise<R>,
413+
): Promise<R[]> {
414+
const results = new Array<R>(items.length)
415+
let nextIndex = 0
416+
const workerCount = Math.max(1, Math.min(concurrency, items.length))
417+
await Promise.all(Array.from({ length: workerCount }, async () => {
418+
while (nextIndex < items.length) {
419+
const index = nextIndex
420+
nextIndex += 1
421+
results[index] = await mapper(items[index] as T, index)
422+
}
423+
}))
424+
return results
425+
}
426+
404427
async function enrichSkillSearchDescriptions(results: SkillHubEntry[]): Promise<SkillHubEntry[]> {
405-
return await Promise.all(results.map(async (result) => {
428+
const enrichedHead = await mapWithConcurrency(
429+
results.slice(0, SKILL_SEARCH_METADATA_LIMIT),
430+
SKILL_SEARCH_METADATA_CONCURRENCY,
431+
async (result) => {
406432
if (!result.source) return result
407433
const metadata = await fetchGithubSkillMetadata(result.source)
408434
return {
409435
...result,
410436
description: metadata.description || result.description,
411437
avatarUrl: metadata.avatarUrl || result.avatarUrl,
412438
}
413-
}))
439+
},
440+
)
441+
return [...enrichedHead, ...results.slice(SKILL_SEARCH_METADATA_LIMIT)]
414442
}
415443

416444
type RpcSkillRecord = {
@@ -1327,7 +1355,7 @@ export async function handleSkillsRoutes(
13271355
return true
13281356
}
13291357
const installedMap = await collectInstalledSkillsMap(appServer)
1330-
const output = await runCommandWithOutput('npx', ['skills', 'find', query], { timeoutMs: 60_000 })
1358+
const output = await runCommandWithOutput('npx', ['--yes', 'skills', 'find', query], { timeoutMs: 60_000 })
13311359
const results = await enrichSkillSearchDescriptions(parseNpxSkillsFindOutput(output, installedMap))
13321360
setJson(res, 200, { results })
13331361
} catch (error) {
@@ -1560,7 +1588,7 @@ export async function handleSkillsRoutes(
15601588
setJson(res, 400, { error: 'Missing or invalid skill source' })
15611589
return true
15621590
}
1563-
await runCommand('npx', ['skills', 'add', installSource, '--yes', '--global'], { timeoutMs: 120_000 })
1591+
await runCommand('npx', ['--yes', 'skills', 'add', installSource, '--yes', '--global'], { timeoutMs: 120_000 })
15641592
try { await withTimeout(appServer.rpc('skills/list', { forceReload: true }), 10_000, 'skills/list reload') } catch {}
15651593
const installedMap = await collectInstalledSkillsMap(appServer)
15661594
const installed = installedMap.get(name || installSource.slice(installSource.lastIndexOf('@') + 1))

tests.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3423,11 +3423,11 @@ The Skills tab includes a registry search panel backed by `npx skills find`, sho
34233423
3. Verify the `Find skills` header shows a `Skills directory` link on the right that opens `https://skills.anyclaw.store/` in a new tab
34243424
4. In `Find skills`, type a query such as `browser`
34253425
5. Click `Search`
3426-
6. Verify the app calls `/codex-api/skills-hub/search?q=browser`, which runs `npx skills find browser`
3426+
6. Verify the app calls `/codex-api/skills-hub/search?q=browser`, which runs `npx --yes skills find browser`
34273427
7. Verify `Search results (count)` appears above `Installed skills (count)`
34283428
8. Verify each registry result card shows its install count metadata, such as `1.2K installs`, even when a GitHub `SKILL.md` description is shown
34293429
9. Open one GitHub-backed result and verify the detail modal shows the skill name, owner/repository, parsed `SKILL.md` description, GitHub-backed icon/avatar, and external link
3430-
10. Click `Install` for a result and verify the backend runs `npx skills add <owner/repo@skill> --yes --global`
3430+
10. Click `Install` for a result and verify the backend runs `npx --yes skills add <owner/repo@skill> --yes --global`
34313431
11. After install, verify the result becomes installed and the installed skills list refreshes from local installed skill data rather than appending the remote registry card
34323432
12. Switch to dark theme and repeat the search visibility check
34333433
13. Search for an already-installed skill and verify its search result shows `Installed`
@@ -3440,11 +3440,14 @@ The Skills tab includes a registry search panel backed by `npx skills find`, sho
34403440

34413441
#### Expected Results
34423442
- Search results are parsed from the real `npx skills find` output, not a static catalog
3443+
- Skills search/install commands use the repo command invocation wrapper so `npx` starts reliably on Windows
3444+
- Skills search/install commands include outer `npx --yes` so first-run package prompts cannot hang with ignored stdin
34433445
- The Skills directory link is visible beside Find skills in light and dark theme and opens the public directory in a new tab
34443446
- Registry installs run noninteractively with `--yes --global`, so the process cannot stop at the agent-selection prompt and falsely report success
34453447
- Registry install responses only return `ok: true` when the local installed `SKILL.md` path is found and validates successfully
34463448
- The UI treats a missing returned path or missing post-refresh local skill as an install failure instead of showing the remote registry card as installed
34473449
- GitHub-backed results fetch the repository `SKILL.md` and show its `description` frontmatter when available, falling back to the install count when unavailable
3450+
- GitHub metadata enrichment is bounded to the first 20 results with limited concurrency, so broad searches still return without unbounded raw GitHub fetch fanout
34483451
- Search result cards keep the registry install count visible as card metadata even when GitHub enrichment replaces the fallback description
34493452
- GitHub-backed results show an explicit frontmatter `icon` when provided, otherwise they show the GitHub repository owner avatar instead of a generic letter fallback
34503453
- The search UI does not replace or hide local installed skills

0 commit comments

Comments
 (0)