diff --git a/.changeset/22659-scaffold-skills-from-package.md b/.changeset/22659-scaffold-skills-from-package.md new file mode 100644 index 00000000000..26e8e20083e --- /dev/null +++ b/.changeset/22659-scaffold-skills-from-package.md @@ -0,0 +1,13 @@ +--- +"create-objectstack": minor +--- + +feat(create-objectstack): a scaffolded project installs its AI skills from the `@objectstack/skills` package at its own `@objectstack/spec` version, never from the repository's `main` branch + +Clause-②: no + +- **The project depends on the catalog.** The blank template's `devDependencies` gain `@objectstack/skills`, at the same `^` as every other `@objectstack/*` dependency, so a new project's dependency set changes: the skills are the ones written for the `@objectstack/spec` it installs. +- **The install reads `node_modules`.** After ` install`, the scaffolder runs `npx -y skills@1.7.2 experimental_sync --agent claude-code --copy -y`, which copies the installed package's catalog into `.claude/skills/` as real files, once, beside a `skills-lock.json` that names the package as every skill's source. The GitHub `npx skills add objectstack-ai/objectstack/skills …` command is gone from the run, its summary and the `AGENTS.md` and `README.md` it writes into the project. +- **The summary says what happened.** It names the package version installed and prints the same command for re-installing after an upgrade of the `@objectstack/*` packages, and for any other agent runtime. When the install leaves no catalog to read, or a skill did not come from the package, the run says so instead of reporting the skills installed. +- **The flags keep their meaning.** `--skip-skills` skips the skills step. `--skip-install` installs no skills, because nothing is in `node_modules` yet; the summary names ` install` and then the sync command. +- **To move a project created earlier onto the package:** add `@objectstack/skills` to `devDependencies` at your `@objectstack/spec` range and run your package manager's install. Remove the skills `skills add` installed — `npx skills remove objectstack-data -y` and so on for each `objectstack-*` skill, or `npx skills remove --all` if you installed no others — because the sync skips a skill it did not install and leaves the old copy in place. Then run `npx skills@1.7.2 experimental_sync --agent claude-code --copy -y` (name your own runtime after `--agent`). diff --git a/.github/workflows/scaffold-e2e.yml b/.github/workflows/scaffold-e2e.yml index 623d89da06d..1859cb050de 100644 --- a/.github/workflows/scaffold-e2e.yml +++ b/.github/workflows/scaffold-e2e.yml @@ -235,11 +235,14 @@ jobs: - name: 'Skills boundary: only the curated skills/ catalog installs' # 15.1 third-party eval: the repo-internal dogfood-verification skill - # (.claude/skills/) leaked into scaffolded projects. Two leak paths, - # two assertions against THIS checkout: - # 1. The scaffolder/docs command is `skills add …/objectstack/skills - # --all` — the /skills subpath is the hard boundary, because the - # skills CLI's --all implies --skill '*' which INCLUDES + # (.claude/skills/) leaked into scaffolded projects. The scaffolder no + # longer reads GitHub — it syncs the installed @objectstack/skills + # package, asserted by the step after the pinned shape below — so + # these two assertions now guard the GitHub path the docs keep as the + # `next` channel, against THIS checkout: + # 1. The documented command is `skills add …/objectstack/skills …` — + # the /skills subpath is the hard boundary, because the skills + # CLI's --all implies --skill '*' which INCLUDES # metadata.internal skills. Install from the subpath and assert # set-equality with the curated catalog. # 2. Interactive `skills add …/framework` (no --all) relies on @@ -293,8 +296,10 @@ jobs: # never RUNNING — a scaffolder that stops calling it, or an install path # that no longer leaves a resolvable CLI behind. # - # `--skip-skills` is kept: skills install after the pin, so dropping it - # would only add a network fetch, never coverage. + # No `--skip-skills` either: the scaffolder's skills step reads the + # catalog out of the node_modules this same install wrote, so this is + # the one project in the job whose skills a real user would get. The + # step after this one asserts them. # # The unpublished-version window is the one degradation, and it is # evidenced rather than blanket. The install step above carries the @@ -337,7 +342,7 @@ jobs: # still catches a genuinely broken scaffolder install. run: | cd "$RUNNER_TEMP" - node "$GITHUB_WORKSPACE/packages/create-objectstack/bin/create-objectstack.js" pinned-probe --skip-skills + node "$GITHUB_WORKSPACE/packages/create-objectstack/bin/create-objectstack.js" pinned-probe cd pinned-probe if [ ! -f node_modules/@objectstack/cli/package.json ]; then @@ -377,6 +382,58 @@ jobs: fi echo "pinned as expected: FROM tag == resolved CLI ($RESOLVED)" + - name: 'Skills: the scaffolder installed exactly the @objectstack/skills catalog' + # The pinned-shape project above was scaffolded the way a real user + # does it, skills included. The scaffolder syncs the catalog out of the + # @objectstack/skills package its own install put in node_modules — at + # the range it gives every @objectstack/* dependency, so the version of + # @objectstack/spec — into .claude/skills/ as real files (the measured + # shapes live in packages/create-objectstack/src/skills-install.ts). + # So the catalog this step compares against is the package's, read off + # the registry-installed node_modules, never this checkout's skills/: + # the same skill names, the same count, copied rather than linked. + # + # The unpublished-version window degrades the way the pinned-shape step + # does, and only that window: with nothing installed at all, that step + # has already classified the registry answer and failed on any other + # cause, so this step asserts the degraded shape — no skills from + # anywhere — instead of nothing. An install that resolved the CLI but + # no @objectstack/skills is a template that lost its dependency: red. + run: | + cd "$RUNNER_TEMP/pinned-probe" + PKG=node_modules/@objectstack/skills + if [ ! -f "$PKG/package.json" ]; then + if [ -f node_modules/@objectstack/cli/package.json ]; then + echo "::error::the scaffolder's install resolved @objectstack/cli but no @objectstack/skills, so this project got no skills catalog at its own version — the template no longer depends on the package" + exit 1 + fi + if [ -e .claude/skills ]; then + echo "::error::nothing was installed, yet .claude/skills exists — the scaffolder installed skills from a source other than the @objectstack/skills package" + exit 1 + fi + echo "::warning::no @objectstack/skills is installed — the unpublished-version window the pinned-shape step classified. Catalog EQUALITY is SKIPPED for this run; the degraded shape (no skills from any source) was asserted." + exit 0 + fi + + skills_in() { for d in "$1"/*/; do [ -f "${d}SKILL.md" ] && [ ! -L "${d%/}" ] && basename "$d"; done | sort; } + expected=$(skills_in "$PKG/dist/skills") + installed=$(skills_in .claude/skills) + echo "@objectstack/skills@$(node -p "require('./$PKG/package.json').version") catalog ($(echo "$expected" | grep -c .) skills):"; echo "$expected" + echo "installed in .claude/skills ($(echo "$installed" | grep -c .) skills):"; echo "$installed" + if [ -z "$expected" ]; then + echo "::error::the installed @objectstack/skills holds no dist/skills//SKILL.md — the layout the scaffolder reads has moved" + exit 1 + fi + if [ "$installed" != "$expected" ]; then + echo "::error::the skills the scaffolder installed are not the installed package's catalog (names and counts above)" + exit 1 + fi + if [ -e .agents ] || [ -n "$(find .claude -type l)" ]; then + echo "::error::the scaffolder linked the catalog instead of copying it — a clone of this project would hold dangling links into node_modules" + exit 1 + fi + echo "skills as expected: .claude/skills == the installed @objectstack/skills catalog" + - name: Build official runtime image from this checkout # The scaffolded app's Dockerfile builds FROM # ghcr.io/objectstack-ai/objectstack. Build that base HERE from diff --git a/packages/create-objectstack/README.md b/packages/create-objectstack/README.md index f0ad03beea3..daec9ff5339 100644 --- a/packages/create-objectstack/README.md +++ b/packages/create-objectstack/README.md @@ -55,13 +55,19 @@ you so rather than failing as an unknown template. **namespace** is derived from the project name (`my-app` → `my_app`) and every object name in the template is re-prefixed to match (`blank_note` → `my_app_note`). -2. Installs dependencies (pnpm if available, otherwise npm). -3. Installs the ObjectStack AI skills bundle for **one** coding agent — - `npx skills add objectstack-ai/objectstack/skills --skill '*' --agent claude-code -y`, - scoped to the curated `skills/` catalog. One agent, not `--all`, so the - bundle lands in exactly one directory (`.claude/skills/`) and a scaffolded - project commits it once; `src/skills-install.ts` carries the measurement. - The closing summary prints the same command for any other runtime. +2. Installs dependencies (pnpm if available, otherwise npm). They include + `@objectstack/skills`, the ObjectStack AI skills catalog, at the same + version as `@objectstack/spec`. +3. Installs that catalog for **one** coding agent, from `node_modules` — + `npx -y skills@1.7.2 experimental_sync --agent claude-code --copy -y` — so + the skills teach the version the project runs, never the repository's + unreleased `main`. One agent and `--copy`, so the catalog lands as real + files in exactly one directory (`.claude/skills/`) and a scaffolded project + commits it once; `src/skills-install.ts` carries the measurement. The + closing summary prints the same command for any other runtime, and for + re-installing after an upgrade. With `--skip-install` nothing is in + `node_modules` yet, so no skills are installed and the summary says how to + get them. 4. Writes `AGENTS.md` and `.github/copilot-instructions.md` with the project conventions — unless the template ships its own. diff --git a/packages/create-objectstack/src/created-summary.test.ts b/packages/create-objectstack/src/created-summary.test.ts index 1840e4802f5..1fdd24855f9 100644 --- a/packages/create-objectstack/src/created-summary.test.ts +++ b/packages/create-objectstack/src/created-summary.test.ts @@ -23,12 +23,13 @@ // ## Why synthetic trees rather than a real scaffold // // The real run's last two write phases are ` install` and -// `npx skills add …` — a package manager and a third-party CLI, both needing -// the network. A unit test that depended on them would be a network test that -// fails for reasons unrelated to this property. The shapes that matter are -// reproduced directly instead: a large tree that must collapse, a symlink farm -// that must be counted without being followed, a single-child chain that must -// compress, and a tree past the measurement budget. +// `npx skills experimental_sync …` — a package manager and a third-party CLI, +// both needing the network. A unit test that depended on them would be a +// network test that fails for reasons unrelated to this property. The shapes +// that matter are reproduced directly instead: a large tree that must +// collapse, a single-child chain that must compress, a tree past the +// measurement budget, and — in a tree of its own — a symlink farm that must be +// counted without being followed. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import fs from 'node:fs'; @@ -105,31 +106,28 @@ beforeAll(() => { write('src/objects/index.ts', 'export {};\n'); write('src/objects/note.object.ts', 'export {};\n'); - // Phase 2 — the package manager. + // Phase 2 — the package manager, which also brings the catalog's source: + // the `@objectstack/skills` package the template depends on. write('pnpm-lock.yaml', 'lockfileVersion: 9.0\n'.repeat(400)); for (let i = 0; i < MEASURE_BUDGET + 50; i += 1) { write(`node_modules/pkg-${i}/index.js`, 'module.exports = {};\n'); } + for (const skill of SKILLS) { + write(`node_modules/@objectstack/skills/dist/skills/${skill}/SKILL.md`, '---\nname: x\n---\n'); + } - // Phase 3 — the skills installer: two real trees plus a symlink farm, the - // layout measured from `npx skills add … --all` (one directory per catalog - // skill, `.claude/skills/*` symlinked into `.agents/skills/`). The COUNT is - // faithful on purpose: the two real trees (two files per skill) sit above - // COLLAPSE_AT and take the collapse path the product takes, while the - // ten-link farm sits exactly AT it and enumerates — a 3-skill fixture would - // put every tree on the enumerate path, testing the branch the product does - // not use. + // Phase 3 — the skills installer, in the shape the scaffolder runs it: + // `experimental_sync --agent claude-code --copy` copies the installed + // package's catalog into ONE real tree, `.claude/skills//`, and + // writes `skills-lock.json` (skills-install.ts carries the measurement). + // The COUNT is faithful on purpose: two files per skill put the tree above + // COLLAPSE_AT, so it takes the collapse path the product takes — a 3-skill + // fixture would put it on the enumerate path, testing the branch the + // product does not use. write('skills-lock.json', '{"version":1}\n'); for (const skill of SKILLS) { - for (const tree of ['.agents/skills', 'agent/skills']) { - write(`${tree}/${skill}/SKILL.md`, '---\nname: x\n---\n'.repeat(60)); - write(`${tree}/${skill}/references/guide.md`, 'guide\n'.repeat(60)); - } - fs.mkdirSync(path.join(root, '.claude/skills'), { recursive: true }); - fs.symlinkSync( - path.join('..', '..', '.agents', 'skills', skill), - path.join(root, '.claude/skills', skill), - ); + write(`.claude/skills/${skill}/SKILL.md`, '---\nname: x\n---\n'.repeat(60)); + write(`.claude/skills/${skill}/references/guide.md`, 'guide\n'.repeat(60)); } }); @@ -146,8 +144,9 @@ describe('created-summary — reachability', () => { expect(written).toContain('AGENTS.md'); expect(written).toContain('skills-lock.json'); expect(written).toContain('pnpm-lock.yaml'); - expect(written.filter((p) => p.startsWith('.agents/')).length).toBeGreaterThan(0); - expect(written.filter((p) => p.startsWith('agent/')).length).toBeGreaterThan(0); + expect(written.filter((p) => p.startsWith('.claude/skills/')).length).toBeGreaterThan( + COLLAPSE_AT, + ); expect(written.filter((p) => p.startsWith('node_modules/')).length).toBeGreaterThan( MEASURE_BUDGET, ); @@ -177,9 +176,9 @@ describe('created-summary — reachability', () => { '.github/copilot-instructions.md', 'pnpm-lock.yaml', 'skills-lock.json', - '.agents/skills/objectstack-ai/SKILL.md', - 'agent/skills/objectstack-ai/SKILL.md', - '.claude/skills/objectstack-ai', + '.claude/skills/objectstack-ai/SKILL.md', + '.claude/skills/objectstack-ai/references/guide.md', + 'node_modules/@objectstack/skills/dist/skills/objectstack-ai/SKILL.md', 'node_modules/pkg-0/index.js', ]); expect(missed).toEqual([]); @@ -196,7 +195,7 @@ describe('created-summary — reachability', () => { describe('created-summary — readability', () => { it('collapses big trees instead of enumerating them', () => { const entries = summarizeTree(root); - // 10 skills x 2 files x 2 trees plus a 2050-entry node_modules: an + // 10 skills x 2 files plus a 2050-entry node_modules: an // enumeration would be thousands of lines. The bar is reachability AND a // summary a human reads, so bulk arrives as directory lines. expect(entries.length).toBeLessThan(60); @@ -205,43 +204,68 @@ describe('created-summary — readability', () => { }); it('compresses single-child chains down to the directory worth opening', () => { - // `.agents/` holds only `skills/`, so the line must read `.agents/skills/` + // `.claude/` holds only `skills/`, so the line must read `.claude/skills/` // — the path the "review your skills" advice actually sends people to. const dirs = summarizeTree(root) .filter((e) => e.kind === 'dir') .map((e) => e.path); - expect(dirs).toContain('.agents/skills/'); - expect(dirs).toContain('agent/skills/'); - expect(dirs).not.toContain('.agents/'); + expect(dirs).toContain('.claude/skills/'); + expect(dirs).not.toContain('.claude/'); }); it('counts symlinks without following them', () => { - // `.claude/skills/*` are symlinks into `.agents/skills/`. Following them - // would double-count that tree and report a size the disk does not hold. - // A ten-skill catalog puts the farm exactly AT COLLAPSE_AT, so it is - // ENUMERATED (a directory collapses only above the threshold) and the - // property is read off the ten symlink lines themselves: each is one - // entry whose size is the link's own bytes — the target path string, as - // `lstat` reports it — never the tree it points at. Should the catalog - // grow past COLLAPSE_AT again, the farm collapses to one line and this - // test moves back to asserting on that line's `entries` and `bytes`. + // The scaffolder's own phase 3 writes no links any more (`--copy`), but the + // walk still meets them: pnpm builds node_modules out of them, and the same + // sync WITHOUT `--copy` writes two farms — `.agents/skills/*` into the + // package in node_modules and `.claude/skills/*` into `.agents/skills/` + // (measured, skills-install.ts). That shape is built here in a tree of its + // own. Following the links would double-count the catalog and report a + // size the disk does not hold. A ten-skill catalog puts each farm exactly + // AT COLLAPSE_AT, so it is ENUMERATED (a directory collapses only above the + // threshold) and the property is read off the symlink lines themselves: + // each is one entry whose size is the link's own bytes — the target path + // string, as `lstat` reports it — never the tree it points at. Should the + // catalog grow past COLLAPSE_AT again, the farms collapse to one line each + // and this test moves to asserting on those lines' `entries` and `bytes`. expect( SKILLS.length, 'the farm must sit at or under COLLAPSE_AT for the enumerate-path reading below', ).toBeLessThanOrEqual(COLLAPSE_AT); - const entries = summarizeTree(root); - const links = entries.filter((e) => e.path.startsWith('.claude/skills/')); - expect(links.map((e) => e.path).sort()).toEqual(SKILLS.map((s) => `.claude/skills/${s}`).sort()); - const agents = entries.find((e) => e.path === '.agents/skills/'); - expect(agents, '.agents/skills/ must still collapse — it holds two files per skill').toBeTruthy(); - for (const link of links) { - const target = path.join('..', '..', '.agents', 'skills', link.path.slice('.claude/skills/'.length)); - expect(link.kind).toBe('file'); - expect(link.entries).toBe(1); - expect(link.bytes, `${link.path} must be sized as the link, not as its target`).toBe( - Buffer.byteLength(target), - ); - expect(link.bytes).toBeLessThan(agents!.bytes / SKILLS.length); + const linked = fs.mkdtempSync(path.join(os.tmpdir(), 'summary-linked-')); + try { + const pkgSkills = path.join('node_modules', '@objectstack', 'skills', 'dist', 'skills'); + const targets = new Map(); + for (const skill of SKILLS) { + for (const f of ['SKILL.md', 'references/guide.md']) { + const abs = path.join(linked, pkgSkills, skill, f); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, 'guide\n'.repeat(60)); + } + for (const [farm, target] of [ + ['.agents/skills', path.join('..', '..', pkgSkills, skill)], + ['.claude/skills', path.join('..', '..', '.agents', 'skills', skill)], + ] as const) { + fs.mkdirSync(path.join(linked, farm), { recursive: true }); + fs.symlinkSync(target, path.join(linked, farm, skill)); + targets.set(`${farm}/${skill}`, target); + } + } + const entries = summarizeTree(linked); + expect(unreachablePaths(entries, walkWritten(linked))).toEqual([]); + const real = entries.find((e) => e.path === `${pkgSkills.split(path.sep).join('/')}/`); + expect(real, 'the real catalog tree must still collapse — two files per skill').toBeTruthy(); + const links = entries.filter((e) => targets.has(e.path)); + expect(links.map((e) => e.path).sort()).toEqual([...targets.keys()].sort()); + for (const link of links) { + expect(link.kind).toBe('file'); + expect(link.entries).toBe(1); + expect(link.bytes, `${link.path} must be sized as the link, not as its target`).toBe( + Buffer.byteLength(targets.get(link.path)!), + ); + expect(link.bytes).toBeLessThan(real!.bytes / SKILLS.length); + } + } finally { + fs.rmSync(linked, { recursive: true, force: true }); } }); @@ -255,7 +279,7 @@ describe('created-summary — readability', () => { }); it('describes a fully measured directory with both count and size', () => { - const skills = summarizeTree(root).find((e) => e.path === '.agents/skills/')!; + const skills = summarizeTree(root).find((e) => e.path === '.claude/skills/')!; expect(skills.truncated).toBe(false); expect(describeEntry(skills)).toMatch(/^\d+ files, [\d.]+ (B|KB|MB)$/); }); diff --git a/packages/create-objectstack/src/created-summary.ts b/packages/create-objectstack/src/created-summary.ts index 0f98ab74dcc..b9ea4cf7724 100644 --- a/packages/create-objectstack/src/created-summary.ts +++ b/packages/create-objectstack/src/created-summary.ts @@ -28,16 +28,19 @@ // // 1. template copy + identity rewrite + AGENTS.md/copilot-instructions.md // 2. ` install` -> pnpm-lock.yaml, node_modules/ -// 3. `npx skills add … --agent …` -> .claude/, skills-lock.json +// 3. `npx skills experimental_sync --agent … --copy` +// -> .claude/, skills-lock.json // // and the list was printed between (1) and (2). Phases 2 and 3 are third-party // processes whose outputs this package does not choose and cannot enumerate // ahead of time. Phase 3's destination set was the sharpest case: the run -// measured above used `--all`, which fanned the catalog out to `.agents/`, -// `agent/` AND `.claude/` — three trees, all committed, which is the defect -// `skills-install.ts` now closes by naming one agent. Naming it does not make -// the destination OURS to enumerate: the path is still the skills CLI's -// choice, and it moves with ITS releases. So any hand-maintained list is not +// measured above used `skills add … --all`, which fanned the catalog out to +// `.agents/`, `agent/` AND `.claude/` — three trees, all committed, which is +// the defect `skills-install.ts` closed by naming one agent; it now also reads +// the catalog from the installed `@objectstack/skills` package, and without +// `--copy` the same sync writes two trees of links instead. Naming the agent +// does not make the destination OURS to enumerate: the path is still the +// skills CLI's choice, and it moves with ITS releases. So any hand-maintained list is not // merely incomplete, it is unmaintainable: it drifts the next time a // dependency learns a new destination, silently, in the one direction that // hides files rather than inventing them. diff --git a/packages/create-objectstack/src/index.ts b/packages/create-objectstack/src/index.ts index 7677081f619..8beec56fba1 100644 --- a/packages/create-objectstack/src/index.ts +++ b/packages/create-objectstack/src/index.ts @@ -39,12 +39,13 @@ * `--skip-install` there is no resolved version, so the template keeps `latest` * and its comment keeps telling the reader to pin by hand — true in that path. * - * Finally we (best-effort) install the ObjectStack skills bundle, for ONE - * agent runtime (`skills-install.ts` carries the command and the measurement - * behind that choice: `--all` writes the same bundle to three destinations and - * the project then commits all three). The `/skills` subpath scopes discovery - * to the curated, customer-published catalog — repo-internal skills (e.g. - * under `.claude/skills/` in THIS repo) must never reach scaffolded projects. + * Finally we (best-effort) install the ObjectStack skills catalog, for ONE + * agent runtime, from the `@objectstack/skills` package the install just put in + * `node_modules` — the catalog at the project's own `@objectstack/spec` + * version, never the repository's `main` branch (`skills-install.ts` carries + * the command and the measurements behind it: why the package, why `--copy`, + * why one agent). With no install there is no package to read, so + * `--skip-install` installs no skills and the summary says how to get them. * * Only THEN is the "Created files" summary printed, and it is a walk of the * finished project directory rather than a list accumulated during the copy @@ -79,7 +80,13 @@ import { DEFAULT_SKILLS_DIR, EXAMPLE_OTHER_AGENT, SKILLS_INSTALL_COMMAND, + SKILLS_PACKAGE, + SKILLS_PACKAGE_CATALOG_DIR, + installedSkillsVersion, + listSkills, skillsInstallHint, + skillsPackageDir, + skillsSyncedFromPackage, } from './skills-install.js'; import { renderVersionBanner } from './banner.js'; @@ -379,6 +386,71 @@ function writeIfAbsent(filePath: string, contents: string) { } } +// Install the skills catalog from the `@objectstack/skills` package that +// ` install` put in node_modules, for one agent. Returns the package +// version installed, or `null` after a warning naming the remedy. +// +// The sync's exit code is not the verdict: with no package to read it prints +// "Nothing to sync." and exits 0, and it exits 0 having skipped a skill it did +// not install (`skills-install.ts`). So both sides are read here — a catalog +// in the package before the call, and every one of its skills in the agent's +// directory, recorded in the lock as synced from the package, after it — and +// a gap on either is said out loud rather than summarised as "Installed". +function installSkillsFromPackage( + targetDir: string, + installed: boolean, + pm: string, +): string | null { + const hint = ` ${skillsInstallHint(DEFAULT_SKILLS_AGENT)}`; + if (!installed) { + printWarning( + `Skills not installed: they are read from ${SKILLS_PACKAGE} in node_modules, ` + + `and \`${pm} install\` did not complete. Once it does, run:\n${hint}`, + ); + console.log(''); + return null; + } + + const version = installedSkillsVersion(targetDir); + const catalog = listSkills(path.join(skillsPackageDir(targetDir), SKILLS_PACKAGE_CATALOG_DIR)); + if (!version || catalog.length === 0) { + printWarning( + `Skills not installed: node_modules holds no ${SKILLS_PACKAGE} catalog ` + + `(${SKILLS_PACKAGE_CATALOG_DIR}//SKILL.md) to read. Depend on ${SKILLS_PACKAGE} ` + + `at your @objectstack/spec version, run \`${pm} install\`, then:\n${hint}`, + ); + console.log(''); + return null; + } + + printStep(`Installing AI skills for your coding agent from ${SKILLS_PACKAGE}@${version}...`); + try { + execSync(SKILLS_INSTALL_COMMAND, { stdio: 'inherit', cwd: targetDir }); + } catch { + printWarning(`Skills installation failed. Run manually:\n${hint}`); + console.log(''); + return null; + } + + // Both the files and the lock entry: the sync leaves an existing directory + // it did not write (one from `skills add`) in place and still exits 0. + const landed = new Set(listSkills(path.join(targetDir, DEFAULT_SKILLS_DIR))); + const synced = skillsSyncedFromPackage(targetDir); + const missing = catalog.filter((skill) => !landed.has(skill) || !synced.has(skill)); + if (missing.length > 0) { + printWarning( + `Skills installation incomplete: ${missing.length} of ${catalog.length} skill(s) in ` + + `${SKILLS_PACKAGE}@${version} are not in ${DEFAULT_SKILLS_DIR} as files synced from it ` + + `(${missing.join(', ')}). The sync leaves a skill installed earlier with \`skills add\` ` + + `in place: remove it (\`npx skills remove -y\`), then run:\n${hint}`, + ); + console.log(''); + return null; + } + console.log(''); + return version; +} + // ─── CLI Program ──────────────────────────────────────────────────── const program = new Command() @@ -495,9 +567,11 @@ const program = new Command() printSuccess(`Template files written (${createdFiles.length})`); console.log(''); + // Hoisted: the skills step below reads its catalog out of what this + // install put in node_modules, so it needs to know whether it ran. + let installed = false; if (!options.skipInstall) { printStep('Installing dependencies...'); - let installed = false; try { execSync(`${pm} install`, { stdio: 'inherit', cwd: targetDir }); installed = true; @@ -538,24 +612,12 @@ const program = new Command() // diffing the directory across the call rather than from a list this // file would have to keep current. const beforeSkills = topLevelNames(targetDir); - let skillsInstalled = false; - if (!options.skipInstall && !options.skipSkills) { - printStep('Installing AI skills for your coding agent...'); - try { - execSync(SKILLS_INSTALL_COMMAND, { - stdio: 'inherit', - cwd: targetDir, - }); - skillsInstalled = true; - console.log(''); - } catch { - printWarning( - 'Skills installation skipped. Run manually:\n' + - ` ${skillsInstallHint(DEFAULT_SKILLS_AGENT)}`, - ); - console.log(''); - } - } + // `--skip-install` implies no skills: the catalog is read out of + // node_modules, and that run leaves none. + const skillsVersion = + !options.skipInstall && !options.skipSkills + ? installSkillsFromPackage(targetDir, installed, pm) + : null; const skillPaths = new Set( [...topLevelNames(targetDir)].filter((p) => !beforeSkills.has(p)), ); @@ -587,20 +649,32 @@ const program = new Command() // succeeded still leaves every other runtime uninstalled — and a // capability that is absent has to say so, with the remedy, rather than // leave the reader to discover the gap. The command printed here is the - // one this scaffolder ran, with a different agent name. + // one this scaffolder ran, with a different agent name — and, because + // the catalog follows the installed package, the same command is also + // the re-install after an upgrade of the @objectstack/* set. console.log(''); - if (skillsInstalled) { + if (skillsVersion) { console.log(chalk.bold(' AI Skills:')); console.log( chalk.dim( - ` Installed for ${DEFAULT_SKILLS_AGENT} → ${DEFAULT_SKILLS_DIR} — commit it with your project.`, + ` Installed for ${DEFAULT_SKILLS_AGENT} → ${DEFAULT_SKILLS_DIR} from ` + + `${SKILLS_PACKAGE}@${skillsVersion} — commit it with your project.`, ), ); + console.log(chalk.dim(' After upgrading the @objectstack/* packages, re-install them at the new version:')); + console.log(chalk.dim(` ${skillsInstallHint(DEFAULT_SKILLS_AGENT)}`)); console.log(chalk.dim(' Using a different agent? Add it one runtime at a time:')); console.log(chalk.dim(` ${skillsInstallHint(EXAMPLE_OTHER_AGENT)}`)); - console.log(chalk.dim(' (`npx skills add --help` lists every agent name)')); + console.log(chalk.dim(' (an unknown agent name makes the skills CLI list every valid one)')); } else { console.log(chalk.bold(' AI Skills (recommended):')); + console.log( + chalk.dim( + options.skipInstall + ? ` Not installed yet — they are read from ${SKILLS_PACKAGE} in node_modules. After \`${pm} install\`:` + : ` Read from ${SKILLS_PACKAGE} in node_modules, at your @objectstack/spec version:`, + ), + ); console.log(chalk.dim(` ${skillsInstallHint(DEFAULT_SKILLS_AGENT)}`)); console.log( chalk.dim(` Replace \`${DEFAULT_SKILLS_AGENT}\` with your own agent runtime if it differs.`), diff --git a/packages/create-objectstack/src/scaffold-skills-single-copy.test.ts b/packages/create-objectstack/src/scaffold-skills-single-copy.test.ts index 12c8e84d3c8..cdd1bb168c2 100644 --- a/packages/create-objectstack/src/scaffold-skills-single-copy.test.ts +++ b/packages/create-objectstack/src/scaffold-skills-single-copy.test.ts @@ -1,49 +1,64 @@ // Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. // -// Commit fd75728bc — a scaffolded project's FIRST `git add -A` must stage the skills -// bundle exactly once, and a clone of that commit must yield readable -// `SKILL.md` files. +// The skills phase of a real scaffold, end to end. Two properties: // -// ## The measurement this file encodes +// 1. Commit fd75728bc — a scaffolded project's FIRST `git add -A` stages the +// skills catalog exactly once, as real files, and a clone of that commit +// yields readable `SKILL.md` files. +// 2. Ruling A on the skills-catalog binding (record 6096124407) — the +// installed skill set IS the catalog of the `@objectstack/skills` package +// the project depends on, at `@objectstack/spec`'s range: the same names, +// the same count, read out of the project's own node_modules and never +// from GitHub. // -// Taken against `skills@1.5.23` and the published `objectstack-ai/objectstack/ -// skills` catalog (11 skills), each into an empty directory: +// ## The measurement the stubs encode // -// --all .agents/ 46 real files (604,102 B) -// (= --skill '*' --agent '*' -y) agent/ 46 real files (602,682 B) -// .claude/ 11 symlinks into .agents/ -// --skill '*' --agent claude-code -y .claude/ 46 real files (604,102 B) -// --skill '*' --agent universal -y .agents/ 46 real files (604,102 B) -// --all --copy 56 destination dirs, 33.8 MB total +// `skills-install.ts` carries it: `skills@1.7.2` against the published +// `@objectstack/skills@18.0.0-next.0`. `experimental_sync` discovers skills in +// the project's DIRECT dependencies only (`skills/` or `dist/skills/` of each), +// and writes, for `--agent claude-code`: // -// and then, with the template's `.gitignore` in place, `git init && git add -A`: +// --copy -y .claude/skills/ real files, plus skills-lock.json +// -y .agents/skills/ links into node_modules//… +// .claude/skills/ links into .agents/skills/ // -// --all shape 22 staged SKILL.md paths + 11 staged symlinks -// claude-code shape 11 staged SKILL.md paths, 0 symlinks +// With nothing to read it prints "Nothing to sync." and exits 0; a skill whose +// directory is already there and whose lock entry is not a sync's (one from +// `skills add`) is SKIPPED, left as it was, and the run still exits 0. // -// 22 is the defect: the same eleven skills, staged twice as real files. +// ## Why both CLIs are stubbed, and how the stubs are kept honest // -// ## Why the skills CLI is stubbed, and how the stub is kept honest +// The real ` install` and the real skills CLI need the network, so a test +// that ran them would be measuring the runner. The stubs encode the measured +// behaviour instead, and three properties keep them from becoming a mirror +// that agrees with whatever the scaffolder does: // -// The real installer needs the network and a GitHub clone, so a test that ran -// it would be measuring the runner. The stub instead encodes the DESTINATION -// MAP above — argv in, directory layout out — and `INSTALL_SHAPES` is that -// table verbatim. Two properties keep it from becoming a mirror that agrees -// with whatever we do: +// * the `pnpm` stub's `install` materializes `@objectstack/skills` ONLY when +// the project's `package.json` declares it, in the layout the package +// publishes (`dist/skills//SKILL.md`, beside the catalog README the +// real tarball carries). A template that stops depending on the package +// gets no catalog, exactly as a real install would. +// * the `npx` stub REFUSES (exit 3) any argv it has no measured row for: a +// CLI version nobody measured, any `skills add` (the GitHub path), an +// agent nobody measured. Point the scaffolder back at GitHub, or move the +// pin without re-measuring, and the end-to-end case fails loudly instead of +// passing against an invented layout. `MEASURED_CLI` below is written out, +// never imported, for that reason. +// * the linked row is exercised too, by the vacuity case, and the +// single-copy assertions must FAIL on it. An assertion that cannot fail is +// the one failure mode a fixture-driven test really has. // -// * the stub REFUSES an argv it has no measured row for (exit 3). Change the -// scaffolder's command to something nobody measured and the end-to-end -// case fails loudly instead of passing against an invented layout. -// * the `--all` row is exercised too, by the vacuity case, and the same -// assertions must FAIL on it. An assertion that cannot fail is the one -// failure mode a fixture-driven test really has. +// The catalog side of property 2 is read twice: off the project's own +// node_modules (what the package really holds), and against `STUB_CATALOG` +// (what the stub install put there). The second reading is the one that makes +// an empty install go red rather than compare empty with empty. // -// The end-to-end case runs the REAL CLI through `tsx` under a stubbed PATH — +// The end-to-end cases run the REAL CLI through `tsx` under a stubbed PATH — // the pattern `scaffold-next-steps-pm.test.ts` established, and for the same // reason: `index.ts` calls `program.parse()` at import, so nothing in it can -// be reached any other way. Its harness guard is the stub's own argv receipt: -// if the child resolved the ambient `npx` instead of ours, there is no receipt -// and the case fails rather than quietly measuring the network. +// be reached any other way. Their harness guard is the stub's own argv +// receipt: if the child resolved the ambient `npx` instead of ours, there is +// no receipt and the case fails rather than quietly measuring the network. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { execFileSync } from 'node:child_process'; @@ -55,10 +70,14 @@ import { DEFAULT_SKILLS_AGENT, DEFAULT_SKILLS_DIR, EXAMPLE_OTHER_AGENT, - SKILLS_CATALOG, + SKILLS_CLI, SKILLS_INSTALL_COMMAND, - skillsAddArgs, + SKILLS_PACKAGE, + SKILLS_PACKAGE_CATALOG_DIR, + listSkills, skillsInstallHint, + skillsPackageDir, + skillsSyncArgs, } from './skills-install.js'; const HERE = path.dirname(fileURLToPath(import.meta.url)); @@ -66,90 +85,127 @@ const PKG_ROOT = path.resolve(HERE, '..'); const REPO_ROOT = path.resolve(PKG_ROOT, '..', '..'); const TSX = path.join(REPO_ROOT, 'node_modules', '.bin', 'tsx'); const INDEX_TS = path.join(PKG_ROOT, 'src', 'index.ts'); -const TEMPLATE_GITIGNORE = path.join( - PKG_ROOT, - 'src', - 'templates', - 'blank', - '_gitignore', -); +const TEMPLATE_GITIGNORE = path.join(PKG_ROOT, 'src', 'templates', 'blank', '_gitignore'); + +/** The skills the stub install puts in the package. Three, so "twice" is unmistakably six. */ +const STUB_CATALOG = ['objectstack-data', 'objectstack-query', 'objectstack-ui']; + +/** The CLI version the rows below were measured with — written out, never imported. */ +const MEASURED_CLI = 'skills@1.7.2'; -/** The skills the stub installs. Three, so "twice" is unmistakably six. */ -const STUB_SKILLS = ['objectstack-data', 'objectstack-query', 'objectstack-ui']; +/** A stub `pnpm` whose `install` materializes the declared catalog package. */ +function pnpmStub(): string { + return `import fs from 'node:fs'; +import path from 'node:path'; +const CATALOG = ${JSON.stringify(STUB_CATALOG)}; +const argv = process.argv.slice(2); +if (argv[0] === '--version') { console.log('10.31.0'); process.exit(0); } +if (argv[0] !== 'install' || process.env.PNPM_STUB_REGISTRY === 'empty') process.exit(0); +const cwd = process.cwd(); +const pkg = JSON.parse(fs.readFileSync(path.join(cwd, 'package.json'), 'utf8')); +const range = { ...pkg.dependencies, ...pkg.devDependencies }['@objectstack/skills']; +if (range === undefined) process.exit(0); +const root = path.join(cwd, 'node_modules', '@objectstack', 'skills'); +const catalog = path.join(root, 'dist', 'skills'); +fs.mkdirSync(catalog, { recursive: true }); +fs.writeFileSync( + path.join(root, 'package.json'), + JSON.stringify({ name: '@objectstack/skills', version: String(range).replace(/^[~^]/, '') }) + '\\n', +); +fs.writeFileSync(path.join(catalog, 'README.md'), '# catalog\\n'); +for (const skill of CATALOG) { + fs.mkdirSync(path.join(catalog, skill, 'references'), { recursive: true }); + fs.writeFileSync(path.join(catalog, skill, 'SKILL.md'), '---\\nname: ' + skill + '\\n---\\n\\n# ' + skill + '\\n'); + fs.writeFileSync(path.join(catalog, skill, 'references', 'guide.md'), '# guide for ' + skill + '\\n'); +} +`; +} -/** The measured destination map — see the header table. */ -const INSTALL_SHAPES = ` +/** A stub `npx` reproducing the measured `experimental_sync` rows; it records every argv. */ +function npxStub(): string { + return `import fs from 'node:fs'; +import path from 'node:path'; +const MEASURED_CLI = ${JSON.stringify(MEASURED_CLI)}; const SHAPES = { - 'agent:*': [ - { dir: '.agents/skills', kind: 'real' }, - { dir: 'agent/skills', kind: 'real' }, + 'copy:claude-code': [{ dir: '.claude/skills', kind: 'copy' }], + 'link:claude-code': [ + { dir: '.agents/skills', kind: 'link-package' }, { dir: '.claude/skills', kind: 'link', into: '../../.agents/skills' }, ], - 'agent:claude-code': [{ dir: '.claude/skills', kind: 'real' }], - 'agent:universal': [{ dir: '.agents/skills', kind: 'real' }], }; -`; - -/** A stub `npx` that reproduces INSTALL_SHAPES and records the argv it saw. */ -function writeStubs(binDir: string, receipt: string): void { - fs.mkdirSync(binDir, { recursive: true }); - - const npxJs = path.join(binDir, 'npx-stub.mjs'); - fs.writeFileSync( - npxJs, - `import fs from 'node:fs'; -import path from 'node:path'; -${INSTALL_SHAPES} -const SKILLS = ${JSON.stringify(STUB_SKILLS)}; const argv = process.argv.slice(2); -fs.appendFileSync(${JSON.stringify(receipt)}, JSON.stringify(argv) + '\\n'); - -if (!argv.includes('skills') || !argv.includes('add')) process.exit(0); -const agents = argv.includes('--all') - ? '*' - : (argv[argv.indexOf('--agent') + 1] ?? ''); -const shape = SHAPES['agent:' + agents]; -if (!shape) { - console.error('npx-stub: no measured install shape for --agent ' + agents); +if (process.env.NPX_STUB_RECEIPT) { + fs.appendFileSync(process.env.NPX_STUB_RECEIPT, JSON.stringify(argv) + '\\n'); +} +const args = argv[0] === '-y' ? argv.slice(1) : argv; +const [cli, command, ...rest] = args; +const at = rest.indexOf('--agent'); +const key = (rest.includes('--copy') ? 'copy' : 'link') + ':' + (at < 0 ? '' : rest[at + 1]); +if (cli !== MEASURED_CLI || command !== 'experimental_sync' || !rest.includes('-y') || !SHAPES[key]) { + console.error('npx-stub: no measured shape for: ' + args.join(' ')); process.exit(3); } const cwd = process.cwd(); -for (const dest of shape) { - fs.mkdirSync(path.join(cwd, dest.dir), { recursive: true }); - for (const skill of SKILLS) { - const at = path.join(cwd, dest.dir, skill); - if (dest.kind === 'link') { - fs.symlinkSync(dest.into + '/' + skill, at); - continue; +const pkg = JSON.parse(fs.readFileSync(path.join(cwd, 'package.json'), 'utf8')); +const deps = new Set( + ['dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies'] + .flatMap((field) => Object.keys(pkg[field] ?? {})), +); +const found = []; +for (const dep of deps) { + const dir = path.join(cwd, 'node_modules', ...dep.split('/')); + let version = null; + try { version = JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')).version; } catch {} + for (const sub of ['skills', 'dist/skills']) { + let names = []; + try { names = fs.readdirSync(path.join(dir, sub)); } catch {} + for (const name of names) { + if (!fs.existsSync(path.join(dir, sub, name, 'SKILL.md'))) continue; + found.push({ dep, name, version, src: path.join(dir, sub, name), skillPath: sub + '/' + name + '/SKILL.md' }); } - fs.mkdirSync(at, { recursive: true }); - fs.writeFileSync( - path.join(at, 'SKILL.md'), - '---\\nname: ' + skill + '\\n---\\n\\n# ' + skill + '\\n', - ); - fs.writeFileSync(path.join(at, 'reference.md'), '# reference for ' + skill + '\\n'); } } -fs.writeFileSync( - path.join(cwd, 'skills-lock.json'), - JSON.stringify({ version: 1, skills: Object.fromEntries(SKILLS.map((s) => [s, {}])) }, null, 2) + '\\n', -); -console.log('npx-stub: installed ' + SKILLS.length + ' skills'); -`, - 'utf8', - ); - - const shim = (body: string) => `#!/bin/sh\n${body}\n`; - fs.writeFileSync(path.join(binDir, 'npx'), shim(`exec "${process.execPath}" "${npxJs}" "$@"`)); - // `pnpm --version` must answer so detectPackageManager reports probe:'ok', - // and `pnpm install` must be a no-op that succeeds: with no node_modules - // written, readResolvedCliVersion has no opinion and the Dockerfile pin is - // skipped, which keeps this file's subject to the skills phase alone. - fs.writeFileSync( - path.join(binDir, 'pnpm'), - shim('if [ "$1" = "--version" ]; then echo 10.31.0; fi\nexit 0'), - ); - for (const f of ['npx', 'pnpm']) fs.chmodSync(path.join(binDir, f), 0o755); +if (found.length === 0) { console.log('Nothing to sync.'); process.exit(0); } +const lockPath = path.join(cwd, 'skills-lock.json'); +let prior = { version: 1, skills: {} }; +try { prior = JSON.parse(fs.readFileSync(lockPath, 'utf8')); } catch {} +const synced = found.filter((skill) => { + const taken = SHAPES[key].some((dest) => fs.existsSync(path.join(cwd, dest.dir, skill.name))); + if (taken && prior.skills?.[skill.name]?.sourceType !== 'node_modules') { + console.warn('Skipped ' + skill.name + ' from ' + skill.dep + ': installed with skills add'); + return false; + } + return true; +}); +for (const dest of SHAPES[key]) { + fs.mkdirSync(path.join(cwd, dest.dir), { recursive: true }); + for (const skill of synced) { + const target = path.join(cwd, dest.dir, skill.name); + if (dest.kind === 'copy') fs.cpSync(skill.src, target, { recursive: true }); + else if (dest.kind === 'link-package') fs.symlinkSync(path.relative(path.dirname(target), skill.src), target); + else fs.symlinkSync(dest.into + '/' + skill.name, target); + } +} +const lock = { version: 1, skills: { ...prior.skills } }; +for (const s of synced) { + lock.skills[s.name] = { source: s.dep, sourceType: 'node_modules', skillPath: s.skillPath, version: s.version }; +} +fs.writeFileSync(path.join(cwd, 'skills-lock.json'), JSON.stringify(lock, null, 2) + '\\n'); +console.log('npx-stub: synced ' + synced.length + ' skills'); +`; +} + +function writeStubs(binDir: string): void { + fs.mkdirSync(binDir, { recursive: true }); + for (const [name, body] of [ + ['npx', npxStub()], + ['pnpm', pnpmStub()], + ] as const) { + const js = path.join(binDir, `${name}-stub.mjs`); + fs.writeFileSync(js, body, 'utf8'); + fs.writeFileSync(path.join(binDir, name), `#!/bin/sh\nexec "${process.execPath}" "${js}" "$@"\n`); + fs.chmodSync(path.join(binDir, name), 0o755); + } } interface Staged { @@ -196,13 +252,38 @@ function cloneAndRead(project: string, into: string) { let TMP: string; let BIN: string; -let RECEIPT: string; + +/** Run the real scaffolder under the stubs; returns its stdout and the argv the npx stub saw. */ +function scaffold(name: string, flags: string[] = [], env: Record = {}) { + const receipt = path.join(TMP, `${name}.npx-argv.log`); + const stdout = execFileSync(TSX, [INDEX_TS, name, ...flags], { + cwd: TMP, + encoding: 'utf8', + env: { + ...process.env, + ...env, + NPX_STUB_RECEIPT: receipt, + PATH: `${BIN}${path.delimiter}${process.env.PATH ?? ''}`, + }, + }); + const argv: string[][] = fs.existsSync(receipt) + ? fs.readFileSync(receipt, 'utf8').split('\n').filter(Boolean).map((l) => JSON.parse(l)) + : []; + return { project: path.join(TMP, name), stdout, argv }; +} + +/** The closing summary's skills block — what the run tells the reader about skills. */ +function skillsBlock(stdout: string): string { + return stdout.slice(stdout.lastIndexOf('AI Skills')); +} + +const catalogOf = (project: string) => + listSkills(path.join(skillsPackageDir(project), SKILLS_PACKAGE_CATALOG_DIR)); beforeAll(() => { TMP = fs.mkdtempSync(path.join(os.tmpdir(), 'skills-single-copy-')); BIN = path.join(TMP, 'bin'); - RECEIPT = path.join(TMP, 'npx-argv.log'); - writeStubs(BIN, RECEIPT); + writeStubs(BIN); }); afterAll(() => { @@ -210,28 +291,30 @@ afterAll(() => { }); // ── 1. the command itself ─────────────────────────────────────────────────── -describe('the skills install command names exactly one agent', () => { - it('never passes --all or --agent *', () => { - expect(SKILLS_INSTALL_COMMAND).not.toContain('--all'); - expect(SKILLS_INSTALL_COMMAND).not.toMatch(/--agent\s+'?\*/); +describe('the skills install command reads the installed package, for one agent', () => { + it('syncs from node_modules and names no GitHub source', () => { + expect(SKILLS_INSTALL_COMMAND).toContain(' experimental_sync '); + expect(SKILLS_INSTALL_COMMAND).not.toMatch(/skills add|objectstack-ai\/|github/i); }); - it("selects every skill, with the glob quoted against the shell", () => { - expect(SKILLS_INSTALL_COMMAND).toContain("--skill '*'"); + it('copies the catalog instead of linking into node_modules', () => { + expect(SKILLS_INSTALL_COMMAND.split(' ')).toContain('--copy'); }); - it('names one agent, once', () => { + it('names one agent, once, and never all of them', () => { const agents = SKILLS_INSTALL_COMMAND.match(/--agent\s+(\S+)/g) ?? []; expect(agents).toEqual([`--agent ${DEFAULT_SKILLS_AGENT}`]); + expect(SKILLS_INSTALL_COMMAND).not.toMatch(/--agent\s+'?\*|--all/); }); - it('installs from the curated catalog subpath', () => { - expect(SKILLS_INSTALL_COMMAND).toContain(`skills add ${SKILLS_CATALOG} `); + it('runs the skills CLI at a pinned version', () => { + expect(SKILLS_CLI).toMatch(/^skills@\d+\.\d+\.\d+$/); + expect(SKILLS_INSTALL_COMMAND.startsWith(`npx -y ${SKILLS_CLI} experimental_sync `)).toBe(true); }); // The printed hint and the executed command must come out of ONE builder: // a user told to run something the scaffolder never ran is how a project - // acquires the second copy this card removes. + // acquires a second copy, or a catalog from the wrong version. it('the printed hint differs from what runs only by the agent name', () => { expect(skillsInstallHint(DEFAULT_SKILLS_AGENT)).toBe( SKILLS_INSTALL_COMMAND.replace('npx -y ', 'npx '), @@ -242,116 +325,219 @@ describe('the skills install command names exactly one agent', () => { `--agent ${EXAMPLE_OTHER_AGENT}`, ), ); - expect(skillsAddArgs(EXAMPLE_OTHER_AGENT)).toContain(`--agent ${EXAMPLE_OTHER_AGENT}`); + expect(skillsSyncArgs(EXAMPLE_OTHER_AGENT)).toContain(`--agent ${EXAMPLE_OTHER_AGENT}`); + }); + + it('listSkills reads real skill directories only, sorted', () => { + const dir = path.join(TMP, 'list-skills'); + for (const skill of ['b-skill', 'a-skill']) { + fs.mkdirSync(path.join(dir, skill), { recursive: true }); + fs.writeFileSync(path.join(dir, skill, 'SKILL.md'), '# x\n'); + } + fs.mkdirSync(path.join(dir, 'not-a-skill'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'README.md'), '# catalog\n'); + fs.symlinkSync('a-skill', path.join(dir, 'linked-skill')); + expect(listSkills(dir)).toEqual(['a-skill', 'b-skill']); + expect(listSkills(path.join(TMP, 'no-such-dir'))).toEqual([]); }); }); // ── 2. end to end: a real scaffold, staged and cloned ─────────────────────── -describe('a scaffolded project stages the bundle exactly once', () => { - let project: string; +describe('a scaffolded project installs the package catalog, once', () => { + let run: ReturnType; let staged: Staged; beforeAll(() => { - project = path.join(TMP, 'staged-once-app'); - execFileSync(TSX, [INDEX_TS, 'staged-once-app'], { - cwd: TMP, - encoding: 'utf8', - env: { ...process.env, PATH: `${BIN}${path.delimiter}${process.env.PATH ?? ''}` }, - }); - staged = stageAll(project); + run = scaffold('staged-once-app'); + staged = stageAll(run.project); }); // Harness guard: without a receipt the child ran SOME other npx and every // assertion below would be measuring that instead. it("ran the stubbed skills CLI, with the scaffolder's own argv", () => { - const lines = fs.readFileSync(RECEIPT, 'utf8').split('\n').filter(Boolean); - const args: string[][] = lines.map((l) => JSON.parse(l)); - const add = args.find((a) => a.includes('add')); - expect(add, 'the scaffolder never reached the stubbed npx').toBeDefined(); - expect(add).toContain(SKILLS_CATALOG); - expect(add?.[add.indexOf('--agent') + 1]).toBe(DEFAULT_SKILLS_AGENT); - expect(add).not.toContain('--all'); + expect(run.argv, 'the scaffolder never reached the stubbed npx').toEqual([ + SKILLS_INSTALL_COMMAND.split(' ').slice(1), + ]); + }); + + it("depends on @objectstack/skills at @objectstack/spec's range", () => { + const pkg = JSON.parse(fs.readFileSync(path.join(run.project, 'package.json'), 'utf8')); + const spec = pkg.dependencies?.['@objectstack/spec']; + expect(spec).toMatch(/^\^\d+\./); + expect(pkg.devDependencies?.[SKILLS_PACKAGE]).toBe(spec); + }); + + it("installs exactly the package's catalog — the same count, the same names", () => { + const catalog = catalogOf(run.project); + expect(catalog, 'the stub install did not put the catalog in node_modules').toEqual(STUB_CATALOG); + const installed = listSkills(path.join(run.project, DEFAULT_SKILLS_DIR)); + expect(installed).toHaveLength(catalog.length); + expect(installed).toEqual(catalog); }); - it('writes the bundle to exactly one directory', () => { + it('writes the catalog to exactly one directory', () => { const roots = new Set(staged.skillMd.map((p) => p.split('/')[0])); expect([...roots]).toEqual([DEFAULT_SKILLS_DIR.split('/')[0]]); + expect(fs.existsSync(path.join(run.project, '.agents'))).toBe(false); }); it('stages one SKILL.md per skill — not two, not three', () => { - expect(staged.skillMd).toHaveLength(STUB_SKILLS.length); + expect(staged.skillMd).toHaveLength(STUB_CATALOG.length); }); it('stages no symlinks at all', () => { expect(staged.symlinks).toEqual([]); }); - it('stages the lockfile that records what was installed', () => { + it('stages the lockfile, which records the package as the source of every skill', () => { expect(staged.all).toContain('skills-lock.json'); + const lock = JSON.parse(fs.readFileSync(path.join(run.project, 'skills-lock.json'), 'utf8')); + expect(Object.keys(lock.skills).sort()).toEqual(STUB_CATALOG); + for (const entry of Object.values(lock.skills) as Array<{ source: string; sourceType: string }>) { + expect(entry).toMatchObject({ source: SKILLS_PACKAGE, sourceType: 'node_modules' }); + } }); it('yields readable SKILL.md files when that commit is cloned', () => { - const clone = cloneAndRead(project, path.join(TMP, 'clone-staged-once')); - expect(clone.skillMd).toHaveLength(STUB_SKILLS.length); + const clone = cloneAndRead(run.project, path.join(TMP, 'clone-staged-once')); + expect(clone.skillMd).toHaveLength(STUB_CATALOG.length); expect(clone.readable).toEqual(clone.skillMd); expect(clone.dangling).toEqual([]); }); + + it('the summary names the package version and the re-install command, and no GitHub source', () => { + const block = skillsBlock(run.stdout); + const pkg = JSON.parse( + fs.readFileSync(path.join(skillsPackageDir(run.project), 'package.json'), 'utf8'), + ); + expect(block).toContain(`${DEFAULT_SKILLS_DIR} from ${SKILLS_PACKAGE}@${pkg.version}`); + expect(block).toContain(skillsInstallHint(DEFAULT_SKILLS_AGENT)); + expect(block).toContain(skillsInstallHint(EXAMPLE_OTHER_AGENT)); + expect(run.stdout).not.toMatch(/skills add|objectstack-ai\/objectstack/); + }); }); -// ── 3. vacuity: the pre-fix shape must FAIL both assertions ───────────────── -// -// Same stub, same git steps, the `--all` row of the measured table. If these -// two cases ever go green the assertions above have stopped meaning anything. -describe('the pre-fix `--all` shape fails the same property', () => { - function installAll(project: string, extraIgnores: string[] = []): void { - fs.mkdirSync(project, { recursive: true }); +// ── 3. the two opt-outs, and an absent package ────────────────────────────── +describe('without a package to read, no skills — and the run says how to get them', () => { + it('--skip-install: nothing in node_modules, so no sync; the summary names install-then-sync', () => { + const run = scaffold('no-install-app', ['--skip-install']); + expect(run.argv).toEqual([]); + expect(fs.existsSync(path.join(run.project, 'node_modules'))).toBe(false); + expect(fs.existsSync(path.join(run.project, '.claude'))).toBe(false); + const block = skillsBlock(run.stdout); + expect(block).toContain('After `pnpm install`'); + expect(block).toContain(skillsInstallHint(DEFAULT_SKILLS_AGENT)); + expect(run.stdout).not.toMatch(/skills add|objectstack-ai\/objectstack/); + }); + + it('--skip-skills: the install runs and the sync does not; the summary recommends it', () => { + const run = scaffold('no-skills-app', ['--skip-skills']); + expect(catalogOf(run.project), 'the install step did not run').toEqual(STUB_CATALOG); + expect(run.argv).toEqual([]); + expect(fs.existsSync(path.join(run.project, '.claude'))).toBe(false); + const block = skillsBlock(run.stdout); + expect(block).toMatch(/^AI Skills \(recommended\)/); + expect(block).toContain(skillsInstallHint(DEFAULT_SKILLS_AGENT)); + }); + + // Scaffolding into the current directory is the one path onto existing + // content. A skill that `skills add` put there earlier is SKIPPED by the + // sync, exit 0, its directory still present — so neither the exit code nor + // the directory can be the verdict; the lock entry naming the package is. + it('a skill already there from `skills add` is reported, not counted as installed', () => { + const project = path.join(TMP, 'here-app'); + const legacy = path.join(project, '.claude', 'skills', 'objectstack-data'); + fs.mkdirSync(legacy, { recursive: true }); + fs.writeFileSync(path.join(legacy, 'SKILL.md'), '---\nname: objectstack-data\n---\n\n# from main\n'); fs.writeFileSync( - path.join(project, '.gitignore'), - fs.readFileSync(TEMPLATE_GITIGNORE, 'utf8') + - extraIgnores.map((l) => `${l}\n`).join(''), + path.join(project, 'skills-lock.json'), + JSON.stringify({ version: 1, skills: { 'objectstack-data': { source: 'objectstack-ai/objectstack', sourceType: 'github' } } }) + '\n', ); - execFileSync(path.join(BIN, 'npx'), ['-y', 'skills', 'add', SKILLS_CATALOG, '--all'], { + const receipt = path.join(TMP, 'here-app.npx-argv.log'); + const stdout = execFileSync(TSX, [INDEX_TS], { cwd: project, encoding: 'utf8', + env: { ...process.env, NPX_STUB_RECEIPT: receipt, PATH: `${BIN}${path.delimiter}${process.env.PATH ?? ''}` }, }); - } + expect(fs.readFileSync(receipt, 'utf8').trim().split('\n')).toHaveLength(1); + expect(listSkills(path.join(project, DEFAULT_SKILLS_DIR))).toEqual(STUB_CATALOG); + expect(fs.readFileSync(path.join(legacy, 'SKILL.md'), 'utf8')).toContain('# from main'); + expect(stdout).toContain('1 of 3 skill(s)'); + expect(stdout).toContain('(objectstack-data)'); + expect(stdout).not.toContain('Installed for'); + }); - it('stages the same skills twice with nothing ignored', () => { - const project = path.join(TMP, 'all-shape'); - installAll(project); - const staged = stageAll(project); - expect(staged.skillMd).toHaveLength(STUB_SKILLS.length * 2); - expect(staged.symlinks).toHaveLength(STUB_SKILLS.length); + // The sync exits 0 with "Nothing to sync." when there is no package, so a + // scaffolder that trusted the exit code would print "Installed". + it('an install that leaves no catalog: the run says so, and never claims Installed', () => { + const run = scaffold('no-catalog-app', [], { PNPM_STUB_REGISTRY: 'empty' }); + expect(catalogOf(run.project)).toEqual([]); + expect(run.argv).toEqual([]); + expect(run.stdout).toContain(`node_modules holds no ${SKILLS_PACKAGE} catalog`); + expect(run.stdout).not.toContain('Installed for'); + expect(skillsBlock(run.stdout)).toContain(skillsInstallHint(DEFAULT_SKILLS_AGENT)); }); +}); + +// ── 4. vacuity: the shapes NOT taken must FAIL ────────────────────────────── +// +// Same stubs, same git steps. If these cases ever go green the assertions +// above have stopped meaning anything. +describe('the shapes the scaffolder does not run fail the same properties', () => { + /** A project holding only what these cases read: the ignore list, the dependency, the install. */ + function projectWithPackage(name: string): string { + const project = path.join(TMP, name); + fs.mkdirSync(project, { recursive: true }); + fs.copyFileSync(TEMPLATE_GITIGNORE, path.join(project, '.gitignore')); + fs.writeFileSync( + path.join(project, 'package.json'), + JSON.stringify({ name, devDependencies: { [SKILLS_PACKAGE]: '^18.0.0' } }) + '\n', + ); + execFileSync(path.join(BIN, 'pnpm'), ['install'], { cwd: project, encoding: 'utf8' }); + return project; + } + + function npx(cwd: string, args: string[]): number { + try { + execFileSync(path.join(BIN, 'npx'), args, { cwd, encoding: 'utf8', stdio: 'pipe' }); + return 0; + } catch (err) { + return (err as { status?: number }).status ?? -1; + } + } - // The route this card did NOT take, measured rather than argued: ignoring - // the real trees while committing `.claude/` produces a clone with nothing - // in it but broken links. - it('leaves dangling symlinks when the real trees are gitignored instead', () => { - const project = path.join(TMP, 'all-shape-ignored'); - installAll(project, ['.agents/', 'agent/']); + // The route this card did NOT take, measured rather than argued: the sync's + // default links point into node_modules, which is never committed, so the + // clone holds nothing but broken links. + it('the linked sync (no --copy) stages links only, and a clone of it reads nothing', () => { + const project = projectWithPackage('linked-shape'); + expect(npx(project, ['-y', MEASURED_CLI, 'experimental_sync', '--agent', DEFAULT_SKILLS_AGENT, '-y'])).toBe(0); const staged = stageAll(project); expect(staged.skillMd).toEqual([]); - expect(staged.symlinks).toHaveLength(STUB_SKILLS.length); + expect(staged.symlinks).toHaveLength(STUB_CATALOG.length * 2); - const clone = cloneAndRead(project, path.join(TMP, 'clone-all-ignored')); - expect(clone.dangling).toHaveLength(STUB_SKILLS.length); - expect(clone.skillMd).toEqual([]); + const clone = cloneAndRead(project, path.join(TMP, 'clone-linked')); + expect(clone.dangling).toHaveLength(STUB_CATALOG.length * 2); + expect(clone.readable).toEqual([]); }); - it('refuses an argv it has no measured shape for', () => { - const project = path.join(TMP, 'unmeasured'); + it('the sync with no package to read installs nothing and still exits 0', () => { + const project = path.join(TMP, 'nothing-to-sync'); fs.mkdirSync(project, { recursive: true }); - let code = 0; - try { - execFileSync( - path.join(BIN, 'npx'), - ['-y', 'skills', 'add', SKILLS_CATALOG, '--agent', 'no-such-runtime'], - { cwd: project, encoding: 'utf8', stdio: 'pipe' }, - ); - } catch (err) { - code = (err as { status?: number }).status ?? 0; + fs.writeFileSync(path.join(project, 'package.json'), '{"name":"nothing-to-sync"}\n'); + expect(npx(project, SKILLS_INSTALL_COMMAND.split(' ').slice(1))).toBe(0); + expect(fs.existsSync(path.join(project, '.claude'))).toBe(false); + }); + + it('refuses an argv it has no measured shape for — the GitHub path among them', () => { + const project = projectWithPackage('unmeasured'); + for (const args of [ + ['-y', 'skills', 'add', 'objectstack-ai/objectstack/skills', '--skill', '*', '--agent', DEFAULT_SKILLS_AGENT, '-y'], + ['-y', 'skills', 'experimental_sync', '--agent', DEFAULT_SKILLS_AGENT, '--copy', '-y'], + ['-y', MEASURED_CLI, 'experimental_sync', '--agent', 'no-such-runtime', '--copy', '-y'], + ]) { + expect(npx(project, args), `the stub agreed with an unmeasured shape: ${args.join(' ')}`).toBe(3); } - expect(code, 'the stub agreed with an install shape nobody measured').toBe(3); + expect(fs.existsSync(path.join(project, '.claude'))).toBe(false); }); }); diff --git a/packages/create-objectstack/src/skills-install.ts b/packages/create-objectstack/src/skills-install.ts index f10ff5de7e3..8ec0100b633 100644 --- a/packages/create-objectstack/src/skills-install.ts +++ b/packages/create-objectstack/src/skills-install.ts @@ -1,65 +1,105 @@ // Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. // -// The skills-bundle install command, in ONE place — because two callers have +// The skills-catalog install command, in ONE place — because two callers have // to agree about it: the scaffolder RUNS it, and the closing summary PRINTS // the same command with a different agent name for the runtimes this run did // not install for. A drifted pair would tell the user to run something other // than what produced their project. // -// ## Why this names ONE agent instead of `--all` +// ## Where the catalog comes from: the installed package, never GitHub // -// `--all` is the skills CLI's shorthand for `--skill '*' --agent '*' -y`, and -// the `--agent '*'` half is the whole defect. Measured against `skills@1.5.23` -// with this catalog (11 skills), in an empty directory: +// The catalog ships as `@objectstack/skills`, in the Changesets `fixed` group, +// so its version is always the version of `@objectstack/spec` (ruling A on the +// skills-catalog binding, record 6096124407). The template depends on it, the +// scaffold-time range rewrite (`pkg-utils.ts`) gives it the same `^` +// as every other `@objectstack/*` dependency, and ` install` puts it in +// `node_modules`. The catalog a project gets is therefore the one written for +// the spec it runs — the repository's `main` branch, which already teaches the +// next major, is read by nothing here. // -// npx skills add --all -// .agents/ 46 real files 604,102 B (the "universal" runtime dir) -// agent/ 46 real files 602,682 B (a second real copy, re-serialized -// frontmatter — same bodies) -// .claude/ 11 symlinks -> ../../.agents/skills/ +// The skills CLI's `experimental_sync` reads that package where it lies. +// Measured with `skills@1.7.2` against the PUBLISHED +// `@objectstack/skills@18.0.0-next.0` (10 skills under +// `dist/skills//SKILL.md`), in a throwaway project, npm and pnpm +// layouts alike: // -// npx skills add --skill '*' --agent claude-code -y -// .claude/ 46 real files 604,102 B and NOTHING else +// experimental_sync --agent claude-code -y +// .agents/skills/ 10 symlinks -> node_modules/@objectstack/skills/… +// .claude/skills/ 10 symlinks -> ../../.agents/skills/ +// 0 real SKILL.md outside node_modules // -// The template's `.gitignore` excluded none of it, so a scaffolded project's -// first `git add -A` staged 22 `SKILL.md` paths — the bundle twice, plus 11 -// symlinks — and that reached the initial commit of a real app before anyone -// noticed. With one agent named it stages 11, once. +// experimental_sync --agent claude-code --copy -y +// .claude/skills/ 10 skills, 64 real files, 0 symlinks +// skills-lock.json every entry `source: @objectstack/skills`, +// `sourceType: node_modules`, the package version +// +// The linked shape is unusable for a project that commits its skills: every +// link points into `node_modules`, which the template's `.gitignore` excludes, +// so a fresh clone holds dangling links (and a `core.symlinks=false` clone, +// git-for-Windows' default, holds files whose whole content is the link +// target). `--copy` is the one-agent shape below, from a versioned source. +// +// `experimental_sync` scans the project's DIRECT dependencies (every +// dependency field of `package.json`) for `skills/` or `dist/skills/`. Measured +// over the template's direct dependency set at 18.0.0-next.0, `@objectstack/skills` +// is the only one that ships any, so the synced set is exactly its catalog. +// With no package to read it prints "Nothing to sync." and exits 0 — which is +// why the scaffolder checks both sides itself (`listSkills` below) instead of +// reading the exit code as "installed". It also exits 0 when it SKIPS a skill +// whose directory is already there and was not put there by a sync — one from +// `skills add`, say ("Skipped objectstack-data from @objectstack/skills: +// installed with `skills add` …"), leaving that copy and its lock entry as +// they were. So a directory being present proves nothing either; the lock +// entry naming the package (`skillsSyncedFromPackage`) does. +// +// The CLI is pinned to the version measured. `experimental_` is that package's +// own word for an interface it may change, and an unpinned `npx skills` would +// move the destination layout under every scaffold at the CLI's next release, +// silently. Re-measure before moving the pin. // -// ## Why the fix is the COMMAND and not a `.gitignore` denylist +// ## Why this names ONE agent instead of `--agent '*'` // -// Both denylist shapes were built and cloned, not reasoned about: +// Measured on `skills add` with `skills@1.5.23` and this catalog (11 skills +// then), in an empty directory: // -// * ignore `.agents/` + `agent/`, commit `.claude/` — a fresh clone gets 11 -// DANGLING symlinks and zero readable `SKILL.md`. The links point into the -// tree that was just excluded. -// * ignore `agent/` only, commit `.agents/` (real) + `.claude/` (symlinks) — -// works on POSIX, but the 11 committed symlinks come out of a -// `core.symlinks=false` clone (git-for-Windows' default) as regular files -// whose entire content is the string `../../.agents/skills/`. +// --all (= --skill '*' --agent '*' -y) +// .agents/ 46 real files 604,102 B (the "universal" runtime dir) +// agent/ 46 real files 602,682 B (a second real copy) +// .claude/ 11 symlinks -> ../../.agents/skills/ // -// and `--all --copy`, the other way to make `.claude/` real, fans out to 56 -// destination directories totalling 33.8 MB. +// --skill '*' --agent claude-code -y +// .claude/ 46 real files 604,102 B and NOTHING else // -// A denylist is also the wrong SHAPE regardless of which paths it names: this -// package does not choose the destination set — the skills CLI does, and it -// moves with THAT package's releases (`created-summary.ts` documents the same -// property for the same reason). An ignore list has to chase it, silently, in -// the direction that re-commits duplicates. Naming our own destination is the -// composition this repo asks for: explicit over default magic. +// and the sync repeats it, measured as above (`skills@1.7.2`, the published +// 18.0.0-next.0 catalog): `experimental_sync --agent '*' --copy -y` writes the +// catalog into 54 agent directories, 540 `SKILL.md` copies, 48 MB. +// +// The template's `.gitignore` excluded none of it, so a scaffolded project's +// first `git add -A` staged the bundle twice plus the links. Both denylist +// shapes were built and cloned rather than reasoned about — ignoring the real +// trees leaves DANGLING links in a clone, committing the links breaks on +// `core.symlinks=false` — and a denylist chases a destination set this package +// does not choose. Naming our own destination is the composition this repo +// asks for: explicit over default magic. // // The cost is the multi-runtime default, and it is paid in the open — the // closing summary prints the one-line command for any other runtime, and the -// bundle it installs is byte-identical whichever agent is named. +// catalog it installs is byte-identical whichever agent is named. + +import fs from 'node:fs'; +import path from 'node:path'; + +/** The published catalog package. Its version is `@objectstack/spec`'s (fixed group). */ +export const SKILLS_PACKAGE = '@objectstack/skills'; /** - * The curated, customer-published catalog. The `/skills` subpath is a hard - * boundary, not a convenience: discovery from the repo ROOT also walks - * `.claude/skills/`, and `--skill '*'` includes `metadata.internal` entries — - * a root-scoped install leaks repo-internal playbooks into customer projects. - * `template-consistency.test.ts` holds every surface in this package to it. + * Where the package keeps its catalog, relative to the package root — the + * layout `@objectstack/skills` publishes and `experimental_sync` reads. */ -export const SKILLS_CATALOG = 'objectstack-ai/objectstack/skills'; +export const SKILLS_PACKAGE_CATALOG_DIR = 'dist/skills'; + +/** The skills CLI, pinned to the version the layout above was measured with. */ +export const SKILLS_CLI = 'skills@1.7.2'; /** The single agent runtime a scaffolded project gets its skills wired for. */ export const DEFAULT_SKILLS_AGENT = 'claude-code'; @@ -71,22 +111,81 @@ export const DEFAULT_SKILLS_DIR = '.claude/skills/'; export const EXAMPLE_OTHER_AGENT = 'codex'; /** - * The `skills add …` argument vector for one agent, as a shell string. - * - * `'*'` is quoted because this is handed to a shell: unquoted, the glob is - * expanded against the project directory before the CLI ever sees it. + * The `skills experimental_sync …` argument vector for one agent, as a shell + * string: copy (never link) the catalog of every installed dependency that + * ships one into `agent`'s own directory, without prompting. */ -export function skillsAddArgs(agent: string): string { - return `skills add ${SKILLS_CATALOG} --skill '*' --agent ${agent} -y`; +export function skillsSyncArgs(agent: string): string { + return `${SKILLS_CLI} experimental_sync --agent ${agent} --copy -y`; } /** - * What the scaffolder RUNS. `npx -y` auto-installs the `skills` CLI itself; - * the trailing `-y` is the CLI's own "skip confirmation prompts". + * What the scaffolder RUNS, after ` install`. `npx -y` auto-installs the + * pinned `skills` CLI itself; the trailing `-y` is the CLI's own "skip + * confirmation prompts". */ -export const SKILLS_INSTALL_COMMAND = `npx -y ${skillsAddArgs(DEFAULT_SKILLS_AGENT)}`; +export const SKILLS_INSTALL_COMMAND = `npx -y ${skillsSyncArgs(DEFAULT_SKILLS_AGENT)}`; /** What we PRINT for a user to run later, for `agent`. */ export function skillsInstallHint(agent: string): string { - return `npx ${skillsAddArgs(agent)}`; + return `npx ${skillsSyncArgs(agent)}`; +} + +/** The installed package's root inside a project, whether or not it is there. */ +export function skillsPackageDir(projectDir: string): string { + return path.join(projectDir, 'node_modules', ...SKILLS_PACKAGE.split('/')); +} + +/** The installed package's version, or `null` when `node_modules` does not hold it. */ +export function installedSkillsVersion(projectDir: string): string | null { + try { + const pkg = JSON.parse( + fs.readFileSync(path.join(skillsPackageDir(projectDir), 'package.json'), 'utf8'), + ); + return typeof pkg.version === 'string' ? pkg.version : null; + } catch { + return null; + } +} + +/** The skills CLI's record of what it installed, at the project root. */ +export const SKILLS_LOCK_FILE = 'skills-lock.json'; + +/** + * The skills the project's `skills-lock.json` records as synced from the + * installed `@objectstack/skills` package (`source` the package, + * `sourceType: node_modules`). Empty when there is no readable lock. + */ +export function skillsSyncedFromPackage(projectDir: string): Set { + try { + const lock = JSON.parse(fs.readFileSync(path.join(projectDir, SKILLS_LOCK_FILE), 'utf8')); + return new Set( + Object.entries((lock?.skills ?? {}) as Record) + .filter(([, entry]) => entry?.source === SKILLS_PACKAGE && entry?.sourceType === 'node_modules') + .map(([name]) => name), + ); + } catch { + return new Set(); + } +} + +/** + * The skills under `dir` — the names of its immediate subdirectories that + * hold a `SKILL.md` — sorted. Empty when `dir` does not exist. A symlinked + * entry is not counted: the scaffolder installs real files, and a link into + * `node_modules` is exactly what a clone of the project would not have. One + * reading for both sides of the comparison the scaffolder makes after the + * sync: the package's catalog and what landed in the agent's directory. + */ +export function listSkills(dir: string): string[] { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return []; + } + return entries + .filter((e) => e.isDirectory() && fs.existsSync(path.join(dir, e.name, 'SKILL.md'))) + .map((e) => e.name) + .sort(); } diff --git a/packages/create-objectstack/src/starter-comments-self-contained.test.ts b/packages/create-objectstack/src/starter-comments-self-contained.test.ts index 082ae4d4c23..19bc0607e7e 100644 --- a/packages/create-objectstack/src/starter-comments-self-contained.test.ts +++ b/packages/create-objectstack/src/starter-comments-self-contained.test.ts @@ -227,7 +227,7 @@ describe('shipped template comments are followable by a stranger', () => { file: 'blank/README.md', facts: [ { what: 'that a provider-bound connector is materialized into a live, dispatchable connector at boot (not written by hand)', re: /materializ\w*[^.]*?\bat\s+boot\b/i }, - { what: 'the followable skills install command (`npx skills add objectstack-ai/objectstack/skills`)', re: /npx skills add objectstack-ai\/objectstack\/skills/i }, + { what: 'the followable skills install command (`npx skills@ experimental_sync … --copy`, reading the installed package)', re: /npx skills@\d+\.\d+\.\d+ experimental_sync --agent \S+ --copy/i }, ], }, ]; diff --git a/packages/create-objectstack/src/template-consistency.test.ts b/packages/create-objectstack/src/template-consistency.test.ts index d32c819cf82..8204eebf835 100644 --- a/packages/create-objectstack/src/template-consistency.test.ts +++ b/packages/create-objectstack/src/template-consistency.test.ts @@ -15,7 +15,12 @@ import { syncObjectStackDeps } from './pkg-utils.js'; import { copyDir, TEMPLATE_FILE_ALIASES } from './template-copy.js'; import { TEMPLATES } from './template-registry.js'; import { gitFreeEnv } from '../../../scripts/git-env.mjs'; -import { SKILLS_CATALOG, SKILLS_INSTALL_COMMAND } from './skills-install.js'; +import { + DEFAULT_SKILLS_AGENT, + SKILLS_INSTALL_COMMAND, + SKILLS_PACKAGE, + skillsInstallHint, +} from './skills-install.js'; const pkgRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const repoRoot = path.resolve(pkgRoot, '..', '..'); @@ -608,10 +613,12 @@ describe('README template table', () => { // also walks `.claude/skills/`. The published catalog is exactly the root // `skills/` directory; everything else must stay repo-internal. // -// The boundary is the `/skills` SUBPATH, and it did not move when the -// scaffolder stopped passing `--all` — `--skill '*'` still selects every -// entry discovery finds, `metadata.internal` ones included, so a root-scoped -// install would leak them exactly as before. +// The scaffolder no longer reads GitHub at all: it syncs the catalog out of +// the installed `@objectstack/skills` package, whose build copies `skills/**` +// and nothing else (`skills-install.ts`). The `/skills` SUBPATH stays the +// boundary for the GitHub path the docs keep as the `next` channel — +// `--skill '*'` selects every entry discovery finds, `metadata.internal` ones +// included, so a root-scoped install would leak them exactly as before. describe('skills catalog boundary', () => { const frontmatterOf = (file: string): string => /^---\n([\s\S]*?)\n---/.exec(fs.readFileSync(file, 'utf8'))?.[1] ?? ''; @@ -660,17 +667,16 @@ describe('skills catalog boundary', () => { // text of the module that happens to hold it today: a `toContain` on source // text goes vacuous the moment the literal is assembled from parts, and a // vacuous half of this pair is the half that guards the leak. - it('scaffolder installs from the curated skills/ subpath, not the repo root', () => { - expect(SKILLS_CATALOG).toBe('objectstack-ai/objectstack/skills'); - expect(SKILLS_INSTALL_COMMAND).toContain(`skills add ${SKILLS_CATALOG} `); - expect(SKILLS_INSTALL_COMMAND).not.toMatch( - /skills add objectstack-ai\/objectstack(?!\/skills)/, - ); + it('scaffolder installs from the @objectstack/skills package, never a GitHub source', () => { + expect(SKILLS_PACKAGE).toBe('@objectstack/skills'); + expect(SKILLS_INSTALL_COMMAND).toMatch(/ experimental_sync /); + expect(SKILLS_INSTALL_COMMAND).not.toMatch(/skills add|objectstack-ai\/objectstack/); }); // The source-text half survives as its own case, aimed at a DIFFERENT - // failure: a second, hand-written `skills add` invocation added to the - // scaffolder later, bypassing the shared constant above. + // failure: a second, hand-written GitHub `skills add` invocation added to + // the scaffolder later, bypassing the shared constant above — subpath or + // root, either one reads `main` instead of the installed package. it('the scaffolder hard-codes no second skills invocation', () => { const SKILLS_SOURCE = fs.readFileSync( path.join(pkgRoot, 'src', 'skills-install.ts'), @@ -682,15 +688,33 @@ describe('skills catalog boundary', () => { ] as const) { expect( src, - `${name} spells a repo-root skills install — discovery from the root ` + - 'also walks .claude/skills/, so internal skills would ship.', - ).not.toMatch(/skills add objectstack-ai\/objectstack(?!\/skills)/); + `${name} spells a GitHub skills install — the catalog is read from the ` + + 'installed @objectstack/skills package, at the project\'s own version.', + ).not.toMatch(/skills add objectstack-ai\/objectstack/); } // Exactly one place builds the command; index.ts imports it. expect(REGISTRY_SOURCE).toContain("from './skills-install.js'"); expect(REGISTRY_SOURCE).not.toMatch(/execSync\(\s*['\`"]npx/); }); + // The files the scaffolder WRITES into a project tell its agents how to + // re-install the skills, and this package's README says what it runs; each + // must name the command the scaffolder really runs, pin included, and never + // the GitHub path a released project must not read. + it('the scaffolded guides and the README name the command the scaffolder runs', () => { + for (const [rel, command] of [ + ['src/templates/AGENTS.md', skillsInstallHint(DEFAULT_SKILLS_AGENT)], + ['src/templates/blank/README.md', skillsInstallHint(DEFAULT_SKILLS_AGENT)], + ['README.md', SKILLS_INSTALL_COMMAND], + ] as const) { + const text = fs.readFileSync(path.join(pkgRoot, rel), 'utf8'); + expect(text, `${rel} no longer names the scaffolder's own command`).toContain(command); + expect(text, `${rel} tells a released project to read GitHub main`).not.toMatch( + /skills add objectstack-ai\/objectstack/, + ); + } + }); + // The /skills subpath is the hard boundary: `--skill '*'` (which `--all` // also implies) INCLUDES metadata.internal skills — so any customer-facing // surface advertising a repo-root install would leak internal skills again. diff --git a/packages/create-objectstack/src/templates/AGENTS.md b/packages/create-objectstack/src/templates/AGENTS.md index 944cb0e4720..f9be8f56ab7 100644 --- a/packages/create-objectstack/src/templates/AGENTS.md +++ b/packages/create-objectstack/src/templates/AGENTS.md @@ -77,18 +77,23 @@ Run `npm run build` when you need the compiled `dist/objectstack.json` artifact. ## AI Skills -This project uses ObjectStack skills from `objectstack-ai/objectstack`. They -were installed for one agent runtime, live in `.claude/skills/`, and are -committed with the project. Install or update them with the standard +This project uses the ObjectStack skills catalog from the `@objectstack/skills` +dev dependency, which is released together with `@objectstack/spec` — so the +skills teach the version this project runs. They were installed for one agent +runtime, live in `.claude/skills/`, and are committed with the project. After +upgrading the `@objectstack/*` packages (`@objectstack/skills` moves with +them), re-install them from `node_modules` with the [skills CLI](https://skills.sh/), naming your runtime: ```bash -npx skills add objectstack-ai/objectstack/skills --skill '*' --agent claude-code -y +npx skills@1.7.2 experimental_sync --agent claude-code --copy -y ``` Adding a second runtime? Re-run it with that runtime's `--agent` name. -⛔ Do not use `--all`: it writes the same bundle to `.agents/`, `agent/` and -`.claude/` at once, and all three then land in your next commit. +⛔ Keep `--copy`: without it the skills are symlinks into `node_modules`, which +is not committed, so a fresh clone of the project has none. +⛔ Do not use `--agent '*'`: it copies the catalog into dozens of agent +directories at once, and all of them then land in your next commit. Skills are triggered automatically based on task context: diff --git a/packages/create-objectstack/src/templates/blank/README.md b/packages/create-objectstack/src/templates/blank/README.md index a2cacfa1321..8e38bad2a70 100644 --- a/packages/create-objectstack/src/templates/blank/README.md +++ b/packages/create-objectstack/src/templates/blank/README.md @@ -160,9 +160,11 @@ covered in [Self-Hosted Deployment](https://objectstack.ai/docs/deployment/self- - Add an AI agent: see `objectstack-ai`. Skills live in `.claude/skills/` and are committed with this project, once. -They are installed one agent runtime at a time (see `AGENTS.md`), and also show -up in the in-IDE assistant catalog: +They come from the `@objectstack/skills` dev dependency, at the version of +`@objectstack/spec`, are installed one agent runtime at a time (see +`AGENTS.md`), and also show up in the in-IDE assistant catalog. Re-install them +from `node_modules` after upgrading the `@objectstack/*` packages: ```bash -npx skills add objectstack-ai/objectstack/skills --skill '*' --agent claude-code -y +npx skills@1.7.2 experimental_sync --agent claude-code --copy -y ``` diff --git a/packages/create-objectstack/src/templates/blank/package.json b/packages/create-objectstack/src/templates/blank/package.json index 525b315283b..c1862cd7199 100644 --- a/packages/create-objectstack/src/templates/blank/package.json +++ b/packages/create-objectstack/src/templates/blank/package.json @@ -25,6 +25,7 @@ }, "devDependencies": { "@objectstack/cli": "^18.0.0", + "@objectstack/skills": "^18.0.0", "@objectstack/verify": "^18.0.0", "typescript": "^5.3.0", "vitest": "~4.0.0"