mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(skills): log ClawHub skill verification errors instead of swallowing (#107143)
* fix(skills): log ClawHub skill verification errors instead of swallowing (#P2)
catch {} in fetchInstallVerificationLock silently swallowed all network
errors (timeout, DNS, 5xx, JSON parse) from the registry verification
request. Callers could not distinguish verified from failed verification.
Pass logger through and log a warning on failure so transient errors
are visible in the operator log.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(skills): surface ClawHub verification failures
Co-authored-by: SunnyShu0925 <shu.zongyu@xydigit.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
co-authored by
Claude
Peter Steinberger
parent
82d2a03d0a
commit
b857fec2ee
@@ -279,6 +279,7 @@ describe("skills gateway handlers (clawhub)", () => {
|
||||
slug: "calendar",
|
||||
version: "1.2.3",
|
||||
force: false,
|
||||
logger: expect.objectContaining({ warn: expect.any(Function) }),
|
||||
config: {},
|
||||
});
|
||||
expect(ok).toBe(true);
|
||||
@@ -376,6 +377,7 @@ describe("skills gateway handlers (clawhub)", () => {
|
||||
version: "1.2.3",
|
||||
force: false,
|
||||
acknowledgeClawHubRisk: true,
|
||||
logger: expect.objectContaining({ warn: expect.any(Function) }),
|
||||
config: {},
|
||||
});
|
||||
expect(ok).toBe(true);
|
||||
@@ -407,6 +409,7 @@ describe("skills gateway handlers (clawhub)", () => {
|
||||
slug: "calendar",
|
||||
version: "1.2.3",
|
||||
force: false,
|
||||
logger: expect.objectContaining({ warn: expect.any(Function) }),
|
||||
config: {},
|
||||
});
|
||||
expect(ok).toBe(true);
|
||||
@@ -464,6 +467,7 @@ describe("skills gateway handlers (clawhub)", () => {
|
||||
expect(updateSkillsFromClawHubMock).toHaveBeenCalledWith({
|
||||
workspaceDir: "/tmp/workspace",
|
||||
slug: "calendar",
|
||||
logger: expect.objectContaining({ warn: expect.any(Function) }),
|
||||
config: {},
|
||||
});
|
||||
expect(ok).toBe(true);
|
||||
@@ -512,6 +516,7 @@ describe("skills gateway handlers (clawhub)", () => {
|
||||
workspaceDir: "/tmp/workspace",
|
||||
slug: "calendar",
|
||||
acknowledgeClawHubRisk: true,
|
||||
logger: expect.objectContaining({ warn: expect.any(Function) }),
|
||||
config: {},
|
||||
});
|
||||
expect(ok).toBe(true);
|
||||
|
||||
@@ -14,7 +14,12 @@ type CapturedGatewayResponse = {
|
||||
function makeGatewayHandlerTestContext(): GatewayRequestContext {
|
||||
return {
|
||||
getRuntimeConfig: () => ({}),
|
||||
logGateway: vi.fn(),
|
||||
logGateway: {
|
||||
debug: vi.fn(),
|
||||
error: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
},
|
||||
} as unknown as GatewayRequestContext;
|
||||
}
|
||||
|
||||
|
||||
@@ -583,6 +583,7 @@ export const skillsHandlers: GatewayRequestHandlers = {
|
||||
version: p.version,
|
||||
force: Boolean(p.force),
|
||||
...(p.acknowledgeClawHubRisk ? { acknowledgeClawHubRisk: true } : {}),
|
||||
logger: context.logGateway,
|
||||
config: cfg,
|
||||
});
|
||||
const errorDetails = result.ok ? undefined : buildClawHubTrustErrorDetails(result);
|
||||
@@ -705,6 +706,7 @@ export const skillsHandlers: GatewayRequestHandlers = {
|
||||
workspaceDir: resolved.workspaceDir,
|
||||
slug: p.slug,
|
||||
...(p.acknowledgeClawHubRisk ? { acknowledgeClawHubRisk: true } : {}),
|
||||
logger: context.logGateway,
|
||||
config: resolved.cfg,
|
||||
});
|
||||
const errors = results.filter((result) => !result.ok);
|
||||
|
||||
@@ -1045,6 +1045,7 @@ describe("skills-clawhub", () => {
|
||||
|
||||
it("persists install artifact and verification provenance in the ClawHub lockfile", async () => {
|
||||
const workspaceDir = await tempDirs.make("openclaw-skills-lock-");
|
||||
const warn = vi.fn();
|
||||
const skillContent = "---\nname: agentreceipt\ndescription: Receipt helper\n---\n";
|
||||
const skillSha256 = createHash("sha256").update(skillContent).digest("hex");
|
||||
installPackageDirMock.mockImplementationOnce(async (params: { targetDir: string }) => {
|
||||
@@ -1057,6 +1058,7 @@ describe("skills-clawhub", () => {
|
||||
const result = await installSkillFromClawHub({
|
||||
workspaceDir,
|
||||
slug: "agentreceipt",
|
||||
logger: { warn },
|
||||
});
|
||||
|
||||
expectInstalledSkill(result, {
|
||||
@@ -1069,6 +1071,7 @@ describe("skills-clawhub", () => {
|
||||
version: "1.0.0",
|
||||
baseUrl: undefined,
|
||||
});
|
||||
expect(warn).not.toHaveBeenCalled();
|
||||
const lock = JSON.parse(
|
||||
await fs.readFile(path.join(workspaceDir, ".clawhub", "lock.json"), "utf8"),
|
||||
) as { skills: Record<string, Record<string, unknown>> };
|
||||
@@ -1335,6 +1338,7 @@ describe("skills-clawhub", () => {
|
||||
|
||||
it("keeps installing when the ClawHub verification snapshot is unavailable", async () => {
|
||||
const workspaceDir = await tempDirs.make("openclaw-skills-lock-");
|
||||
const warn = vi.fn();
|
||||
fetchClawHubSkillVerificationMock.mockRejectedValueOnce(new Error("verification down"));
|
||||
installPackageDirMock.mockImplementationOnce(async (params: { targetDir: string }) => {
|
||||
await fs.mkdir(params.targetDir, { recursive: true });
|
||||
@@ -1346,9 +1350,14 @@ describe("skills-clawhub", () => {
|
||||
const result = await installSkillFromClawHub({
|
||||
workspaceDir,
|
||||
slug: "agentreceipt",
|
||||
logger: { warn },
|
||||
});
|
||||
|
||||
expectInstalledSkill(result, { slug: "agentreceipt", version: "1.0.0" });
|
||||
expect(warn).toHaveBeenCalledOnce();
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
"Skill verification for agentreceipt failed: verification down",
|
||||
);
|
||||
const lock = JSON.parse(
|
||||
await fs.readFile(path.join(workspaceDir, ".clawhub", "lock.json"), "utf8"),
|
||||
) as { skills: Record<string, Record<string, unknown>> };
|
||||
|
||||
@@ -487,6 +487,7 @@ async function fetchInstallVerificationLock(params: {
|
||||
ownerHandle?: string;
|
||||
version?: string;
|
||||
baseUrl?: string;
|
||||
logger?: Logger;
|
||||
}): Promise<ClawHubSkillVerificationLock | undefined> {
|
||||
try {
|
||||
const verification = await fetchClawHubSkillVerification({
|
||||
@@ -496,7 +497,10 @@ async function fetchInstallVerificationLock(params: {
|
||||
baseUrl: params.baseUrl,
|
||||
});
|
||||
return snapshotClawHubSkillVerification(verification);
|
||||
} catch {
|
||||
} catch (err) {
|
||||
params.logger?.warn?.(
|
||||
`Skill verification for ${formatClawHubSkillRef(params)} failed: ${formatErrorMessage(err)}`,
|
||||
);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
@@ -1472,6 +1476,7 @@ async function performClawHubSkillInstall(
|
||||
...(params.ownerHandle ? { ownerHandle: params.ownerHandle } : {}),
|
||||
version: verificationVersion,
|
||||
baseUrl: params.baseUrl,
|
||||
logger: params.logger,
|
||||
}),
|
||||
]);
|
||||
const sourceUrl =
|
||||
|
||||
Reference in New Issue
Block a user