diff --git a/.changeset/scaffold-skills-single-committed-copy.md b/.changeset/scaffold-skills-single-committed-copy.md new file mode 100644 index 0000000000..5d3b6113a7 --- /dev/null +++ b/.changeset/scaffold-skills-single-committed-copy.md @@ -0,0 +1,44 @@ +--- +"create-objectstack": minor +--- + +Scaffolded projects now install the AI skills bundle for **one** agent runtime +instead of every runtime the skills CLI knows, so the bundle is committed once. + +**Route B of the two the card offered was taken**, and the choice was measured +rather than argued. Against `skills@1.5.23` and the 11-skill catalog, the old +`--all` (shorthand for `--skill '*' --agent '*' -y`) wrote the same bundle to +three destinations — `.agents/` (46 real files, 604,102 B), `agent/` (46 real +files, 602,682 B, identical bodies with re-serialised frontmatter) and +`.claude/` (11 symlinks into `.agents/`). The template's `.gitignore` excluded +none of it, so a new project's first `git add -A` staged 22 `SKILL.md` paths +plus 11 symlinks. That reached the initial commit of a real app before anyone +noticed. + +The scaffolder now runs +`npx skills add objectstack-ai/objectstack/skills --skill '*' --agent claude-code -y`, +which writes 46 real files to `.claude/skills/` and nothing else: 11 staged +`SKILL.md` paths, no symlinks, and a clone of that commit has readable skill +files on every platform. + +Route A (keep `--all`, exclude the duplicates in the template `_gitignore`) was +built and cloned, not reasoned about, and both of its shapes were rejected. +Ignoring `.agents/` and `agent/` while committing `.claude/` gives a fresh +cloner 11 dangling symlinks and zero readable `SKILL.md`. Ignoring only +`agent/` works on POSIX but commits 11 symlinks that a `core.symlinks=false` +clone — git-for-Windows' default — materialises as ordinary files whose whole +content is the link target. `--all --copy`, the other way to make `.claude/` +real, fans out to 56 destination directories totalling 33.8 MB. 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. + +The cost is the multi-runtime default, and it is paid in the open: the closing +summary now always prints an **AI Skills** block naming where the bundle landed +and the one-line command for any other runtime, one agent at a time. The +bundle is identical whichever agent is named. + +Existing projects are unaffected. To shrink one that already carries the +triplicate, delete `.agents/` and `agent/` and re-run the single-agent command +above; `skills-lock.json` records source and hash, not paths, so it does not +change. diff --git a/packages/create-objectstack/README.md b/packages/create-objectstack/README.md index 48e80cc204..30683ea1c1 100644 --- a/packages/create-objectstack/README.md +++ b/packages/create-objectstack/README.md @@ -56,9 +56,12 @@ you so rather than failing as an unknown template. 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 coding agents - (`npx skills add objectstack-ai/objectstack/skills --all` — scoped to the - curated `skills/` catalog). +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. 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.ts b/packages/create-objectstack/src/created-summary.ts index eec164d6b0..0f98ab74dc 100644 --- a/packages/create-objectstack/src/created-summary.ts +++ b/packages/create-objectstack/src/created-summary.ts @@ -28,15 +28,18 @@ // // 1. template copy + identity rewrite + AGENTS.md/copilot-instructions.md // 2. ` install` -> pnpm-lock.yaml, node_modules/ -// 3. `npx skills add … --all` -> .agents/, agent/, .claude/, -// skills-lock.json +// 3. `npx skills add … --agent …` -> .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 — the `skills` CLI fans out to every agent runtime it knows, -// and that set changes with ITS releases, not ours. 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 +// 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 +// 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. // // Reading the directory afterwards is what makes the summary self-correcting. diff --git a/packages/create-objectstack/src/index.ts b/packages/create-objectstack/src/index.ts index b7eec54823..1f4ac6cb9c 100644 --- a/packages/create-objectstack/src/index.ts +++ b/packages/create-objectstack/src/index.ts @@ -39,11 +39,12 @@ * `--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 via - * `npx skills add objectstack-ai/objectstack/skills --all`. - * The `/skills` subpath scopes discovery to the curated, customer-published - * catalog — repo-internal skills (e.g. under `.claude/skills/`) must never - * reach scaffolded projects. + * 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. * * 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 @@ -72,6 +73,13 @@ import { import { lookupTemplate, templateNames } from './template-registry.js'; import { readResolvedCliVersion, pinRuntimeImage } from './runtime-image.js'; import { summarizeTree, describeEntry } from './created-summary.js'; +import { + DEFAULT_SKILLS_AGENT, + DEFAULT_SKILLS_DIR, + EXAMPLE_OTHER_AGENT, + SKILLS_INSTALL_COMMAND, + skillsInstallHint, +} from './skills-install.js'; import { renderVersionBanner } from './banner.js'; const __filename = fileURLToPath(import.meta.url); @@ -521,24 +529,26 @@ const program = new Command() } // Which top-level paths belong to the skills install is measured, not - // assumed: `skills add --all` fans the catalog out to every agent - // runtime IT knows about (77 at the version measured), so the - // destination set moves with that package's releases. Diffing the - // directory across the call keeps the ⚠ marks correct without this file - // carrying a list it cannot keep current. + // assumed. Naming one agent makes the destination set small and + // knowable (`skills-install.ts`), but it is still THEIR path to choose, + // and it moves with that package's releases — so the ⚠ marks come from + // 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('npx -y skills add objectstack-ai/objectstack/skills --all', { + execSync(SKILLS_INSTALL_COMMAND, { stdio: 'inherit', cwd: targetDir, }); + skillsInstalled = true; console.log(''); } catch { printWarning( 'Skills installation skipped. Run manually:\n' + - ' npx skills add objectstack-ai/objectstack/skills', + ` ${skillsInstallHint(DEFAULT_SKILLS_AGENT)}`, ); console.log(''); } @@ -569,10 +579,29 @@ const program = new Command() console.log(chalk.dim(` ${devLabel.padEnd(labelWidth)}# Start development server`)); console.log(chalk.dim(` ${validateLabel.padEnd(labelWidth)}# Verify metadata: schema + predicates + bindings`)); console.log(chalk.dim(` ${' '.repeat(labelWidth)}# (run after every metadata edit — see AGENTS.md)`)); - if (options.skipInstall || options.skipSkills) { - console.log(''); + // The skills block is printed on EVERY path, not only the skipped one. + // The install now targets a single agent runtime, so a run that + // 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. + console.log(''); + if (skillsInstalled) { + console.log(chalk.bold(' AI Skills:')); + console.log( + chalk.dim( + ` Installed for ${DEFAULT_SKILLS_AGENT} → ${DEFAULT_SKILLS_DIR} — commit it with your project.`, + ), + ); + 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)')); + } else { console.log(chalk.bold(' AI Skills (recommended):')); - console.log(chalk.dim(' npx skills add objectstack-ai/objectstack/skills')); + console.log(chalk.dim(` ${skillsInstallHint(DEFAULT_SKILLS_AGENT)}`)); + console.log( + chalk.dim(` Replace \`${DEFAULT_SKILLS_AGENT}\` with your own agent runtime if it differs.`), + ); } console.log(''); } catch (error) { diff --git a/packages/create-objectstack/src/scaffold-skills-single-copy.test.ts b/packages/create-objectstack/src/scaffold-skills-single-copy.test.ts new file mode 100644 index 0000000000..fd91653aaa --- /dev/null +++ b/packages/create-objectstack/src/scaffold-skills-single-copy.test.ts @@ -0,0 +1,357 @@ +// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. +// +// #16331 — 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 measurement this file encodes +// +// Taken against `skills@1.5.23` and the published `objectstack-ai/objectstack/ +// skills` catalog (11 skills), each into an empty directory: +// +// --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 +// +// and then, with the template's `.gitignore` in place, `git init && git add -A`: +// +// --all shape 22 staged SKILL.md paths + 11 staged symlinks +// claude-code shape 11 staged SKILL.md paths, 0 symlinks +// +// 22 is the defect: the same eleven skills, staged twice as real files. +// +// ## Why the skills CLI is stubbed, and how the stub is kept honest +// +// 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 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 end-to-end case runs 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. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + DEFAULT_SKILLS_AGENT, + DEFAULT_SKILLS_DIR, + EXAMPLE_OTHER_AGENT, + SKILLS_CATALOG, + SKILLS_INSTALL_COMMAND, + skillsAddArgs, + skillsInstallHint, +} from './skills-install.js'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +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', +); + +/** The skills the stub installs. Three, so "twice" is unmistakably six. */ +const STUB_SKILLS = ['objectstack-data', 'objectstack-query', 'objectstack-ui']; + +/** The measured destination map — see the header table. */ +const INSTALL_SHAPES = ` +const SHAPES = { + 'agent:*': [ + { dir: '.agents/skills', kind: 'real' }, + { dir: 'agent/skills', kind: 'real' }, + { 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); + 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; + } + 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); +} + +interface Staged { + skillMd: string[]; + symlinks: string[]; + all: string[]; +} + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', ['-c', 'user.email=t@example.com', '-c', 'user.name=t', ...args], { + cwd, + encoding: 'utf8', + }); +} + +/** `git init && git add -A`, then what the index holds. */ +function stageAll(project: string): Staged { + git(project, 'init', '-q', '.'); + git(project, 'add', '-A'); + const all = git(project, 'diff', '--cached', '--name-only').split('\n').filter(Boolean); + const symlinks = git(project, 'ls-files', '-s') + .split('\n') + .filter((l) => l.startsWith('120000')) + .map((l) => l.split('\t')[1]); + return { all, symlinks, skillMd: all.filter((p) => p.endsWith('SKILL.md')) }; +} + +/** Commit, clone into a fresh directory, and report what a cloner can read. */ +function cloneAndRead(project: string, into: string) { + git(project, 'commit', '-qm', 'initial commit'); + execFileSync('git', ['clone', '-q', project, into], { encoding: 'utf8' }); + const tracked = git(into, 'ls-files').split('\n').filter(Boolean); + const skillMd = tracked.filter((p) => p.endsWith('SKILL.md')); + const readable = skillMd.filter((p) => { + const abs = path.join(into, p); + return fs.existsSync(abs) && fs.statSync(abs).size > 0; + }); + const dangling = tracked.filter((p) => { + const abs = path.join(into, p); + return fs.lstatSync(abs).isSymbolicLink() && !fs.existsSync(abs); + }); + return { tracked, skillMd, readable, dangling }; +} + +let TMP: string; +let BIN: string; +let RECEIPT: string; + +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); +}); + +afterAll(() => { + fs.rmSync(TMP, { recursive: true, force: true }); +}); + +// ── 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+'?\*/); + }); + + it("selects every skill, with the glob quoted against the shell", () => { + expect(SKILLS_INSTALL_COMMAND).toContain("--skill '*'"); + }); + + it('names one agent, once', () => { + const agents = SKILLS_INSTALL_COMMAND.match(/--agent\s+(\S+)/g) ?? []; + expect(agents).toEqual([`--agent ${DEFAULT_SKILLS_AGENT}`]); + }); + + it('installs from the curated catalog subpath', () => { + expect(SKILLS_INSTALL_COMMAND).toContain(`skills add ${SKILLS_CATALOG} `); + }); + + // 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. + 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 '), + ); + expect(skillsInstallHint(EXAMPLE_OTHER_AGENT)).toBe( + skillsInstallHint(DEFAULT_SKILLS_AGENT).replace( + `--agent ${DEFAULT_SKILLS_AGENT}`, + `--agent ${EXAMPLE_OTHER_AGENT}`, + ), + ); + expect(skillsAddArgs(EXAMPLE_OTHER_AGENT)).toContain(`--agent ${EXAMPLE_OTHER_AGENT}`); + }); +}); + +// ── 2. end to end: a real scaffold, staged and cloned ─────────────────────── +describe('a scaffolded project stages the bundle exactly once', () => { + let project: string; + 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); + }); + + // 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'); + }); + + it('writes the bundle to exactly one directory', () => { + const roots = new Set(staged.skillMd.map((p) => p.split('/')[0])); + expect([...roots]).toEqual([DEFAULT_SKILLS_DIR.split('/')[0]]); + }); + + it('stages one SKILL.md per skill — not two, not three', () => { + expect(staged.skillMd).toHaveLength(STUB_SKILLS.length); + }); + + it('stages no symlinks at all', () => { + expect(staged.symlinks).toEqual([]); + }); + + it('stages the lockfile that records what was installed', () => { + expect(staged.all).toContain('skills-lock.json'); + }); + + 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); + expect(clone.readable).toEqual(clone.skillMd); + expect(clone.dangling).toEqual([]); + }); +}); + +// ── 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 }); + fs.writeFileSync( + path.join(project, '.gitignore'), + fs.readFileSync(TEMPLATE_GITIGNORE, 'utf8') + + extraIgnores.map((l) => `${l}\n`).join(''), + ); + execFileSync(path.join(BIN, 'npx'), ['-y', 'skills', 'add', SKILLS_CATALOG, '--all'], { + cwd: project, + encoding: 'utf8', + }); + } + + 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 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/']); + const staged = stageAll(project); + expect(staged.skillMd).toEqual([]); + expect(staged.symlinks).toHaveLength(STUB_SKILLS.length); + + const clone = cloneAndRead(project, path.join(TMP, 'clone-all-ignored')); + expect(clone.dangling).toHaveLength(STUB_SKILLS.length); + expect(clone.skillMd).toEqual([]); + }); + + it('refuses an argv it has no measured shape for', () => { + const project = path.join(TMP, 'unmeasured'); + 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; + } + expect(code, 'the stub agreed with an install shape nobody measured').toBe(3); + }); +}); diff --git a/packages/create-objectstack/src/skills-install.ts b/packages/create-objectstack/src/skills-install.ts new file mode 100644 index 0000000000..f10ff5de7e --- /dev/null +++ b/packages/create-objectstack/src/skills-install.ts @@ -0,0 +1,92 @@ +// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. +// +// The skills-bundle 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` +// +// `--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: +// +// 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/ +// +// npx skills add --skill '*' --agent claude-code -y +// .claude/ 46 real files 604,102 B and NOTHING else +// +// 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. +// +// ## Why the fix is the COMMAND and not a `.gitignore` denylist +// +// Both denylist shapes were built and cloned, not reasoned about: +// +// * 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/`. +// +// and `--all --copy`, the other way to make `.claude/` real, fans out to 56 +// destination directories totalling 33.8 MB. +// +// 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. +// +// 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. + +/** + * 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. + */ +export const SKILLS_CATALOG = 'objectstack-ai/objectstack/skills'; + +/** The single agent runtime a scaffolded project gets its skills wired for. */ +export const DEFAULT_SKILLS_AGENT = 'claude-code'; + +/** Where `DEFAULT_SKILLS_AGENT` lands them — real files, no symlinks. */ +export const DEFAULT_SKILLS_DIR = '.claude/skills/'; + +/** An agent name to show as the example in the "other runtimes" hint. */ +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. + */ +export function skillsAddArgs(agent: string): string { + return `skills add ${SKILLS_CATALOG} --skill '*' --agent ${agent} -y`; +} + +/** + * What the scaffolder RUNS. `npx -y` auto-installs the `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)}`; + +/** What we PRINT for a user to run later, for `agent`. */ +export function skillsInstallHint(agent: string): string { + return `npx ${skillsAddArgs(agent)}`; +} diff --git a/packages/create-objectstack/src/template-consistency.test.ts b/packages/create-objectstack/src/template-consistency.test.ts index c6fcd68e3e..fe0df2a334 100644 --- a/packages/create-objectstack/src/template-consistency.test.ts +++ b/packages/create-objectstack/src/template-consistency.test.ts @@ -14,6 +14,7 @@ import { fileURLToPath } from 'node:url'; import { syncObjectStackDeps } from './pkg-utils.js'; import { copyDir, TEMPLATE_FILE_ALIASES } from './template-copy.js'; import { TEMPLATES } from './template-registry.js'; +import { SKILLS_CATALOG, SKILLS_INSTALL_COMMAND } from './skills-install.js'; const pkgRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const repoRoot = path.resolve(pkgRoot, '..', '..'); @@ -586,9 +587,14 @@ describe('README template table', () => { // Skills catalog boundary (15.1 third-party eval): scaffolded projects once // received the repo-internal `dogfood-verification` skill because the -// scaffolder installed with a repo-wide `skills add … --all`, whose discovery +// scaffolder installed with a repo-wide `skills add …`, whose discovery // 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. describe('skills catalog boundary', () => { const frontmatterOf = (file: string): string => /^---\n([\s\S]*?)\n---/.exec(fs.readFileSync(file, 'utf8'))?.[1] ?? ''; @@ -633,19 +639,44 @@ describe('skills catalog boundary', () => { } }); + // Asserted against the VALUE the scaffolder runs, not against the source + // 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(REGISTRY_SOURCE).toContain( - 'skills add objectstack-ai/objectstack/skills --all', - ); - expect(REGISTRY_SOURCE).not.toMatch( + 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)/, ); }); - // The /skills subpath is the hard boundary: the skills CLI's `--all` - // implies `--skill '*'`, which INCLUDES metadata.internal skills — so any - // customer-facing surface advertising a repo-root install would leak - // internal skills again. + // 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. + it('the scaffolder hard-codes no second skills invocation', () => { + const SKILLS_SOURCE = fs.readFileSync( + path.join(pkgRoot, 'src', 'skills-install.ts'), + 'utf8', + ); + for (const [name, src] of [ + ['index.ts', REGISTRY_SOURCE], + ['skills-install.ts', SKILLS_SOURCE], + ] 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)/); + } + // 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 /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. it('no customer-facing surface advertises a repo-root skills install', () => { const surfaces = [ 'content/docs', diff --git a/packages/create-objectstack/src/templates/AGENTS.md b/packages/create-objectstack/src/templates/AGENTS.md index c4b5ed0aa9..55324f2ecd 100644 --- a/packages/create-objectstack/src/templates/AGENTS.md +++ b/packages/create-objectstack/src/templates/AGENTS.md @@ -73,13 +73,19 @@ Run `npm run build` when you need the compiled `dist/objectstack.json` artifact. ## AI Skills -This project uses ObjectStack skills from `objectstack-ai/objectstack`. -Install or update skills with the standard [skills CLI](https://skills.sh/): +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 +[skills CLI](https://skills.sh/), naming your runtime: ```bash -npx skills add objectstack-ai/objectstack/skills +npx skills add objectstack-ai/objectstack/skills --skill '*' --agent claude-code -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. + Skills are triggered automatically based on task context: | Skill | Trigger Context | diff --git a/packages/create-objectstack/src/templates/blank/README.md b/packages/create-objectstack/src/templates/blank/README.md index d8486405be..f1dc70ba54 100644 --- a/packages/create-objectstack/src/templates/blank/README.md +++ b/packages/create-objectstack/src/templates/blank/README.md @@ -140,5 +140,10 @@ covered in [Self-Hosted Deployment](https://objectstack.ai/docs/deployment/self- - Add a flow or automation: see `objectstack-automation`. - Add an AI agent: see `objectstack-ai`. -Skills are installed with `npx skills add objectstack-ai/objectstack/skills` -(see `AGENTS.md`) and also show up in the in-IDE assistant catalog. +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: + +```bash +npx skills add objectstack-ai/objectstack/skills --skill '*' --agent claude-code -y +```