feat(skillify): add review-only push preview

harnesses/pi/extension-source/hivemind.ts

@@ -1288,6 +1288,7 @@ const PI_SKILLIFY_COMMANDS: { cmd: string; desc: string }[] = [
   { cmd: "hivemind skillify push <skill-name>",          desc: "upload a local skill to the org table (inverse of pull)" },
   { cmd: "hivemind skillify push --from <project|global>", desc: "which local skills dir to read (default: project)" },
   { cmd: "hivemind skillify push --dry-run",             desc: "preview without writing to the org table" },
+  { cmd: "hivemind skillify push --review",              desc: "show the exact candidate and proposed version without publishing" },
   { cmd: "hivemind skillify unpull",                      desc: "remove every skill previously installed by pull" },
   { cmd: "hivemind skillify unpull --user <email>",       desc: "remove only that author's pulls" },
   { cmd: "hivemind skillify unpull --not-mine",           desc: "remove all pulls except your own" },

src/cli/skillify-spec.ts

@@ -49,6 +49,7 @@ export const SKILLIFY_COMMANDS: SkillifyCommand[] = [
   { cmd: "hivemind skillify push <skill-name>",          desc: "upload a local skill to the org table (inverse of pull)" },
   { cmd: "hivemind skillify push --from <project|global>", desc: "which local skills dir to read (default: project)" },
   { cmd: "hivemind skillify push --dry-run",             desc: "preview without writing to the org table" },
+  { cmd: "hivemind skillify push --review",              desc: "show the exact candidate and proposed version without publishing" },
   { cmd: "hivemind skillify unpull",                     desc: "remove every skill previously installed by pull" },
   { cmd: "hivemind skillify unpull --user <email>",      desc: "remove only that author's pulls" },
   { cmd: "hivemind skillify unpull --not-mine",          desc: "remove all pulls except your own" },
@@ -114,6 +115,7 @@ export const SKILLIFY_SPEC: SkillifySubcommand[] = [
     options: [
       { flag: "--from <project|global>", desc: "which local skills dir to read (default: project)" },
       { flag: "--dry-run",               desc: "preview without writing to the org table" },
+      { flag: "--review",                desc: "show the exact candidate and proposed version without publishing" },
     ],
     note: "the manual counterpart to mining — push a skill Claude wrote locally straight to the org table (append-only: re-pushing an existing skill lands a new version). Authorship/lineage is preserved; you're added as a contributor.",
   },

src/commands/skillify.ts

@@ -12,6 +12,7 @@
  *   hivemind skillify team remove <username>       — remove a username from the team list
  *   hivemind skillify team list                    — list current team members
  *   hivemind skillify pull [skill-name] [opts]     — fetch skills from Deeplake to local FS
+ *   hivemind skillify push <skill-name> --review   — inspect a candidate without publishing
  *   hivemind skillify status                       — show counter + per-project state
  *
  * The team list is consumed by the worker when scope=team: SQL filter
@@ -199,6 +200,42 @@ function takeBooleanFlag(args: string[], flag: string): boolean {
   return true;
 }
 
+function isBidirectionalControl(code: number): boolean {
+  return code === 0x061c ||
+    code === 0x200e ||
+    code === 0x200f ||
+    (code >= 0x202a && code <= 0x202e) ||
+    (code >= 0x2066 && code <= 0x2069);
+}
+
+function assertTerminalSafeReview(name: string, text: string): void {
+  for (let offset = 0; offset < text.length; offset++) {
+    const code = text.charCodeAt(offset);
+    const allowedWhitespace = code === 0x09 || code === 0x0a || code === 0x0d;
+    if (
+      (!allowedWhitespace && code < 0x20) ||
+      (code >= 0x7f && code <= 0x9f) ||
+      isBidirectionalControl(code)
+    ) {
+      const point = `U+${code.toString(16).toUpperCase().padStart(4, "0")}`;
+      throw new Error(`cannot review '${name}': SKILL.md contains terminal control character ${point} at offset ${offset}`);
+    }
+  }
+}
+
+function escapeTerminalMetadata(text: string): string {
+  let escaped = "";
+  for (let offset = 0; offset < text.length; offset++) {
+    const code = text.charCodeAt(offset);
+    if (code < 0x20 || (code >= 0x7f && code <= 0x9f) || isBidirectionalControl(code)) {
+      escaped += `\\u${code.toString(16).toUpperCase().padStart(4, "0")}`;
+    } else {
+      escaped += text[offset];
+    }
+  }
+  return escaped;
+}
+
 async function pullSkills(args: string[]): Promise<void> {
   // Parse flags first so the remaining positional is the optional skill name
   const work = [...args];
@@ -274,6 +311,7 @@ async function pushSkills(args: string[]): Promise<void> {
   // Parse flags first so the remaining positional is the required skill name.
   const work = [...args];
   const fromRaw = takeFlagValue(work, "--from") ?? "project";
+  const review = takeBooleanFlag(work, "--review");
   const dryRun = takeBooleanFlag(work, "--dry-run");
   const skillName = work[0];
 
@@ -283,7 +321,7 @@ async function pushSkills(args: string[]): Promise<void> {
     throw new Error(`Invalid --from '${fromRaw}'. Use 'project' or 'global'.`);
   }
   if (!skillName) {
-    throw new Error("Usage: hivemind skillify push <skill-name> [--from project|global] [--dry-run]");
+    throw new Error("Usage: hivemind skillify push <skill-name> [--from project|global] [--dry-run] [--review]");
   }
 
   const config = loadRoutedConfig();
@@ -294,7 +332,7 @@ async function pushSkills(args: string[]): Promise<void> {
     config.token, config.apiUrl, config.orgId, config.workspaceId, config.skillsTableName,
   );
   const query = (sql: string) => api.query(sql) as Promise<Record<string, unknown>[]>;
-  const scopeCfg = loadScopeConfig();
+  const scopeCfg = loadScopeConfig({ migrateLegacy: !review });
 
   const summary = await runPush({
     query,
@@ -306,7 +344,7 @@ async function pushSkills(args: string[]): Promise<void> {
     pusher: config.userName,
     scope: scopeCfg.scope,
     agent: "cli",
-    dryRun,
+    dryRun: dryRun || review,
   });
 
   const src = fromRaw === "global"
@@ -315,6 +353,20 @@ async function pushSkills(args: string[]): Promise<void> {
   const verDesc = summary.previousVersion === null
     ? `v${summary.version} (new)`
     : `v${summary.previousVersion} → v${summary.version}`;
+
+  if (review) {
+    assertTerminalSafeReview(summary.name, summary.sourceText);
+    console.log(`Review candidate: ${summary.name} (proposed v${summary.version})`);
+    console.log(`File:         ${escapeTerminalMetadata(summary.localPath)}`);
+    console.log(`Author:       ${escapeTerminalMetadata(summary.author)}`);
+    console.log(`Scope:        ${summary.scope}`);
+    console.log("--- BEGIN SKILL.md ---");
+    console.log(summary.sourceText);
+    console.log("--- END SKILL.md ---");
+    console.log("Review mode — nothing published. Rerun without --review to publish.");
+    return;
+  }
+
   const tag = summary.action === "pushed" ? "✓ pushed" : "→ would push";
   console.log(`Source:      ${src}`);
   console.log(`  ${tag.padEnd(15)} ${summary.name.padEnd(40)} ${verDesc.padEnd(18)} (${summary.author}, scope=${summary.scope})`);

src/skillify/push.ts

@@ -66,6 +66,8 @@ export interface PushSummary {
   project: string;
   projectKey: string;
   scope: Scope;
+  /** Exact local SKILL.md snapshot used to derive this push proposal. */
+  sourceText: string;
 }
 
 export interface ParsedLocalSkill {
@@ -78,6 +80,8 @@ export interface ParsedLocalSkill {
   version: number;
   agent?: string;
   createdAt?: string;
+  /** Exact local SKILL.md bytes decoded as UTF-8, before parsing or trimming. */
+  sourceText: string;
 }
 
 /**
@@ -92,7 +96,8 @@ export function readLocalSkill(skillsRoot: string, name: string): ParsedLocalSki
   if (!existsSync(path)) {
     throw new Error(`skill '${name}' not found at ${path}`);
   }
-  const parsed = parseFrontmatter(readFileSync(path, "utf-8"));
+  const sourceText = readFileSync(path, "utf-8");
+  const parsed = parseFrontmatter(sourceText);
   if (!parsed) {
     throw new Error(`skill '${name}' at ${path} has no valid frontmatter — cannot push`);
   }
@@ -107,6 +112,7 @@ export function readLocalSkill(skillsRoot: string, name: string): ParsedLocalSki
     version: typeof fm.version === "number" && fm.version > 0 ? fm.version : 1,
     agent: typeof fm.created_by_agent === "string" ? fm.created_by_agent : undefined,
     createdAt: typeof fm.created_at === "string" ? fm.created_at : undefined,
+    sourceText,
   };
 }
 
@@ -202,5 +208,6 @@ export async function runPush(args: PushArgs): Promise<PushSummary> {
     project,
     projectKey,
     scope: args.scope,
+    sourceText: local.sourceText,
   };
 }

src/skillify/scope-config.ts

@@ -16,7 +16,7 @@
  */
 
 import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 import { migrateLegacyStateDir } from "./legacy-migration.js";
 import { getStateDir } from "./state-dir.js";
 
@@ -43,9 +43,26 @@ function configPath(): string {
 
 const DEFAULT: ScopeConfig = { scope: "me", team: [], install: "project" };
 
-export function loadScopeConfig(): ScopeConfig {
-  migrateLegacyStateDir();
-  const CONFIG_PATH = configPath();
+export interface LoadScopeConfigOptions {
+  /** Set false for read-only commands that must not rename the legacy state directory. */
+  migrateLegacy?: boolean;
+}
+
+export function loadScopeConfig(options: LoadScopeConfigOptions = {}): ScopeConfig {
+  const shouldMigrate = options.migrateLegacy ?? true;
+  if (shouldMigrate) migrateLegacyStateDir();
+
+  const currentStateDir = getStateDir();
+  let CONFIG_PATH = configPath();
+  if (
+    !shouldMigrate &&
+    !existsSync(CONFIG_PATH) &&
+    !existsSync(currentStateDir) &&
+    !process.env.HIVEMIND_STATE_DIR?.trim()
+  ) {
+    const legacyPath = join(dirname(currentStateDir), "skilify", "config.json");
+    if (existsSync(legacyPath)) CONFIG_PATH = legacyPath;
+  }
   if (!existsSync(CONFIG_PATH)) return DEFAULT;
   try {
     const raw = JSON.parse(readFileSync(CONFIG_PATH, "utf-8"));

tests/claude-code/skillify-cli.test.ts

@@ -3,6 +3,8 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync
 import { tmpdir, homedir } from "node:os";
 import { join } from "node:path";
 
+const { apiQueries } = vi.hoisted(() => ({ apiQueries: [] as string[] }));
+
 // Logged paths use the native separator (product builds them with path.join).
 // Compare against join(...) substrings rather than "/"-literal regexes so the
 // assertions hold on Windows too.
@@ -16,7 +18,8 @@ vi.mock("../../src/config.js", () => ({
 }));
 vi.mock("../../src/deeplake-api.js", () => ({
   DeeplakeApi: class {
-    async query(_sql: string) {
+    async query(sql: string) {
+      apiQueries.push(sql);
       return [{
         name: "fake-skill", project: "p", project_key: "pk",
         body: "body", version: 1, source_agent: "claude_code",
@@ -57,6 +60,7 @@ beforeEach(() => {
   else configBackup = null;
   try { rmSync(CONFIG_PATH); } catch { /* nothing */ }
   logged = []; erred = [];
+  apiQueries.length = 0;
   // Default: logged in. Individual tests can `loadConfigMock.mockReturnValueOnce(null)`
   // to exercise the unauthenticated path of unpull (no login needed) vs --not-mine
   // (which still requires myUsername).
@@ -299,11 +303,12 @@ describe("push", () => {
   // The DeeplakeApi mock returns a fake row (version 1) for the version SELECT,
   // so a real push bumps to v2; the INSERT response is ignored.
   let pushDir: string;
-  function writeProjectSkill(name: string): void {
+  function writeProjectSkill(name: string): string {
     const dir = join(process.cwd(), ".claude", "skills", name);
     mkdirSync(dir, { recursive: true });
+    const path = join(dir, "SKILL.md");
     writeFileSync(
-      join(dir, "SKILL.md"),
+      path,
       [
         "---",
         `name: ${name}`,
@@ -317,6 +322,7 @@ describe("push", () => {
         "## Body",
       ].join("\n"),
     );
+    return path;
   }
   beforeEach(() => {
     pushDir = mkdtempSync(join(tmpdir(), "skillify-cli-push-"));
@@ -338,6 +344,68 @@ describe("push", () => {
     expect(out).toContain("Dry run — nothing written to the org skills table.");
   });
 
+  it("--review prints the exact candidate and proposed version without writing", async () => {
+    const path = writeProjectSkill("demo-skill");
+    const candidate = `${readFileSync(path, "utf-8")}  \n\n`;
+    writeFileSync(path, candidate);
+    runSkillifyCommand(["push", "demo-skill", "--review"]);
+    await new Promise(r => setImmediate(r));
+    const out = logged.join("\n");
+
+    expect(out).toContain("Review candidate: demo-skill (proposed v2)");
+    expect(out).toContain("--- BEGIN SKILL.md ---");
+    expect(out).toContain("## Body");
+    expect(out).toContain("--- END SKILL.md ---");
+    expect(logged).toContain(candidate);
+    expect(out).toContain("Review mode — nothing published");
+    expect(apiQueries.some(sql => sql.includes("INSERT INTO"))).toBe(false);
+  });
+
+  it("--review rejects terminal control sequences without publishing", async () => {
+    const path = writeProjectSkill("unsafe-skill");
+    const candidate = `${readFileSync(path, "utf-8")}\n\u001b]8;;https://example.com\u0007spoof\u001b]8;;\u0007`;
+    writeFileSync(path, candidate);
+
+    runSkillifyCommand(["push", "unsafe-skill", "--review"]);
+    await new Promise(r => setImmediate(r));
+
+    expect(erred).toEqual([
+      `push error: cannot review 'unsafe-skill': SKILL.md contains terminal control character U+001B at offset ${candidate.indexOf("\u001b")}`,
+    ]);
+    expect(logged.join("\n")).not.toContain("spoof");
+    expect(apiQueries.some(sql => sql.includes("INSERT INTO"))).toBe(false);
+  });
+
+  it("--review rejects bidirectional formatting controls", async () => {
+    const path = writeProjectSkill("bidi-skill");
+    const candidate = `${readFileSync(path, "utf-8")}\nvisible \u202Ehidden`;
+    writeFileSync(path, candidate);
+
+    runSkillifyCommand(["push", "bidi-skill", "--review"]);
+    await new Promise(r => setImmediate(r));
+
+    expect(erred).toEqual([
+      `push error: cannot review 'bidi-skill': SKILL.md contains terminal control character U+202E at offset ${candidate.indexOf("\u202E")}`,
+    ]);
+    expect(logged.join("\n")).not.toContain("hidden");
+    expect(apiQueries.some(sql => sql.includes("INSERT INTO"))).toBe(false);
+  });
+
+  it("--review escapes formatting controls in the displayed source path", async () => {
+    const unsafeDir = join(pushDir, "path\u202Espoof");
+    mkdirSync(unsafeDir, { recursive: true });
+    process.chdir(unsafeDir);
+    writeProjectSkill("path-skill");
+
+    runSkillifyCommand(["push", "path-skill", "--review"]);
+    await new Promise(r => setImmediate(r));
+    const out = logged.join("\n");
+
+    expect(out).not.toContain("\u202E");
+    expect(out).toContain("\\u202E");
+    expect(apiQueries.some(sql => sql.includes("INSERT INTO"))).toBe(false);
+  });
+
   it("real push reports the published version (remote v1 → v2)", async () => {
     writeProjectSkill("demo-skill");
     runSkillifyCommand(["push", "demo-skill"]);
@@ -364,7 +432,7 @@ describe("push", () => {
   it("missing skill name is rejected with the exact usage line", async () => {
     runSkillifyCommand(["push"]);
     await new Promise(r => setImmediate(r));
-    expect(erred.join("\n")).toContain("Usage: hivemind skillify push <skill-name> [--from project|global] [--dry-run]");
+    expect(erred.join("\n")).toContain("Usage: hivemind skillify push <skill-name> [--from project|global] [--dry-run] [--review]");
   });
 
   it("requires login with the exact message", async () => {