fix: keep skill sync inside the package and correct install docs

README.md

@@ -16,7 +16,7 @@ I got tired of typing the same follow-ups, so I wrote them down once and gave th
 npx astack-skills@latest install --agent all
 ```
 
-That puts the skills in your project for Claude Code and Codex. `--scope global` installs them everywhere, and `--skill ship,deslop` picks a few. Folders you've edited are left alone; `--force` replaces them.
+That puts the skills in your project for Claude Code and Codex. `--scope global` installs them for every project, in your home directory, and `--skill ship,deslop` picks a few. An unchanged folder is skipped. If a selected folder differs, nothing is written until you pass `--force`, which replaces that folder.
 
 As a plugin:
 

docs/installation.md

@@ -6,11 +6,11 @@
 npx astack-skills@latest install --agent all              # this project
 npx astack-skills@latest install --agent all --scope global
 npx astack-skills@latest install --agent claude --skill ship,deslop
-npx astack-skills@latest install --agent all --dry-run    # show what would change
+npx astack-skills@latest install --agent all --dry-run    # print the plan without writing
 npx astack-skills@latest list
 ```
 
-Project installs go to `.claude/skills/` (Claude Code) and `.agents/skills/` (Codex). `--scope global` uses the same folders in your home directory. A folder you've edited is left alone unless you pass `--force`; symlinked destinations are always refused. The installer has no network calls, background processes, telemetry or install hooks, and running it with no arguments only prints help.
+Project installs go to `.claude/skills/` (Claude Code) and `.agents/skills/` (Codex). `--scope global` uses the same folders in your home directory. An unchanged skill is skipped. If a selected folder differs, the command stops and writes nothing, including `--dry-run`, until you pass `--force` to replace that folder. Symlinked destinations are always refused. The installer has no network calls, background processes, telemetry or install hooks, and running it with no arguments only prints help. `list` prints the catalogue and accepts only `--json`. `--version` prints the package version.
 
 To update, run the same command again with `@latest`. To remove, delete the skill folders it copied.
 

scripts/metadata.ts

@@ -1,6 +1,6 @@
-import { existsSync, lstatSync, mkdirSync, readFileSync, realpathSync, writeFileSync } from "node:fs";
-import { dirname, resolve } from "node:path";
-import { type Entry, inside, readJson } from "./package";
+import { lstatSync, mkdirSync, readFileSync, realpathSync, writeFileSync } from "node:fs";
+import { dirname, isAbsolute, relative, resolve, sep } from "node:path";
+import { type Entry, independentFile, inside, readJson } from "./package";
 
 const root = resolve(import.meta.dir, "..");
 const { skills } = readJson<{ skills: Entry[] }>(resolve(root, "catalog.json"));
@@ -21,25 +21,46 @@ for (const skill of skills) {
     if (info && (info.isSymbolicLink() || !info.isDirectory() || !inside(realpathSync(root), realpathSync(component))))
       throw new Error("Metadata directories must be real directories inside the package");
   }
-  if (!existsSync(resolve(skillDir, "SKILL.md"))) throw new Error(`Missing skill source: ${skill.name}`);
+  if (!independentFile(resolve(skillDir, "SKILL.md"))) throw new Error(`Skill source must be a regular file: ${skill.name}; no metadata written`);
   const path = resolve(dir, "openai.yaml");
   const info = lstatSync(path, { throwIfNoEntry: false });
-  if (info && (info.isSymbolicLink() || !info.isFile())) throw new Error("Metadata must be a regular file");
-  const existing = existsSync(path) ? Bun.YAML.parse(readFileSync(path, "utf8")) as any : {};
+  if (info && !independentFile(path)) throw new Error("Metadata must be a regular independent file; no metadata written");
+  const existing = info ? Bun.YAML.parse(readFileSync(path, "utf8")) as any : {};
   if (!existing || typeof existing !== "object" || Array.isArray(existing) ||
       (existing.interface !== undefined && (!existing.interface || typeof existing.interface !== "object" || Array.isArray(existing.interface))))
     throw new Error(`Invalid existing metadata for ${skill.name}; no metadata written`);
   existing.interface = { ...existing.interface, display_name: skill.title, short_description: skill.summary };
-  pending.push({ dir, path, content: Bun.YAML.stringify(existing, null, 2).trimEnd() + "\n" });
+  // The emitter puts a trailing space on nested keys. The committed files do not, and a second run must match the first.
+  const yaml = Bun.YAML.stringify(existing, null, 2).replace(/[ \t]+$/gm, "").trimEnd() + "\n";
+  pending.push({ dir, path, content: yaml });
 }
-const shared = readJson<{ files: Record<string, string[]> }>(resolve(root, "shared/manifest.json")).files;
+const sharedRoot = resolve(root, "shared");
+const shared = readJson<{ files: Record<string, string[]> }>(resolve(sharedRoot, "manifest.json")).files;
 for (const [file, targets] of Object.entries(shared)) {
-  const content = readFileSync(resolve(root, "shared", file));
+  if (typeof file !== "string" || isAbsolute(file) || file.split(/[\\/]/).includes("..") || !inside(sharedRoot, resolve(sharedRoot, file)))
+    throw new Error(`Shared file escapes the package: ${file}; no metadata written`);
+  const source = resolve(sharedRoot, file);
+  if (!independentFile(source) || !inside(realpathSync(sharedRoot), realpathSync(source)))
+    throw new Error(`Shared source must be a regular independent file: ${file}; no metadata written`);
+  if (!Array.isArray(targets)) throw new Error(`Shared file ${file} targets must be a list; no metadata written`);
+  const content = readFileSync(source).toString("utf8");
   for (const skill of targets) {
-    if (!names.has(skill)) throw new Error(`Shared file ${file} targets unknown skill ${skill}`);
-    const path = resolve(root, "skills", skill, file);
-    if (!inside(resolve(root, "skills", skill), path)) throw new Error("Shared file escapes its skill");
-    pending.push({ dir: dirname(path), path, content: content.toString("utf8") });
+    if (typeof skill !== "string" || !names.has(skill)) throw new Error(`Shared file ${file} targets unknown skill ${String(skill)}; no metadata written`);
+    const skillDir = resolve(root, "skills", skill);
+    const path = resolve(skillDir, file);
+    if (!inside(skillDir, path)) throw new Error("Shared file escapes its skill; no metadata written");
+    const segments = relative(skillDir, path).split(sep).filter(Boolean);
+    let cursor = skillDir;
+    for (let index = 0; index < segments.length; index++) {
+      cursor = resolve(cursor, segments[index]!);
+      const destination = lstatSync(cursor, { throwIfNoEntry: false });
+      if (!destination) continue;
+      if (destination.isSymbolicLink()) throw new Error(`Refusing symlinked shared path: ${file}; no metadata written`);
+      const last = index === segments.length - 1;
+      if (last ? !destination.isFile() || destination.nlink !== 1 : !destination.isDirectory())
+        throw new Error(`Shared destination must be a regular independent file: ${file}; no metadata written`);
+    }
+    pending.push({ dir: dirname(path), path, content });
   }
 }
 for (const file of pending) {

scripts/package.ts

@@ -16,6 +16,12 @@ export function inside(parent: string, child: string): boolean {
   return rel === "" || (!isAbsolute(rel) && rel !== ".." && !rel.startsWith(`..${sep}`));
 }
 
+// A second directory entry for the same inode would let a later write land outside the package.
+export function independentFile(path: string): boolean {
+  const info = lstatSync(path, { throwIfNoEntry: false });
+  return !!info && info.isFile() && !info.isSymbolicLink() && info.nlink === 1;
+}
+
 export function readJson<T = any>(path: string): T {
   return JSON.parse(readFileSync(path, "utf8"));
 }
@@ -102,15 +108,20 @@ export function validatePackage(root: string): Finding[] {
         if (metadata?.name !== entry.name) fail(path, "Frontmatter name differs from directory");
         if (typeof metadata?.description !== "string" || !metadata.description.trim() ||
             metadata.description.length > 1024) fail(path, "Invalid skill description");
+        if (entry.name === "oneshot" && metadata?.["disable-model-invocation"] !== true)
+          fail(path, "oneshot must set disable-model-invocation: true");
       } catch (error) { fail(path, `Invalid YAML: ${String(error)}`); }
     }
     if (text.split(/\r?\n/).length > 500) fail(path, "Disclose conditional detail outside the entrypoint");
     if (/\b(?:REPLACE_ME|INSERT_HERE|your-skill-name)\b/.test(text)) fail(path, "Unfinished scaffold");
     const uiPath = resolve(dir, "agents/openai.yaml");
-    try {
+    if (!independentFile(uiPath)) fail(uiPath, "Picker metadata must be a regular independent file");
+    else try {
       const ui = Bun.YAML.parse(readFileSync(uiPath, "utf8")) as any;
       if (ui?.interface?.display_name !== entry.title || ui?.interface?.short_description !== entry.summary)
         fail(uiPath, "Picker metadata differs from catalogue; run bun run metadata");
+      if (entry.name === "oneshot" && ui?.policy?.allow_implicit_invocation !== false)
+        fail(uiPath, "oneshot must set allow_implicit_invocation: false");
     } catch (error) { fail(uiPath, `Invalid or missing picker metadata: ${String(error)}`); }
 
     for (const file of filesUnder(dir)) {
@@ -134,16 +145,29 @@ export function validatePackage(root: string): Finding[] {
   }
 
   const sharedPath = resolve(root, "shared/manifest.json");
+  const sharedRoot = resolve(root, "shared");
   const shared = json(sharedPath);
   for (const [file, targets] of Object.entries<string[]>(shared?.files ?? {})) {
-    const canonical = resolve(root, "shared", file);
-    if (!inside(resolve(root, "shared"), canonical) || !existsSync(canonical)) { fail(sharedPath, `Missing shared source: ${file}`); continue; }
+    if (isAbsolute(file) || file.split(/[\\/]/).includes("..") || !inside(sharedRoot, resolve(sharedRoot, file))) {
+      fail(sharedPath, `Shared file escapes the package: ${file}`); continue;
+    }
+    const canonical = resolve(sharedRoot, file);
+    if (!lstatSync(canonical, { throwIfNoEntry: false })) { fail(sharedPath, `Missing shared source: ${file}`); continue; }
+    if (!independentFile(canonical) || !inside(realpathSync(sharedRoot), realpathSync(canonical))) {
+      fail(sharedPath, `Shared source must be a regular independent file: ${file}`); continue;
+    }
+    if (!Array.isArray(targets)) { fail(sharedPath, `Shared file targets must be a list: ${file}`); continue; }
     const expected = readFileSync(canonical);
     for (const skill of targets) {
-      if (!names.includes(skill)) { fail(sharedPath, `Shared file targets unknown skill: ${skill}`); continue; }
+      if (typeof skill !== "string" || !names.includes(skill)) { fail(sharedPath, `Shared file targets unknown skill: ${skill}`); continue; }
       const copy = resolve(skillRoot, skill, file);
-      if (!inside(resolve(skillRoot, skill), copy) || !existsSync(copy) || !readFileSync(copy).equals(expected))
-        fail(copy, `Out of sync with shared/${file}; run bun run metadata`);
+      const skillDir = resolve(skillRoot, skill);
+      if (!inside(skillDir, copy)) { fail(sharedPath, `Shared file escapes its skill: ${file}`); continue; }
+      if (!lstatSync(copy, { throwIfNoEntry: false })) { fail(copy, `Out of sync with shared/${file}; run bun run metadata`); continue; }
+      if (!independentFile(copy) || !inside(realpathSync(skillDir), realpathSync(copy))) {
+        fail(copy, `Shared copy must be a regular independent file: ${file}`); continue;
+      }
+      if (!readFileSync(copy).equals(expected)) fail(copy, `Out of sync with shared/${file}; run bun run metadata`);
     }
   }
 

src/cli.ts

@@ -27,7 +27,7 @@ class CliError extends Error {
   constructor(message: string, readonly code = 1) { super(message); }
 }
 const skillName = /^[a-z0-9]+(?:-[a-z0-9]+)*$/;
-const help = `astack: install the standalone skill package\n\nUsage:\n  astack list [--json]\n  astack install --agent claude|codex|all [options]\n\nOptions:\n  --scope project|global   Destination scope (default: project)\n  --skill NAME[,NAME]     Install selected skills; repeatable (default: all)\n  --dry-run               Show the plan without writing files\n  --force                 Replace existing regular skill folders\n  --json                  Emit a machine-readable result\n  --help                  Show this help\n  --version               Show the package version\n\nProject destinations are .claude/skills and .agents/skills. Global scope uses\nthose paths under the user's home. Symlinked destinations are always refused.\nNo-argument invocation shows help; installation never runs automatically.\n`;
+const help = `astack: install the standalone skill package\n\nUsage:\n  astack list [--json]\n  astack install --agent claude|codex|all [options]\n\nOptions:\n  --scope project|global   Destination scope (default: project)\n  --skill NAME[,NAME]     Install selected skills; repeatable (default: all)\n  --dry-run               Show the plan without writing files\n  --force                 Replace existing regular skill folders\n  --json                  Emit a machine-readable result\n  --help                  Show this help\n  --version               Show the package version\n\nProject destinations are .claude/skills and .agents/skills. Global scope uses\nthose paths under the user's home. An unchanged skill is skipped. A differing\nfolder stops the install, including --dry-run, until --force is set. Symlinked\ndestinations are always refused. No-argument invocation shows help; installation\nnever runs automatically.\n`;
 
 function within(base: string, target: string): boolean {
   const rel = relative(base, target);

tests/install.test.ts

@@ -295,6 +295,97 @@ test("malformed package metadata fails before any installation side effects", ()
     expect(readdirSync(f.home)).toEqual([]);
   }
 });
+test("a regular file blocking the destination is preserved", () => {
+  const f = fixture();
+  writeFileSync(resolve(f.project, ".claude"), "not a directory\n");
+  const blocked = runCLI(["install", "--agent", "claude", "--skill", "ship", "--force"], f.runtime);
+  expect(blocked.code).toBe(1);
+  expect(blocked.stderr).toContain("not a directory");
+  expect(readFileSync(resolve(f.project, ".claude"), "utf8")).toBe("not a directory\n");
+  const target = resolve(f.project, ".agents/skills/ship");
+  mkdirSync(resolve(f.project, ".agents/skills"), { recursive: true });
+  writeFileSync(target, "user file\n");
+  const file = runCLI(["install", "--agent", "codex", "--skill", "ship", "--force"], f.runtime);
+  expect(file.code).toBe(1);
+  expect(readFileSync(target, "utf8")).toBe("user file\n");
+  expect(existsSync(resolve(f.project, ".agents/skills/astack"))).toBe(false);
+});
+test("a symlinked package skill tree is refused before any copy", () => {
+  const f = fixture();
+  const outside = resolve(f.root, "outside-skills");
+  mkdirSync(resolve(outside, "ship"), { recursive: true });
+  writeFileSync(resolve(outside, "ship/SKILL.md"), "pwn\n");
+  rmSync(resolve(f.pkg, "skills"), { recursive: true });
+  symlinkSync(outside, resolve(f.pkg, "skills"));
+  const result = runCLI(["install", "--agent", "all", "--force"], f.runtime);
+  expect(result.code).toBe(1);
+  expect(result.stderr).toContain("escapes its installation");
+  expect(readdirSync(f.project)).toEqual([]);
+  expect(readdirSync(f.home)).toEqual([]);
+  expect(readFileSync(resolve(outside, "ship/SKILL.md"), "utf8")).toBe("pwn\n");
+});
+test("a symlinked skill directory is refused before any copy", () => {
+  const f = fixture();
+  const outside = resolve(f.root, "outside-ship");
+  mkdirSync(outside);
+  writeFileSync(resolve(outside, "SKILL.md"), "pwn\n");
+  rmSync(resolve(f.pkg, "skills/ship"), { recursive: true });
+  symlinkSync(outside, resolve(f.pkg, "skills/ship"));
+  const result = runCLI(["install", "--agent", "claude", "--skill", "ship"], f.runtime);
+  expect(result.code).toBe(1);
+  expect(result.stderr).toContain("escapes its installation");
+  expect(readdirSync(f.project)).toEqual([]);
+  expect(readFileSync(resolve(outside, "SKILL.md"), "utf8")).toBe("pwn\n");
+});
+test("a symlinked skills directory is not a writable destination", () => {
+  const f = fixture();
+  const outside = resolve(f.root, "outside-dest");
+  mkdirSync(outside);
+  mkdirSync(resolve(f.project, ".claude"));
+  symlinkSync(outside, resolve(f.project, ".claude/skills"));
+  const result = runCLI(["install", "--agent", "claude", "--force"], f.runtime);
+  expect(result.code).toBe(1);
+  expect(result.stderr).toContain("Refusing symlinked destination");
+  expect(readdirSync(outside)).toEqual([]);
+});
+test("dry-run stops on a differing folder and previews a replacement only with --force", () => {
+  const f = fixture();
+  const target = resolve(f.project, ".claude/skills/ship");
+  mkdirSync(target, { recursive: true });
+  writeFileSync(resolve(target, "SKILL.md"), "edited\n");
+  const blocked = runCLI(["install", "--agent", "claude", "--dry-run", "--json"], f.runtime);
+  expect(blocked.code).toBe(1);
+  expect(blocked.stderr).toContain("Nothing changed");
+  expect(existsSync(resolve(f.project, ".claude/skills/astack"))).toBe(false);
+  const preview = runCLI(["install", "--agent", "claude", "--dry-run", "--force", "--json"], f.runtime);
+  expect(preview.code).toBe(0);
+  const actions = JSON.parse(preview.stdout).actions;
+  expect(actions.find((item: { skill: string }) => item.skill === "ship").action).toBe("replace");
+  expect(actions.find((item: { skill: string }) => item.skill === "astack").action).toBe("install");
+  expect(readFileSync(resolve(target, "SKILL.md"), "utf8")).toBe("edited\n");
+  expect(existsSync(resolve(f.project, ".claude/skills/astack"))).toBe(false);
+});
+test("list and version stay read-only", () => {
+  const f = fixture();
+  expect(runCLI(["list", "--skill", "ship"], f.runtime).code).toBe(2);
+  expect(runCLI(["list", "--scope", "global"], f.runtime).stderr).toContain("accepts only --json");
+  expect(runCLI(["--version"], f.runtime)).toEqual({ code: 0, stdout: "0.2.0\n", stderr: "" });
+  expect(runCLI(["install", "--agent", "claude", "--skill", "ship,"], f.runtime).code).toBe(2);
+  expect(runCLI(["explode"], f.runtime).code).toBe(2);
+  expect(readdirSync(f.project)).toEqual([]);
+  expect(readdirSync(f.home)).toEqual([]);
+});
+test("a script that lost its executable bit is a conflict, not a skip", () => {
+  const f = fixture();
+  const args = ["install", "--agent", "claude", "--skill", "ship", "--json"];
+  expect(runCLI(args, f.runtime).code).toBe(0);
+  const script = resolve(f.project, ".claude/skills/ship/scripts/example.sh");
+  chmodSync(script, 0o644);
+  const again = runCLI(args, f.runtime);
+  expect(again.code).toBe(1);
+  expect(again.stderr).toContain("Existing skill differs");
+  expect(statSync(script).mode & 0o777).toBe(0o644);
+});
 test("repeated and equals-style selectors are normalized", () => {
   const result = parseArguments(["install", "--agent=claude", "--agent", "all", "--skill=ship", "--skill", "ship,astack"]);
   expect(result.agents).toEqual(["claude", "codex"]);