Compare commits

...

2 Commits

Author SHA1 Message Date
故璃 9e6f5ca9cb fix(skill): retry registry fetches and tolerate blocked backup cleanup
- bl skill init: raise index timeout 10s→30s and retry transient network
  failures (3 attempts); advisor sync silent channel stays fail-fast
- make post-swap backup deletion best-effort so host safe-delete guards
  cannot fail a completed install (postinstall.js mirror included)
- add unit coverage for guard-blocked cleanup and registry retry policy
2026-09-01 16:37:12 +08:00
gujieye 3f78de2dc1 Merge pull request #183 from modelstudioai/feat/confirm-destructive-ops
feat: require --yes confirmation for destructive operations
2026-08-31 19:47:17 +08:00
6 changed files with 226 additions and 92 deletions
+12 -2
View File
@@ -181,7 +181,13 @@ function atomicSwap(tmpDir, catalogDir) {
if (existsSync(backup) && !existsSync(catalogDir)) renameSync(backup, catalogDir);
throw err;
}
if (existsSync(backup)) rmSync(backup, { recursive: true, force: true });
// Best-effort cleanup (symmetric with core skills/extract.ts): the swap already
// succeeded, so a backup deletion failure must not fail the pre-download
try {
if (existsSync(backup)) rmSync(backup, { recursive: true, force: true });
} catch {
/* keep the backup on disk rather than report a completed swap as failed */
}
}
async function main() {
@@ -214,7 +220,11 @@ async function main() {
}
atomicSwap(tmpDir, catalogDir);
} catch (err) {
if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true, force: true });
try {
if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true, force: true });
} catch {
/* cleanup must not mask the original error */
}
throw err;
}
+4 -1
View File
@@ -37,6 +37,8 @@ const MODELS_FILE = "models.jsonl";
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;
/** No retries on the silent channel: a failed sync simply tries again on the next recommend */
const FETCH_ATTEMPTS = 1;
interface SyncState {
lastChecked: number;
@@ -108,7 +110,7 @@ function wikiLockNeedsBackfill(contentHash: string): boolean {
/** Fetch skills/index.json via the shared registry client and extract the wiki skill entry; returns null on any failure */
async function fetchIndexEntry(): Promise<SkillIndexEntry | null> {
try {
const index = await fetchSkillsIndex(INDEX_TIMEOUT_MS);
const index = await fetchSkillsIndex(INDEX_TIMEOUT_MS, FETCH_ATTEMPTS);
return index.skills[WIKI_SKILL_NAME] ?? null;
} catch {
return null;
@@ -160,6 +162,7 @@ export async function maybeSyncWikiData(): Promise<boolean> {
entry,
detectInstalledAgents(),
previousLinks,
FETCH_ATTEMPTS,
);
recordWikiInLock(record.lockEntry);
} catch {
+8 -1
View File
@@ -100,5 +100,12 @@ export function atomicSwap(tmpDir: string, destDir: string): void {
if (existsSync(backup) && !existsSync(destDir)) renameSync(backup, destDir);
throw err;
}
if (existsSync(backup)) rmSync(backup, { recursive: true, force: true });
// Best-effort cleanup: the swap already succeeded, so a backup deletion failure
// (permissions, host safe-delete guards on large dirs) must not fail the install;
// leftover .old-* dirs are inert (skill status scans ignore them)
try {
if (existsSync(backup)) rmSync(backup, { recursive: true, force: true });
} catch {
/* keep the backup on disk rather than report a completed install as failed */
}
}
+17 -4
View File
@@ -61,12 +61,22 @@ export async function installSkillFromBuffer(
atomicSwap(tmpDir, dest);
return { name, path: dest, meta };
} finally {
if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true, force: true });
// Best-effort cleanup: on failure paths this must not mask the original error,
// and on success the dir is already renamed away (existsSync → false)
try {
if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true, force: true });
} catch {
/* leave the temp dir rather than hide the real error */
}
}
}
/** Install a single skill by index entry (download + validate + write to disk) */
export async function installSkill(name: string, entry: SkillIndexEntry): Promise<InstalledSkill> {
export async function installSkill(
name: string,
entry: SkillIndexEntry,
downloadAttempts?: number,
): Promise<InstalledSkill> {
if (entry.compression && entry.compression !== "tar.br") {
throw new BailianError(
`Skill ${name} uses unsupported compression format: ${entry.compression}`,
@@ -74,7 +84,7 @@ export async function installSkill(name: string, entry: SkillIndexEntry): Promis
"Upgrade bailian-cli to the latest version and retry",
);
}
const buffer = await downloadSkillAsset(name, entry);
const buffer = await downloadSkillAsset(name, entry, downloadAttempts);
return installSkillFromBuffer(name, buffer, entry.contentHash);
}
@@ -116,14 +126,17 @@ export interface SkillInstallRecord {
* entry (batch writeSkillLock for commands, best-effort upsertSkillLockEntry for silent channels).
* recordedLinks = the skill's previously recorded fan-out paths from the lock; lets the
* fan-out replace copy-fallback artifacts and keeps unvisited paths reclaimable.
* downloadAttempts = registry fetch attempts (undefined → interactive default; silent
* background channels pass 1 to fail fast instead of stalling the host command).
*/
export async function installSkillWithFanout(
name: string,
entry: SkillIndexEntry,
agents: AgentTarget[] = detectInstalledAgents(),
recordedLinks: string[] = [],
downloadAttempts?: number,
): Promise<SkillInstallRecord> {
await installSkill(name, entry);
await installSkill(name, entry, downloadAttempts);
const fanout = fanOutSkillToAgents(name, agents, recordedLinks);
return {
lockEntry: buildSkillLockEntry(entry, fanout.links),
+95 -74
View File
@@ -1,5 +1,6 @@
import { BailianError } from "../errors/base.ts";
import { ExitCode } from "../errors/codes.ts";
import { withRetry } from "../utils/retry.ts";
import type { SkillIndexEntry, SkillsIndex } from "./types.ts";
/**
@@ -9,8 +10,10 @@ import type { SkillIndexEntry, SkillsIndex } from "./types.ts";
*/
const DEFAULT_REGISTRY_BASE_URL = "https://bailian-wiki.oss-cn-hangzhou.aliyuncs.com/skills";
const INDEX_TIMEOUT_MS = 10_000;
const INDEX_TIMEOUT_MS = 30_000;
const ASSET_TIMEOUT_MS = 120_000;
/** Interactive channels retry transient failures; silent background channels pass 1 to fail fast */
const DEFAULT_ATTEMPTS = 3;
export function getSkillRegistryBaseUrl(): string {
const override = process.env.BAILIAN_SKILL_REGISTRY_URL?.trim();
@@ -20,55 +23,64 @@ 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.
* Silent background channels (advisor sync) may pass a tighter timeout and attempts=1
* than the interactive defaults.
*/
export async function fetchSkillsIndex(timeoutMs: number = INDEX_TIMEOUT_MS): Promise<SkillsIndex> {
const url = `${getSkillRegistryBaseUrl()}/index.json`;
let res: Response;
try {
res = await fetch(url, { signal: AbortSignal.timeout(timeoutMs) });
} catch (err) {
throw new BailianError(
`Cannot access skill registry: ${url}`,
ExitCode.NETWORK,
"Check network connectivity; if using a private mirror, verify BAILIAN_SKILL_REGISTRY_URL configuration",
{ cause: err },
);
}
if (!res.ok) {
throw new BailianError(
`Skill registry returned HTTP ${res.status}: ${url}`,
ExitCode.NETWORK,
res.status === 404
? "Skill index not yet published or registry URL is incorrect; confirm the publisher has generated index.json"
: "Remote error, retry later",
);
}
let parsed: unknown;
try {
parsed = await res.json();
} catch (err) {
throw new BailianError(
"Skill index index.json is not valid JSON",
ExitCode.GENERAL,
"Remote may be in the middle of publishing, retry later",
{ cause: err },
);
}
const index = parsed as SkillsIndex;
if (
typeof index !== "object" ||
index === null ||
typeof index.skills !== "object" ||
index.skills === null
) {
throw new BailianError(
"Skill index index.json has invalid structure",
ExitCode.GENERAL,
"Retry later or contact the publisher",
);
}
return index;
export async function fetchSkillsIndex(
timeoutMs: number = INDEX_TIMEOUT_MS,
attempts: number = DEFAULT_ATTEMPTS,
): Promise<SkillsIndex> {
return withRetry(
async () => {
const url = `${getSkillRegistryBaseUrl()}/index.json`;
let res: Response;
try {
res = await fetch(url, { signal: AbortSignal.timeout(timeoutMs) });
} catch (err) {
throw new BailianError(
`Cannot access skill registry: ${url}`,
ExitCode.NETWORK,
"Check network connectivity; if using a private mirror, verify BAILIAN_SKILL_REGISTRY_URL configuration",
{ cause: err },
);
}
if (!res.ok) {
throw new BailianError(
`Skill registry returned HTTP ${res.status}: ${url}`,
ExitCode.NETWORK,
res.status === 404
? "Skill index not yet published or registry URL is incorrect; confirm the publisher has generated index.json"
: "Remote error, retry later",
);
}
let parsed: unknown;
try {
parsed = await res.json();
} catch (err) {
throw new BailianError(
"Skill index index.json is not valid JSON",
ExitCode.GENERAL,
"Remote may be in the middle of publishing, retry later",
{ cause: err },
);
}
const index = parsed as SkillsIndex;
if (
typeof index !== "object" ||
index === null ||
typeof index.skills !== "object" ||
index.skills === null
) {
throw new BailianError(
"Skill index index.json has invalid structure",
ExitCode.GENERAL,
"Retry later or contact the publisher",
);
}
return index;
},
{ attempts },
);
}
/**
@@ -84,29 +96,38 @@ export function resolveAssetFileName(entry?: SkillIndexEntry): string {
}
/** Download the tar.br archive for a single skill (one skill = one GET) */
export async function downloadSkillAsset(name: string, entry?: SkillIndexEntry): Promise<Buffer> {
const url = `${getSkillRegistryBaseUrl()}/${name}/${resolveAssetFileName(entry)}`;
let res: Response;
try {
res = await fetch(url, { signal: AbortSignal.timeout(ASSET_TIMEOUT_MS) });
} catch (err) {
throw new BailianError(
`Failed to download skill ${name}: ${url}`,
ExitCode.NETWORK,
"Network error, retryable",
{
cause: err,
},
);
}
if (!res.ok) {
throw new BailianError(
`Failed to download skill ${name}: HTTP ${res.status}`,
ExitCode.NETWORK,
res.status === 404
? "index.json and skill object are temporarily inconsistent (publishing in progress), retry later"
: "Remote error, retry later",
);
}
return Buffer.from(await res.arrayBuffer());
export async function downloadSkillAsset(
name: string,
entry?: SkillIndexEntry,
attempts: number = DEFAULT_ATTEMPTS,
): Promise<Buffer> {
return withRetry(
async () => {
const url = `${getSkillRegistryBaseUrl()}/${name}/${resolveAssetFileName(entry)}`;
let res: Response;
try {
res = await fetch(url, { signal: AbortSignal.timeout(ASSET_TIMEOUT_MS) });
} catch (err) {
throw new BailianError(
`Failed to download skill ${name}: ${url}`,
ExitCode.NETWORK,
"Network error, retryable",
{
cause: err,
},
);
}
if (!res.ok) {
throw new BailianError(
`Failed to download skill ${name}: HTTP ${res.status}`,
ExitCode.NETWORK,
res.status === 404
? "index.json and skill object are temporarily inconsistent (publishing in progress), retry later"
: "Remote error, retry later",
);
}
return Buffer.from(await res.arrayBuffer());
},
{ attempts },
);
}
+90 -10
View File
@@ -1,8 +1,8 @@
import { existsSync, lstatSync, mkdtempSync, readFileSync, readdirSync, rmSync } from "fs";
import { createHash } from "crypto";
import { tmpdir } from "os";
import { join } from "path";
import { brotliCompressSync } from "zlib";
import { existsSync, lstatSync, mkdtempSync, readFileSync, readdirSync, rmSync } from "node:fs";
import { createHash } from "node:crypto";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { brotliCompressSync } from "node:zlib";
import tar from "tar-stream";
import { afterEach, expect, test, vi } from "vite-plus/test";
import { BailianError } from "../src/errors/base.ts";
@@ -10,6 +10,18 @@ import type { AgentTarget } from "../src/skills/agents.ts";
import { isSafeEntryName } from "../src/skills/extract.ts";
import { installSkillFromBuffer, installSkillWithFanout } from "../src/skills/installer.ts";
import { getSkillsDir } from "../src/skills/lock.ts";
import { downloadSkillAsset, fetchSkillsIndex } from "../src/skills/registry.ts";
/** rmSync wrapped in a spy so tests can simulate host deletion guards (e.g. safe-delete) */
const fsMocks = vi.hoisted(() => ({
rmSync: vi.fn(),
}));
vi.mock("node:fs", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:fs")>();
fsMocks.rmSync.mockImplementation(actual.rmSync);
return { ...actual, rmSync: fsMocks.rmSync };
});
/** Run in an isolated temp config dir, restore env afterwards. */
async function inTempConfigDir(fn: () => Promise<void>): Promise<void> {
@@ -225,15 +237,83 @@ test("fanout install: downloads, links agents, and builds lock entry with merged
});
});
test("fanout install: download failure surfaces as BailianError and leaves no canonical dir", async () => {
test("fanout install: download failure exhausts retries and leaves no canonical dir", async () => {
await inTempConfigDir(async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () => ({ ok: false, status: 404 })),
);
const fetchMock = vi.fn(async () => ({ ok: false, status: 404 }));
vi.stubGlobal("fetch", fetchMock);
await expect(
installSkillWithFanout("demo", { contentHash: "sha256:whatever" }, []),
).rejects.toThrow(BailianError);
expect(fetchMock).toHaveBeenCalledTimes(3);
expect(existsSync(join(getSkillsDir(), "demo"))).toBe(false);
});
});
// ---- host deletion guards (safe-delete): backup cleanup must not fail a completed install ----
const GUARD_ERROR =
'[safe-delete][SAFE_DELETE_BULK_CONFIRM_REQUIRED] {"count":1583,"threshold":500}';
test("installer: guard blocking backup deletion does not fail the install", async () => {
await inTempConfigDir(async () => {
await installSkillFromBuffer("demo", await buildTarBr({ "SKILL.md": VALID_SKILL_MD }));
fsMocks.rmSync.mockImplementationOnce(() => {
throw new Error(GUARD_ERROR);
});
const v2 = "---\nname: demo\ndescription: demo skill v2\n---\n";
const installed = await installSkillFromBuffer("demo", await buildTarBr({ "SKILL.md": v2 }));
expect(installed.name).toBe("demo");
expect(readFileSync(join(getSkillsDir(), "demo", "SKILL.md"), "utf-8")).toBe(v2);
// The blocked backup stays on disk but is inert (status scans ignore .old-*)
const leftovers = readdirSync(getSkillsDir()).filter((entry) => entry.startsWith("demo.old-"));
expect(leftovers).toHaveLength(1);
});
});
test("installer: temp cleanup failure does not mask the original error", async () => {
await inTempConfigDir(async () => {
fsMocks.rmSync.mockImplementationOnce(() => {
throw new Error(GUARD_ERROR);
});
const buf = await buildTarBr({ "SKILL.md": VALID_SKILL_MD, "../evil.txt": "pwned\n" });
await expect(installSkillFromBuffer("demo", buf)).rejects.toThrow(/unsafe tar entry/);
});
});
// ---- registry retry policy ----
test("registry: index fetch retries transient failures and succeeds", async () => {
const indexPayload = { skills: { demo: { contentHash: "sha256:abc" } } };
const fetchMock = vi.fn(async () => {
if (fetchMock.mock.calls.length < 3) throw new Error("network down");
return { ok: true, status: 200, json: async () => indexPayload };
});
vi.stubGlobal("fetch", fetchMock);
const index = await fetchSkillsIndex(1000);
expect(index.skills.demo.contentHash).toBe("sha256:abc");
expect(fetchMock).toHaveBeenCalledTimes(3);
});
test("registry: attempts=1 keeps the silent channel fail-fast", async () => {
const fetchMock = vi.fn(async () => {
throw new Error("network down");
});
vi.stubGlobal("fetch", fetchMock);
await expect(fetchSkillsIndex(1000, 1)).rejects.toThrow(BailianError);
expect(fetchMock).toHaveBeenCalledTimes(1);
});
test("registry: asset download retries transient HTTP errors", async () => {
const fetchMock = vi.fn(async () => {
if (fetchMock.mock.calls.length === 1) return { ok: false, status: 503 };
return {
ok: true,
status: 200,
arrayBuffer: async () => new Uint8Array([1, 2, 3]).buffer,
};
});
vi.stubGlobal("fetch", fetchMock);
const buffer = await downloadSkillAsset("demo");
expect([...buffer]).toEqual([1, 2, 3]);
expect(fetchMock).toHaveBeenCalledTimes(2);
});