diff --git a/.gitignore b/.gitignore index f1c88cb..20dac14 100644 --- a/.gitignore +++ b/.gitignore @@ -46,3 +46,6 @@ packages/cli/scene/**/outputs/ # Environment variables (sensitive data) .env + +# Local scratch / plan drafts (never commit) +.scratch/ diff --git a/packages/cli/postinstall.js b/packages/cli/postinstall.js index ee99f66..a502a0b 100644 --- a/packages/cli/postinstall.js +++ b/packages/cli/postinstall.js @@ -116,7 +116,11 @@ async function extractTarBr(tarBrBuffer, destDir) { extract.on("entry", (header, stream, next) => { if (!isSafeEntryName(header.name)) { - next(new Error(`unsafe tar entry: ${header.name}`)); + // Same semantics as core skills/extract.ts: destroy so the pipeline rejects with this + // error; silence the entry stream to avoid its companion error becoming unhandled + stream.on("error", () => {}); + stream.resume(); + extract.destroy(new Error(`unsafe tar entry: ${header.name}`)); return; } const filePath = join(destDir, header.name); diff --git a/packages/commands/src/commands/skill/add.ts b/packages/commands/src/commands/skill/add.ts index 1cea7ff..5cb6f97 100644 --- a/packages/commands/src/commands/skill/add.ts +++ b/packages/commands/src/commands/skill/add.ts @@ -6,13 +6,13 @@ import { detectInstalledAgents, fetchSkillsIndex, getSkillRegistryBaseUrl, - installSkill, - linkSkillToAgents, + installSkillWithFanout, + parseSkillNames, readSkillLock, + runWithConcurrency, writeSkillLock, } from "bailian-cli-core"; import { emitBare, emitResult, formatTable } from "bailian-cli-runtime"; -import { parseSkillNames, runWithConcurrency } from "./shared.ts"; interface AddOutcome { name: string; @@ -56,22 +56,13 @@ export default defineCommand({ return { name, status: "failed", reason: "skill not found in registry" }; } try { - await installSkill(name, entry); - const links = linkSkillToAgents(name, agents); - const effective = links.filter((link) => link.mode !== "skipped"); - lock.skills[name] = { - ...(entry.contentHash ? { contentHash: entry.contentHash } : {}), - ...(entry.publishedAt ? { publishedAt: entry.publishedAt } : {}), - installedAt: new Date().toISOString(), - sourceType: "oss", - ...(entry.description ? { description: entry.description } : {}), - links: effective.map((link) => link.path), - }; + const record = await installSkillWithFanout(name, entry, agents); + lock.skills[name] = record.lockEntry; return { name, status: "installed", publishedAt: entry.publishedAt, - agents: effective.map((link) => link.agent), + agents: record.linkedAgents, }; } catch (err) { return { diff --git a/packages/commands/src/commands/skill/remove.ts b/packages/commands/src/commands/skill/remove.ts index b6f4116..e51106c 100644 --- a/packages/commands/src/commands/skill/remove.ts +++ b/packages/commands/src/commands/skill/remove.ts @@ -4,13 +4,13 @@ import { defineCommand, detectOutputFormat, listSkillDirsOnDisk, + parseSkillNames, readSkillLock, removeSkillDir, unlinkSkillFromAgents, writeSkillLock, } from "bailian-cli-core"; import { emitBare, emitResult, formatTable } from "bailian-cli-runtime"; -import { parseSkillNames } from "./shared.ts"; interface RemoveOutcome { name: string; diff --git a/packages/commands/src/commands/skill/update.ts b/packages/commands/src/commands/skill/update.ts index e2d6bca..127e641 100644 --- a/packages/commands/src/commands/skill/update.ts +++ b/packages/commands/src/commands/skill/update.ts @@ -6,14 +6,14 @@ import { detectInstalledAgents, fetchSkillsIndex, getSkillRegistryBaseUrl, - installSkill, - linkSkillToAgents, + installSkillWithFanout, listSkillDirsOnDisk, + parseSkillNames, readSkillLock, + runWithConcurrency, writeSkillLock, } from "bailian-cli-core"; import { emitBare, emitResult, formatTable } from "bailian-cli-runtime"; -import { parseSkillNames, runWithConcurrency } from "./shared.ts"; interface UpdateOutcome { name: string; @@ -87,17 +87,8 @@ export default defineCommand({ return { name, status: "failed", reason: "skill not found in registry" }; } try { - await installSkill(name, entry); - const links = linkSkillToAgents(name, agents); - const effective = links.filter((link) => link.mode !== "skipped"); - lock.skills[name] = { - ...(entry.contentHash ? { contentHash: entry.contentHash } : {}), - ...(entry.publishedAt ? { publishedAt: entry.publishedAt } : {}), - installedAt: new Date().toISOString(), - sourceType: "oss", - ...(entry.description ? { description: entry.description } : {}), - links: effective.map((link) => link.path), - }; + const record = await installSkillWithFanout(name, entry, agents); + lock.skills[name] = record.lockEntry; return { name, status: "updated", publishedAt: entry.publishedAt }; } catch (err) { return { diff --git a/packages/commands/tests/e2e/skill.e2e.test.ts b/packages/commands/tests/e2e/skill.e2e.test.ts new file mode 100644 index 0000000..d0af753 --- /dev/null +++ b/packages/commands/tests/e2e/skill.e2e.test.ts @@ -0,0 +1,140 @@ +import { existsSync, mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, test } from "vite-plus/test"; +import { isBailianE2EEnabled, parseStdoutJson, runCommandE2e } from "./helpers.ts"; +import { SKILL_ROUTES } from "./topic-routes.ts"; + +/** Canonical always-published skill; also the backbone of advisor wiki sync */ +const WIKI_SKILL = "bailian-docs-llm-wiki"; + +/** Redirect ~/.bailian into a throwaway dir so lock/skill writes never touch the real user config */ +function makeTempConfigDir(): string { + return mkdtempSync(join(tmpdir(), "bl-skill-e2e-")); +} + +describe("e2e: skill", () => { + test("skill add --help exits successfully", async () => { + const { stderr, exitCode } = await runCommandE2e(SKILL_ROUTES, ["skill", "add", "--help"]); + expect(exitCode, stderr).toBe(0); + expect(stderr).toMatch(/--name/); + }); + + test("skill update --help exits successfully", async () => { + const { stderr, exitCode } = await runCommandE2e(SKILL_ROUTES, ["skill", "update", "--help"]); + expect(exitCode, stderr).toBe(0); + expect(stderr).toMatch(/--name/); + }); + + test("skill remove --help exits successfully", async () => { + const { stderr, exitCode } = await runCommandE2e(SKILL_ROUTES, ["skill", "remove", "--help"]); + expect(exitCode, stderr).toBe(0); + expect(stderr).toMatch(/--name/); + }); + + test("skill list --help exits successfully", async () => { + const { stderr, exitCode } = await runCommandE2e(SKILL_ROUTES, ["skill", "list", "--help"]); + expect(exitCode, stderr).toBe(0); + expect(stderr).toMatch(/list|registry/i); + }); +}); + +// Local-only cases: auth "none" + validation happens before any network access, no gating needed +describe("e2e: skill (local, no credentials)", () => { + test("skill add without --name errors as usage error (2)", async () => { + const { stdout, stderr, exitCode } = await runCommandE2e(SKILL_ROUTES, [ + "skill", + "add", + "--quiet", + ]); + expect(exitCode).toBe(2); + expect(`${stdout}\n${stderr}`).toMatch(/--name|Usage:/i); + }); + + test("skill remove without --name errors as usage error (2)", async () => { + const { stdout, stderr, exitCode } = await runCommandE2e(SKILL_ROUTES, [ + "skill", + "remove", + "--quiet", + ]); + expect(exitCode).toBe(2); + expect(`${stdout}\n${stderr}`).toMatch(/--name|Usage:/i); + }); + + test("skill add rejects mixing all with specific names (2)", async () => { + // parseSkillNames throws UsageError before fetchSkillsIndex — offline-safe + const { stdout, stderr, exitCode } = await runCommandE2e(SKILL_ROUTES, [ + "skill", + "add", + "--name", + "all,spark-video", + "--quiet", + ]); + expect(exitCode).toBe(2); + expect(`${stdout}\n${stderr}`).toMatch(/all/i); + }); + + test("skill remove of a not-installed skill fails with reason (1)", async () => { + const configDir = makeTempConfigDir(); + const { stdout, exitCode } = await runCommandE2e( + SKILL_ROUTES, + ["skill", "remove", "--name", "definitely-not-installed", "--output", "json"], + { BAILIAN_CONFIG_DIR: configDir }, + ); + expect(exitCode).toBe(1); + const data = parseStdoutJson<{ + skills?: Array<{ name?: string; status?: string; reason?: string }>; + }>(stdout); + expect(data.skills?.[0]?.status).toBe("failed"); + expect(data.skills?.[0]?.reason).toMatch(/not installed/i); + }); +}); + +describe.skipIf(!isBailianE2EEnabled())("e2e: skill (real registry)", () => { + test("skill list --output json returns registry and status rows", async () => { + const configDir = makeTempConfigDir(); + const { stdout, stderr, exitCode } = await runCommandE2e( + SKILL_ROUTES, + ["skill", "list", "--output", "json"], + { BAILIAN_CONFIG_DIR: configDir }, + ); + expect(exitCode, stderr).toBe(0); + const data = parseStdoutJson<{ + registry?: string; + skills?: Array<{ name?: string; status?: string }>; + }>(stdout); + expect(data.registry).toMatch(/^https?:\/\//); + expect(Array.isArray(data.skills)).toBe(true); + }, 60_000); + + test("skill add + remove full lifecycle in isolated dirs", async () => { + const configDir = makeTempConfigDir(); + // Empty fake home → no agents detected → fan-out never leaves the sandbox + const fakeHome = makeTempConfigDir(); + const env = { BAILIAN_CONFIG_DIR: configDir, HOME: fakeHome, USERPROFILE: fakeHome }; + + const added = await runCommandE2e( + SKILL_ROUTES, + ["skill", "add", "--name", WIKI_SKILL, "--output", "json"], + env, + ); + expect(added.exitCode, added.stderr).toBe(0); + const addData = parseStdoutJson<{ skills?: Array<{ name?: string; status?: string }> }>( + added.stdout, + ); + expect(addData.skills?.[0]?.status).toBe("installed"); + expect(existsSync(join(configDir, "skills", WIKI_SKILL, "SKILL.md"))).toBe(true); + + const removed = await runCommandE2e( + SKILL_ROUTES, + ["skill", "remove", "--name", WIKI_SKILL, "--output", "json"], + env, + ); + expect(removed.exitCode, removed.stderr).toBe(0); + const removeData = parseStdoutJson<{ skills?: Array<{ name?: string; status?: string }> }>( + removed.stdout, + ); + expect(removeData.skills?.[0]?.status).toBe("removed"); + expect(existsSync(join(configDir, "skills", WIKI_SKILL))).toBe(false); + }, 300_000); +}); diff --git a/packages/commands/tests/e2e/topic-routes.ts b/packages/commands/tests/e2e/topic-routes.ts index faf1850..8771042 100644 --- a/packages/commands/tests/e2e/topic-routes.ts +++ b/packages/commands/tests/e2e/topic-routes.ts @@ -143,3 +143,10 @@ export const TOKEN_PLAN_ROUTES: E2eRouteExports = { "token-plan assign-seats": "tokenPlanAssignSeats", "token-plan add-member": "tokenPlanAddMember", }; + +export const SKILL_ROUTES: E2eRouteExports = { + "skill add": "skillAdd", + "skill update": "skillUpdate", + "skill remove": "skillRemove", + "skill list": "skillList", +}; diff --git a/packages/core/src/advisor/sync.ts b/packages/core/src/advisor/sync.ts index 70ff0be..6e36ce7 100644 --- a/packages/core/src/advisor/sync.ts +++ b/packages/core/src/advisor/sync.ts @@ -23,20 +23,18 @@ import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir } from "../config/paths.ts"; -import { detectInstalledAgents, linkSkillToAgents } from "../skills/agents.ts"; -import { installSkill } from "../skills/installer.ts"; +import { buildSkillLockEntry, installSkillWithFanout } from "../skills/installer.ts"; import { readSkillLock, upsertSkillLockEntry } from "../skills/lock.ts"; -import type { SkillIndexEntry } from "../skills/types.ts"; +import { fetchSkillsIndex } from "../skills/registry.ts"; +import type { SkillIndexEntry, SkillLockEntry } from "../skills/types.ts"; -/** Public-read OSS skill registry root (hardcoded, does not use env). */ -const REGISTRY_BASE_URL = "https://bailian-wiki.oss-cn-hangzhou.aliyuncs.com/skills"; const WIKI_SKILL_NAME = "bailian-docs-llm-wiki"; const SKILL_DIR_NAME = "skills/bailian-docs-llm-wiki"; const STATE_FILE_NAME = "wiki-sync-state.json"; const MODELS_FILE = "models.jsonl"; -const INDEX_KEY = "index.json"; const THROTTLE_MS = 12 * 60 * 60 * 1000; // 12h +/** Tighter than the interactive default: the silent channel must not stall `bl advisor recommend` */ const INDEX_TIMEOUT_MS = 3000; interface SyncState { @@ -45,11 +43,6 @@ interface SyncState { contentHash: string; } -interface SkillsIndex { - updatedAt?: string; - skills: Record; -} - function getCatalogDir(): string { return join(getConfigDir(), SKILL_DIR_NAME); } @@ -89,16 +82,9 @@ function writeState(state: SyncState): void { * Includes fan-out links so bl skill remove can reclaim agent symlinks. * Bookkeeping in the silent channel must be best-effort: failure does not affect sync results. */ -function recordWikiInLock(entry: SkillIndexEntry, links: string[]): void { +function recordWikiInLock(lockEntry: SkillLockEntry): void { try { - upsertSkillLockEntry(WIKI_SKILL_NAME, { - ...(entry.contentHash ? { contentHash: entry.contentHash } : {}), - ...(entry.publishedAt ? { publishedAt: entry.publishedAt } : {}), - installedAt: new Date().toISOString(), - sourceType: "oss", - ...(entry.description ? { description: entry.description } : {}), - links, - }); + upsertSkillLockEntry(WIKI_SKILL_NAME, lockEntry); } catch { /* Bookkeeping failure does not block sync; next sync or bl skill add will fill it in */ } @@ -113,15 +99,10 @@ function wikiLockUpToDate(contentHash: string): boolean { } } -/** Fetch skills/index.json and extract the wiki skill entry; returns null on any failure */ +/** Fetch skills/index.json via the shared registry client and extract the wiki skill entry; returns null on any failure */ async function fetchIndexEntry(): Promise { try { - const res = await fetch(`${REGISTRY_BASE_URL}/${INDEX_KEY}`, { - signal: AbortSignal.timeout(INDEX_TIMEOUT_MS), - }); - if (!res.ok) return null; - const index = (await res.json()) as SkillsIndex; - if (!index?.skills || typeof index.skills !== "object") return null; + const index = await fetchSkillsIndex(INDEX_TIMEOUT_MS); return index.skills[WIKI_SKILL_NAME] ?? null; } catch { return null; @@ -156,20 +137,15 @@ export async function maybeSyncWikiData(): Promise { if (dataOk && (!state || state.contentHash === entry.contentHash)) { writeState({ lastChecked: now, contentHash: entry.contentHash }); // Data and content are ready but lock record is missing/stale (e.g. postinstall landed before this mechanism) → backfill - if (!wikiLockUpToDate(entry.contentHash)) recordWikiInLock(entry, []); + if (!wikiLockUpToDate(entry.contentHash)) recordWikiInLock(buildSkillLockEntry(entry, [])); return false; } // 4. Different content or missing data: delegate to the shared skill install pipeline // (download → extract → SKILL.md validate → atomic swap → fan-out → lock with links) try { - await installSkill(WIKI_SKILL_NAME, entry); - const agents = detectInstalledAgents(); - const linkResults = linkSkillToAgents(WIKI_SKILL_NAME, agents); - const effectiveLinks = linkResults - .filter((link) => link.mode !== "skipped") - .map((link) => link.path); - recordWikiInLock(entry, effectiveLinks); + const record = await installSkillWithFanout(WIKI_SKILL_NAME, entry); + recordWikiInLock(record.lockEntry); } catch { // Install failed → clean exit, leave existing data untouched, do not write state; next recommend retries return false; diff --git a/packages/core/src/skills/index.ts b/packages/core/src/skills/index.ts index fd2ec5c..58529b2 100644 --- a/packages/core/src/skills/index.ts +++ b/packages/core/src/skills/index.ts @@ -22,6 +22,7 @@ export { upsertSkillLockEntry, } from "./lock.ts"; export { sanitizeSkillName, isSafeSkillName } from "./sanitize.ts"; +export { parseSkillNames } from "./names.ts"; export { validateSkillDir, type SkillMeta } from "./validate.ts"; export { extractTarBr, atomicSwap, isSafeEntryName, computeDirContentHash } from "./extract.ts"; export { @@ -35,7 +36,10 @@ export { export { installSkill, installSkillFromBuffer, + installSkillWithFanout, + buildSkillLockEntry, removeSkillDir, type InstalledSkill, + type SkillInstallRecord, } from "./installer.ts"; export { listSkillDirsOnDisk, computeSkillStatuses } from "./status.ts"; diff --git a/packages/core/src/skills/installer.ts b/packages/core/src/skills/installer.ts index 5d9d42c..0bcba40 100644 --- a/packages/core/src/skills/installer.ts +++ b/packages/core/src/skills/installer.ts @@ -2,12 +2,13 @@ import { existsSync, mkdirSync, rmSync } from "node:fs"; import { join } from "node:path"; import { BailianError } from "../errors/base.ts"; import { ExitCode } from "../errors/codes.ts"; +import { detectInstalledAgents, linkSkillToAgents, type AgentTarget } from "./agents.ts"; import { atomicSwap, computeDirContentHash, extractTarBr } from "./extract.ts"; import { getSkillsDir } from "./lock.ts"; import { downloadSkillAsset } from "./registry.ts"; import { isSafeSkillName } from "./sanitize.ts"; import { validateSkillDir, type SkillMeta } from "./validate.ts"; -import type { SkillIndexEntry } from "./types.ts"; +import type { SkillIndexEntry, SkillLockEntry } from "./types.ts"; /** * Skill installer: download → extract to tmpdir (with tar-slip check) → validate SKILL.md → @@ -85,3 +86,48 @@ export function removeSkillDir(name: string): boolean { rmSync(dest, { recursive: true, force: true }); return true; } + +/** + * Build a skill-lock entry from an index entry + effective fan-out link paths. + * Single source of truth for the "installation fact" shape shared by bl skill add/update, + * advisor wiki sync, and any future install channel. + */ +export function buildSkillLockEntry(entry: SkillIndexEntry, links: string[]): SkillLockEntry { + return { + ...(entry.contentHash ? { contentHash: entry.contentHash } : {}), + ...(entry.publishedAt ? { publishedAt: entry.publishedAt } : {}), + installedAt: new Date().toISOString(), + sourceType: "oss", + ...(entry.description ? { description: entry.description } : {}), + links, + }; +} + +export interface SkillInstallRecord { + /** Ready-to-persist lock entry (links = effective fan-out paths) */ + lockEntry: SkillLockEntry; + /** Ids of agents that actually received a link/copy (skipped ones excluded) */ + linkedAgents: string[]; +} + +/** + * Full install workflow for one skill: install into canonical, fan out to agents, and build + * the lock entry recording effective links. Callers decide how to persist the lock entry + * (batch writeSkillLock for commands, best-effort upsertSkillLockEntry for silent channels). + */ +export async function installSkillWithFanout( + name: string, + entry: SkillIndexEntry, + agents: AgentTarget[] = detectInstalledAgents(), +): Promise { + await installSkill(name, entry); + const links = linkSkillToAgents(name, agents); + const effective = links.filter((link) => link.mode !== "skipped"); + return { + lockEntry: buildSkillLockEntry( + entry, + effective.map((link) => link.path), + ), + linkedAgents: effective.map((link) => link.agent), + }; +} diff --git a/packages/commands/src/commands/skill/shared.ts b/packages/core/src/skills/names.ts similarity index 54% rename from packages/commands/src/commands/skill/shared.ts rename to packages/core/src/skills/names.ts index 839fee2..397664c 100644 --- a/packages/commands/src/commands/skill/shared.ts +++ b/packages/core/src/skills/names.ts @@ -1,4 +1,4 @@ -import { UsageError } from "bailian-cli-core"; +import { UsageError } from "../errors/base.ts"; /** * Parse --name: `all` or a comma-separated list of skill names (deduplicated, trimmed). @@ -28,26 +28,3 @@ export function parseSkillNames(raw: string | undefined, defaultAll: boolean): s } return parts; } - -/** - * Run async task factories with a bounded concurrency pool. - * Returns results in the same order as the input tasks array. - */ -export async function runWithConcurrency( - tasks: Array<() => Promise>, - limit: number, -): Promise { - const results: T[] = new Array(tasks.length); - let nextIndex = 0; - - async function worker(): Promise { - while (nextIndex < tasks.length) { - const currentIndex = nextIndex++; - results[currentIndex] = await tasks[currentIndex](); - } - } - - const workers = Array.from({ length: Math.min(limit, tasks.length) }, () => worker()); - await Promise.all(workers); - return results; -} diff --git a/packages/core/src/skills/registry.ts b/packages/core/src/skills/registry.ts index 534403d..9bbf260 100644 --- a/packages/core/src/skills/registry.ts +++ b/packages/core/src/skills/registry.ts @@ -20,12 +20,13 @@ export function getSkillRegistryBaseUrl(): string { /** * Fetch the remote skill index. No local caching — the diff comparison is always * "live remote index vs local skill-lock.json". + * Silent background channels (advisor sync) may pass a tighter timeout than the interactive default. */ -export async function fetchSkillsIndex(): Promise { +export async function fetchSkillsIndex(timeoutMs: number = INDEX_TIMEOUT_MS): Promise { const url = `${getSkillRegistryBaseUrl()}/index.json`; let res: Response; try { - res = await fetch(url, { signal: AbortSignal.timeout(INDEX_TIMEOUT_MS) }); + res = await fetch(url, { signal: AbortSignal.timeout(timeoutMs) }); } catch (err) { throw new BailianError( `Cannot access skill registry: ${url}`, diff --git a/packages/core/src/utils/concurrency.ts b/packages/core/src/utils/concurrency.ts new file mode 100644 index 0000000..40a68ed --- /dev/null +++ b/packages/core/src/utils/concurrency.ts @@ -0,0 +1,22 @@ +/** + * Run async task factories with a bounded concurrency pool. + * Returns results in the same order as the input tasks array. + */ +export async function runWithConcurrency( + tasks: Array<() => Promise>, + limit: number, +): Promise { + const results: T[] = []; + let nextIndex = 0; + + async function worker(): Promise { + while (nextIndex < tasks.length) { + const currentIndex = nextIndex++; + results[currentIndex] = await tasks[currentIndex](); + } + } + + const workers = Array.from({ length: Math.min(limit, tasks.length) }, () => worker()); + await Promise.all(workers); + return results; +} diff --git a/packages/core/src/utils/index.ts b/packages/core/src/utils/index.ts index 4d28595..38943a0 100644 --- a/packages/core/src/utils/index.ts +++ b/packages/core/src/utils/index.ts @@ -3,6 +3,7 @@ export { resolveOutputDir } from "./output-dir.ts"; export { maskToken } from "./token.ts"; export { stripUndefined } from "./object.ts"; export { readTextFromPathOrStdin } from "./fs.ts"; +export { runWithConcurrency } from "./concurrency.ts"; export { parseBooleanValue, parseOptionalBooleanValue,