mirror of
https://github.com/openclaw/clawhub.git
synced 2026-08-14 08:52:21 +00:00
fix: harden clawhub cli sync reporting
This commit is contained in:
@@ -0,0 +1,73 @@
|
||||
/* @vitest-environment node */
|
||||
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { getCliBuildLabel } from "./buildInfo.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
async function makeTmpDir(prefix: string) {
|
||||
const dir = await mkdtemp(join(tmpdir(), prefix));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
function git(cwd: string, args: string[]) {
|
||||
return execFileSync("git", args, {
|
||||
cwd,
|
||||
encoding: "utf8",
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
}).trim();
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
while (tempDirs.length > 0) {
|
||||
await rm(tempDirs.pop()!, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
describe("getCliBuildLabel", () => {
|
||||
it("includes the current commit when running inside a linked worktree", async () => {
|
||||
const root = await makeTmpDir("clawhub-build-info-");
|
||||
const repo = join(root, "repo");
|
||||
const worktree = join(root, "linked");
|
||||
await mkdir(repo, { recursive: true });
|
||||
git(repo, ["init"]);
|
||||
git(repo, ["config", "user.name", "Test"]);
|
||||
git(repo, ["config", "user.email", "test@example.com"]);
|
||||
await writeFile(join(repo, "README.md"), "test\n", "utf8");
|
||||
git(repo, ["add", "README.md"]);
|
||||
git(repo, ["commit", "-m", "init"]);
|
||||
git(repo, ["worktree", "add", "-b", "linked", worktree]);
|
||||
|
||||
const expected = git(worktree, ["rev-parse", "HEAD"]).slice(0, 8);
|
||||
const previousCwd = process.cwd();
|
||||
const previousEnv = {
|
||||
CLAWHUB_COMMIT: process.env.CLAWHUB_COMMIT,
|
||||
CLAWDHUB_COMMIT: process.env.CLAWDHUB_COMMIT,
|
||||
VERCEL_GIT_COMMIT_SHA: process.env.VERCEL_GIT_COMMIT_SHA,
|
||||
GITHUB_SHA: process.env.GITHUB_SHA,
|
||||
COMMIT_SHA: process.env.COMMIT_SHA,
|
||||
};
|
||||
try {
|
||||
for (const key of Object.keys(previousEnv)) {
|
||||
delete process.env[key as keyof typeof previousEnv];
|
||||
}
|
||||
process.chdir(worktree);
|
||||
|
||||
expect(getCliBuildLabel()).toContain(`(${expected})`);
|
||||
} finally {
|
||||
process.chdir(previousCwd);
|
||||
for (const [key, value] of Object.entries(previousEnv)) {
|
||||
if (value === undefined) {
|
||||
delete process.env[key as keyof typeof previousEnv];
|
||||
} else {
|
||||
process.env[key as keyof typeof previousEnv] = value;
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,3 +1,4 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { existsSync, readFileSync, statSync } from "node:fs";
|
||||
import { dirname, join, resolve } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
@@ -49,6 +50,9 @@ export function getCliBuildLabel() {
|
||||
}
|
||||
|
||||
function readGitCommitFromCwd() {
|
||||
const revParseCommit = readGitCommitFromRevParse();
|
||||
if (revParseCommit) return revParseCommit;
|
||||
|
||||
try {
|
||||
const gitDir = findGitDir(process.cwd());
|
||||
if (!gitDir) return null;
|
||||
@@ -68,6 +72,19 @@ function readGitCommitFromCwd() {
|
||||
}
|
||||
}
|
||||
|
||||
function readGitCommitFromRevParse() {
|
||||
try {
|
||||
const sha = execFileSync("git", ["rev-parse", "HEAD"], {
|
||||
cwd: process.cwd(),
|
||||
encoding: "utf8",
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
});
|
||||
return shortCommit(sha);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function findGitDir(start: string) {
|
||||
let current = resolve(start);
|
||||
for (;;) {
|
||||
|
||||
@@ -527,6 +527,46 @@ describe("cmdSync", () => {
|
||||
expect(parsed.wouldPublish.map((entry) => entry.slug)).toEqual(["ci-skill"]);
|
||||
});
|
||||
|
||||
it("reports fallback roots used for JSON sync output", async () => {
|
||||
interactive = false;
|
||||
const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true);
|
||||
const { findSkillFolders, getFallbackSkillRoots } = await import("../scanSkills.js");
|
||||
mocked(getFallbackSkillRoots).mockImplementation(() => ["/fallback"]);
|
||||
mocked(findSkillFolders).mockImplementation(async (root: string) => {
|
||||
if (root === "/fallback") {
|
||||
return [
|
||||
{
|
||||
folder: "/fallback/fallback-skill",
|
||||
slug: "fallback-skill",
|
||||
displayName: "Fallback Skill",
|
||||
},
|
||||
];
|
||||
}
|
||||
return [];
|
||||
});
|
||||
mockApiRequest.mockImplementation(async (_registry: string, args: { path: string }) => {
|
||||
if (args.path.startsWith("/api/v1/resolve?")) {
|
||||
throw new Error("Skill not found");
|
||||
}
|
||||
throw new Error(`Unexpected apiRequest: ${args.path}`);
|
||||
});
|
||||
|
||||
let output = "";
|
||||
try {
|
||||
await cmdSync(makeOpts(), { all: true, dryRun: true, json: true }, false);
|
||||
output = String(stdoutWrite.mock.calls.at(-1)?.[0] ?? "").trim();
|
||||
} finally {
|
||||
stdoutWrite.mockRestore();
|
||||
}
|
||||
|
||||
const parsed = JSON.parse(output) as {
|
||||
roots: string[];
|
||||
wouldPublish: Array<{ slug: string }>;
|
||||
};
|
||||
expect(parsed.roots).toEqual(["/fallback"]);
|
||||
expect(parsed.wouldPublish.map((entry) => entry.slug)).toEqual(["fallback-skill"]);
|
||||
});
|
||||
|
||||
it("does not fall back to ambient roots when exact CI scans find no skills", async () => {
|
||||
interactive = false;
|
||||
const { findSkillFolders, getFallbackSkillRoots } = await import("../scanSkills.js");
|
||||
@@ -752,6 +792,55 @@ describe("cmdSync", () => {
|
||||
expect(process.exitCode).toBe(1);
|
||||
});
|
||||
|
||||
it("records publishes that resolve with a non-zero exitCode as per-skill failures", async () => {
|
||||
interactive = false;
|
||||
const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true);
|
||||
mockApiRequest.mockImplementation(async (_registry: string, args: { path: string }) => {
|
||||
if (args.path === "/api/v1/whoami") return { user: { handle: "steipete" } };
|
||||
if (args.path === "/api/cli/telemetry/install") return { ok: true };
|
||||
if (args.path.startsWith("/api/v1/resolve?")) {
|
||||
const u = new URL(`https://x.test${args.path}`);
|
||||
const slug = u.searchParams.get("slug");
|
||||
if (slug === "new-skill") {
|
||||
throw new Error("Skill not found");
|
||||
}
|
||||
if (slug === "synced-skill") {
|
||||
return { match: { version: "1.2.3" }, latestVersion: { version: "1.2.3" } };
|
||||
}
|
||||
if (slug === "update-skill") {
|
||||
return { match: null, latestVersion: { version: "1.0.0" } };
|
||||
}
|
||||
}
|
||||
throw new Error(`Unexpected apiRequest: ${args.path}`);
|
||||
});
|
||||
mockCmdPublish.mockImplementation(async (_opts, _folder, options?: unknown) => {
|
||||
const { slug } = options as { slug: string };
|
||||
if (slug === "new-skill") {
|
||||
process.exitCode = 1;
|
||||
}
|
||||
});
|
||||
|
||||
let output = "";
|
||||
try {
|
||||
await cmdSync(makeOpts(), { root: ["/scan"], all: true, dryRun: false, json: true }, true);
|
||||
output = String(stdoutWrite.mock.calls.at(-1)?.[0] ?? "").trim();
|
||||
} finally {
|
||||
stdoutWrite.mockRestore();
|
||||
}
|
||||
|
||||
const parsed = JSON.parse(output) as {
|
||||
ok: boolean;
|
||||
published: Array<{ slug: string }>;
|
||||
failed: Array<{ slug: string; message: string }>;
|
||||
};
|
||||
expect(parsed.ok).toBe(false);
|
||||
expect(parsed.published.map((entry) => entry.slug)).toEqual(["update-skill"]);
|
||||
expect(parsed.failed).toEqual([
|
||||
{ slug: "new-skill", message: "Publish command exited with code 1" },
|
||||
]);
|
||||
expect(process.exitCode).toBe(1);
|
||||
});
|
||||
|
||||
it("aborts command-level failures before publishing", async () => {
|
||||
interactive = false;
|
||||
const { findSkillFolders } = await import("../scanSkills.js");
|
||||
|
||||
@@ -52,6 +52,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
const spinner = jsonMode ? null : createSpinner("Scanning for local skills");
|
||||
const primaryScan = await scanRootsWithLabels(combinedRoots, clawdbotRoots.labels);
|
||||
let scan = primaryScan;
|
||||
let outputRoots = primaryScan.roots;
|
||||
if (primaryScan.skills.length === 0) {
|
||||
if (!includeClawdbotRoots) {
|
||||
fail("No skills found (checked configured roots)");
|
||||
@@ -60,6 +61,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
const fallbackScan = await scanRootsWithLabels(fallback);
|
||||
spinner?.stop();
|
||||
scan = fallbackScan;
|
||||
outputRoots = fallbackScan.roots;
|
||||
if (fallbackScan.skills.length === 0)
|
||||
fail("No skills found (checked workdir and known Clawdis/Clawd locations)");
|
||||
if (!jsonMode) {
|
||||
@@ -144,7 +146,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
ok: true,
|
||||
dryRun: Boolean(options.dryRun),
|
||||
registry,
|
||||
roots: combinedRoots,
|
||||
roots: outputRoots,
|
||||
owner: normalizeOwner(options.owner),
|
||||
duplicates: deduped.duplicates,
|
||||
alreadySynced: synced.map(formatSyncedJson),
|
||||
@@ -187,7 +189,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
ok: true,
|
||||
dryRun: Boolean(options.dryRun),
|
||||
registry,
|
||||
roots: combinedRoots,
|
||||
roots: outputRoots,
|
||||
owner: normalizeOwner(options.owner),
|
||||
duplicates: deduped.duplicates,
|
||||
alreadySynced: synced.map(formatSyncedJson),
|
||||
@@ -224,7 +226,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
ok: true,
|
||||
dryRun: true,
|
||||
registry,
|
||||
roots: combinedRoots,
|
||||
roots: outputRoots,
|
||||
owner: normalizeOwner(options.owner),
|
||||
duplicates: deduped.duplicates,
|
||||
alreadySynced: synced.map(formatSyncedJson),
|
||||
@@ -257,6 +259,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
: undefined
|
||||
: undefined;
|
||||
try {
|
||||
const previousExitCode = process.exitCode;
|
||||
await cmdPublish(opts, skill.folder, {
|
||||
slug: skill.slug,
|
||||
name: skill.displayName,
|
||||
@@ -274,6 +277,15 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
const publishExitCode = process.exitCode;
|
||||
if (isNonZeroExitCode(publishExitCode) && publishExitCode !== previousExitCode) {
|
||||
process.exitCode = previousExitCode;
|
||||
failedUploads.push({
|
||||
slug: skill.slug,
|
||||
message: `Publish command exited with code ${String(publishExitCode)}`,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
uploaded += 1;
|
||||
published.push({ slug: skill.slug, folder: skill.folder, version: publishVersion });
|
||||
} catch (error) {
|
||||
@@ -288,7 +300,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
ok: false,
|
||||
dryRun: false,
|
||||
registry,
|
||||
roots: combinedRoots,
|
||||
roots: outputRoots,
|
||||
owner: normalizeOwner(options.owner),
|
||||
duplicates: deduped.duplicates,
|
||||
alreadySynced: synced.map(formatSyncedJson),
|
||||
@@ -318,7 +330,7 @@ export async function cmdSync(opts: GlobalOpts, options: SyncOptions, inputAllow
|
||||
ok: true,
|
||||
dryRun: false,
|
||||
registry,
|
||||
roots: combinedRoots,
|
||||
roots: outputRoots,
|
||||
owner: normalizeOwner(options.owner),
|
||||
duplicates: deduped.duplicates,
|
||||
alreadySynced: synced.map(formatSyncedJson),
|
||||
@@ -337,6 +349,12 @@ function normalizeRegistry(value: string) {
|
||||
return value.trim().replace(/\/+$/, "").toLowerCase();
|
||||
}
|
||||
|
||||
function isNonZeroExitCode(value: string | number | null | undefined) {
|
||||
if (typeof value === "number") return value !== 0;
|
||||
if (typeof value === "string") return value.trim() !== "" && value.trim() !== "0";
|
||||
return false;
|
||||
}
|
||||
|
||||
function normalizeOwner(value: string | undefined) {
|
||||
return value?.trim().replace(/^@+/, "") || undefined;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user