diff --git a/.github/workflows/cross-platform-tests.yml b/.github/workflows/cross-platform-tests.yml index 65ce7fba..cdb68494 100644 --- a/.github/workflows/cross-platform-tests.yml +++ b/.github/workflows/cross-platform-tests.yml @@ -74,6 +74,12 @@ jobs: run: bun run build shell: bash + # The skills package copies a directory tree and resolves it back by + # path, so it is exactly the kind of code that breaks on Windows only. + - name: Run skills package tests + run: bun run --cwd packages/skills test + shell: bash + # Split into two bun-test invocations (two processes). The unit specs # under src/ use `mock.module(...)` to stub out `StorageManager` and # other modules; Bun does not scope those mocks to a single file — diff --git a/.github/workflows/skills-npm.yml b/.github/workflows/skills-npm.yml new file mode 100644 index 00000000..f169166b --- /dev/null +++ b/.github/workflows/skills-npm.yml @@ -0,0 +1,112 @@ +name: skills + +# The skills package versions on its own line. release.yml bumps sdk, app, and +# server in lockstep and is deliberately not used here: a skill edit should not +# need a server release, which is the whole point of splitting the package out. + +on: + push: + branches: [main] + paths: + - "skills/**" + - "packages/skills/**" + - ".github/workflows/skills-npm.yml" + pull_request: + paths: + - "skills/**" + - "packages/skills/**" + - ".github/workflows/skills-npm.yml" + workflow_dispatch: + +env: + BUN_VERSION: 1.3.13 + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +jobs: + check_pack: + name: Verify the published tarball + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + + - name: Install Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: ${{ env.BUN_VERSION }} + + - name: Install dependencies + run: bun install --frozen-lockfile + + # A skills/ dir that failed to copy packs as nothing at all, with exit 0 + # and no warning, so this is the only thing standing between a bad refactor + # and publishing an empty package. + - name: Check pack contents + run: bun run --cwd packages/skills check-pack + + - name: Test + run: bun run --cwd packages/skills test + + publish: + name: Publish to NPM + if: github.event_name == 'workflow_dispatch' + needs: check_pack + runs-on: ubuntu-latest + permissions: + contents: read + id-token: write + steps: + - uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "24" + registry-url: "https://registry.npmjs.org" + + - name: Install Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: ${{ env.BUN_VERSION }} + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Read version + id: version_step + run: | + CURRENT_VERSION="$(node -p "require('./packages/skills/package.json').version")" + + # Stable versions publish under "latest"; any pre-release under "next". + if [[ "$CURRENT_VERSION" == *-* ]]; then + NPM_TAG="next" + else + NPM_TAG="latest" + fi + + echo "current_version=$CURRENT_VERSION" >> $GITHUB_OUTPUT + echo "npm_tag=$NPM_TAG" >> $GITHUB_OUTPUT + echo "::notice title=Current Version::Publishing skills version $CURRENT_VERSION under npm dist-tag '$NPM_TAG'" + + # Trusted publishing (OIDC); there is no NPM_TOKEN fallback in this repo. + # + # npm configures trusted publishing per package, on the package's own + # settings page, so the package has to exist before the entry can be + # created. The first publish of a new name is therefore manual, by an + # @malloy-publisher owner. Order: publish once by hand, add the + # trusted-publisher entry naming this workflow file, then every release + # after that is a dispatch here. Run before that entry exists and npm's + # OIDC helper returns undefined rather than raising, so this falls through + # to the empty token and fails with a bare ENEEDAUTH. + # + # Run that manual publish with scripts ENABLED. prepack is what copies + # skills/ into the package, so `npm publish --ignore-scripts`, or an + # ignore-scripts=true in your own npmrc, ships three files and no skills + # at all, without an error. + - name: Publish to NPM + env: + NODE_AUTH_TOKEN: "" + run: | + cd packages/skills + npm publish --access public --tag ${{ steps.version_step.outputs.npm_tag }} diff --git a/.gitignore b/.gitignore index 78f0e31f..2e887c58 100644 --- a/.gitignore +++ b/.gitignore @@ -39,6 +39,7 @@ publisher_data/ # These are local Credible workflow files (an installed skill tree + IDE rules) and are # ignored by the `credible-` prefix so a stray `git add` can't pull them in. .credible/ +skills/credible-*/ .claude/skills/credible-*/ .claude/rules/credible-*.md .cursor/rules/credible-*.mdc diff --git a/bun.lock b/bun.lock index 34f0fa84..0bfc054c 100644 --- a/bun.lock +++ b/bun.lock @@ -1,5 +1,6 @@ { "lockfileVersion": 1, + "configVersion": 0, "workspaces": { "": { "name": "malloy-publisher", @@ -210,6 +211,19 @@ "supertest": "^7.0.0", }, }, + "packages/skills": { + "name": "@malloy-publisher/skills", + "version": "0.1.0", + "devDependencies": { + "@eslint/js": "^8.57.0", + "@types/bun": "^1.2.21", + "@types/node": "^24.10.0", + "@typescript-eslint/eslint-plugin": "^8.16.0", + "@typescript-eslint/parser": "^8.16.0", + "eslint": "^8.50.0", + "typescript": "^5.3.3", + }, + }, }, "overrides": { "@malloydata/malloy-explorer": "0.0.338-dev260215172810", @@ -587,6 +601,8 @@ "@malloy-publisher/server": ["@malloy-publisher/server@workspace:packages/server"], + "@malloy-publisher/skills": ["@malloy-publisher/skills@workspace:packages/skills"], + "@malloydata/db-bigquery": ["@malloydata/db-bigquery@0.0.422", "", { "dependencies": { "@google-cloud/bigquery": "7.9.4", "@google-cloud/common": "5.0.2", "@google-cloud/paginator": "5.0.2", "@malloydata/malloy": "0.0.422", "gaxios": "^4.2.0" } }, "sha512-pltGa1nEhRiL9wvupeDKgdfXAc1DEzS64Dn+Nxwu7vZ2Mn2GgbF4fTQMvaR8biHusc9CTpxaApO+MMcuQGQoHQ=="], "@malloydata/db-databricks": ["@malloydata/db-databricks@0.0.422", "", { "dependencies": { "@databricks/sql": "1.15.0", "@malloydata/malloy": "0.0.422", "@types/node": "^22.20.0" } }, "sha512-uexBL5ECXNmx9DwnfJjol0BL7vJYyfe25CdflwYbPxzJAbh8EVaJYo/+QMoLZk5oIxtwbuhCrS8x1odQubsCqA=="], @@ -3605,6 +3621,10 @@ "@malloy-publisher/server/eslint-plugin-react-hooks": ["eslint-plugin-react-hooks@5.0.0", "", { "peerDependencies": { "eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0" } }, "sha512-hIOwI+5hYGpJEc4uPRmz2ulCjAGD/N13Lukkh8cLV0i2IRk/bdZDYjgLVHj+U9Z704kLIdIO6iueGvxNur0sgw=="], + "@malloy-publisher/skills/@eslint/js": ["@eslint/js@8.57.1", "", {}, "sha512-d9zaMRSTIKDLhctzH12MtXvJKSSUhaHcjV+2Z+GK+EEY7XKpP5yR4x+N3TAcHTcu963nIr+TMcCb4DBCYX1z6Q=="], + + "@malloy-publisher/skills/@types/node": ["@types/node@24.10.1", "", { "dependencies": { "undici-types": "~7.16.0" } }, "sha512-GNWcUTRBgIRJD5zj+Tq0fKOJ5XZajIiBroOF0yvj2bSU1WvNdYS/dn9UxwsujGW4JX06dnHyjV2y9rRaybH0iQ=="], + "@malloydata/db-bigquery/gaxios": ["gaxios@4.3.3", "", { "dependencies": { "abort-controller": "^3.0.0", "extend": "^3.0.2", "https-proxy-agent": "^5.0.0", "is-stream": "^2.0.0", "node-fetch": "^2.6.7" } }, "sha512-gSaYYIO1Y3wUtdfHmjDUZ8LWaxJQpiavzbF5Kq53akSzvmVg0RfyOcFDbO1KJ/KCGRFz2qG+lS81F0nkr7cRJA=="], "@malloydata/db-databricks/@types/node": ["@types/node@22.20.0", "", { "dependencies": { "undici-types": "~6.21.0" } }, "sha512-QWlFW2wf3nTjC13/DqRnBpR4ZO36VJH/JVBkA/vcnmbTBNQIlnObqyqZE1tUR7+Ni23Lda8R1BxMfbXRpCUx5g=="], diff --git a/package.json b/package.json index 09c68275..4aa5de43 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "build:server": "cd packages/server && bun run build", "build": "NODE_ENV=production bun run build:sdk && NODE_ENV=production bun run build:server", "build:cli": "cd packages/cli && bun run build", + "build:skills": "cd packages/skills && bun run build", "build:server-deploy": "NODE_ENV=production bun run build:sdk && NODE_ENV=production bun run build:app:server && NODE_ENV=production bun run build:server", "start": "cd packages/server && bun run start", "start:init": "cd packages/server && bun run start:init", @@ -24,7 +25,8 @@ "start:dev:init": "cd packages/server && bun run start:dev:init", "start:dev:react": "cd packages/app && bun run dev", "test:server": "cd packages/server && NODE_ENV=test bun run test", - "test": "NODE_ENV=test bun run test:server", + "test:skills": "cd packages/skills && bun run test", + "test": "NODE_ENV=test bun run test:server && bun run test:skills", "generate-api-types:sdk": "cd packages/sdk && bun run generate-api-types", "generate-api-types:server": "cd packages/server && bun run generate-api-types", "generate-api-types": "bun run generate-api-types:sdk && bun run generate-api-types:server", @@ -32,13 +34,15 @@ "lint:sdk": "cd packages/sdk && bun run lint", "lint:app": "cd packages/app && bun run lint", "lint:server": "cd packages/server && bun run lint", - "lint": "bun run lint:sdk && bun run lint:app && bun run lint:server", + "lint:skills": "cd packages/skills && bun run lint", + "lint": "bun run lint:sdk && bun run lint:app && bun run lint:server && bun run lint:skills", "lint:cli": "cd packages/cli && bun run lint", "typecheck:sdk": "cd packages/sdk && bun run tsc --noEmit", "typecheck:app": "cd packages/app && bun run tsc --noEmit", "typecheck:server": "cd packages/server && bun run tsc --noEmit", "typecheck:cli": "cd packages/cli && bun run typecheck", - "typecheck": "bun run typecheck:sdk && bun run typecheck:app && bun run typecheck:server", + "typecheck:skills": "cd packages/skills && bun run typecheck", + "typecheck": "bun run typecheck:sdk && bun run typecheck:app && bun run typecheck:server && bun run typecheck:skills", "format:sdk": "cd packages/sdk && bun run format", "format:app": "cd packages/app && bun run format", "format:server": "cd packages/server && bun run format", diff --git a/packages/server/src/mcp/skills/build_skills_bundle.ts b/packages/server/src/mcp/skills/build_skills_bundle.ts index 1671b8a5..db121152 100644 --- a/packages/server/src/mcp/skills/build_skills_bundle.ts +++ b/packages/server/src/mcp/skills/build_skills_bundle.ts @@ -13,7 +13,7 @@ import * as fs from "fs"; import * as path from "path"; -interface SkillEntry { +export interface SkillEntry { name: string; description: string; body: string; @@ -36,6 +36,33 @@ export function parseSkill(md: string, dirName: string): SkillEntry { return { name, description, body }; } +/** + * `credible-*` skills are Credible-platform-specific and never ship (see + * skills/README.md). They can legitimately sit in skills/ uncommitted: the + * directory is a gitignored local install target (`skills/credible-*` in + * .gitignore), so the bundle must skip them or a regeneration on a machine + * with Credible installed would bake them into the committed bundle, and the + * regenerate-and-compare spec would fail on a clean checkout. + * + * The canonical predicate lives in packages/skills/scripts/exclusions.ts; + * duplicated here because the server does not depend on that package. + */ +export function isCredible(dirName: string): boolean { + return dirName.toLowerCase().startsWith("credible-"); +} + +/** Read every skill under a skills directory, in the bundle's own order. */ +export function buildSkills(skillsDir: string): SkillEntry[] { + const skills: SkillEntry[] = []; + for (const entry of fs.readdirSync(skillsDir, { withFileTypes: true })) { + if (!entry.isDirectory() || isCredible(entry.name)) continue; + const skillFile = path.join(skillsDir, entry.name, "SKILL.md"); + if (!fs.existsSync(skillFile)) continue; + skills.push(parseSkill(fs.readFileSync(skillFile, "utf8"), entry.name)); + } + return skills.sort((a, b) => a.name.localeCompare(b.name)); +} + function main(): void { const skillsDir = process.argv[2]; const outFile = @@ -47,14 +74,7 @@ function main(): void { process.exit(1); } - const skills: SkillEntry[] = []; - for (const entry of fs.readdirSync(skillsDir, { withFileTypes: true })) { - if (!entry.isDirectory()) continue; - const skillFile = path.join(skillsDir, entry.name, "SKILL.md"); - if (!fs.existsSync(skillFile)) continue; - skills.push(parseSkill(fs.readFileSync(skillFile, "utf8"), entry.name)); - } - skills.sort((a, b) => a.name.localeCompare(b.name)); + const skills = buildSkills(skillsDir); fs.writeFileSync(outFile, JSON.stringify({ skills })); console.log(`Wrote ${skills.length} skills to ${outFile}`); diff --git a/packages/server/src/mcp/skills/skills_bundle.spec.ts b/packages/server/src/mcp/skills/skills_bundle.spec.ts index c105b262..c0ce81d2 100644 --- a/packages/server/src/mcp/skills/skills_bundle.spec.ts +++ b/packages/server/src/mcp/skills/skills_bundle.spec.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import { buildSkills, type SkillEntry } from "./build_skills_bundle"; import bundle from "./skills_bundle.json"; const skills = ( @@ -7,9 +9,36 @@ const skills = ( } ).skills; +/** The repo's top-level skills/, which this bundle is generated from. */ +const sourceDir = path.join( + import.meta.dir, + "..", + "..", + "..", + "..", + "..", + "skills", +); + +/** + * Codepoint order. The committed bundle is sorted with localeCompare, which + * depends on the runtime's locale, and this suite runs on three platforms. + * Membership and content are what matter here; file order is cosmetic. + */ +const byName = (a: SkillEntry, b: SkillEntry) => + a.name < b.name ? -1 : a.name > b.name ? 1 : 0; + describe("skills_bundle.json (generated dual-channel asset)", () => { - it("contains the full ported skill set", () => { - expect(skills.length).toBeGreaterThanOrEqual(24); + it("is in sync with skills/", () => { + // Regenerate with: + // bun run src/mcp/skills/build_skills_bundle.ts ../../skills + expect([...skills].sort(byName)).toEqual( + [...buildSkills(sourceDir)].sort(byName), + ); + }); + + it("is not empty", () => { + expect(skills.length).toBeGreaterThan(0); }); it("every skill has a nonempty name, description, and body", () => { diff --git a/packages/skills/.gitignore b/packages/skills/.gitignore new file mode 100644 index 00000000..3bbb9e2d --- /dev/null +++ b/packages/skills/.gitignore @@ -0,0 +1,5 @@ +# Generated by scripts/copy-skills.ts at pack time; the source of truth is the +# repo's top-level skills/ directory. Shipped anyway: package.json "files" +# overrides .gitignore. +skills/ +dist/ diff --git a/packages/skills/.prettierrc b/packages/skills/.prettierrc new file mode 100644 index 00000000..7e76f63c --- /dev/null +++ b/packages/skills/.prettierrc @@ -0,0 +1,4 @@ +{ + "printWidth": 80, + "tabWidth": 3 +} diff --git a/packages/skills/LICENSE b/packages/skills/LICENSE new file mode 100644 index 00000000..db145fd3 --- /dev/null +++ b/packages/skills/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) The Linux Foundation + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/packages/skills/README.md b/packages/skills/README.md new file mode 100644 index 00000000..b51f707b --- /dev/null +++ b/packages/skills/README.md @@ -0,0 +1,68 @@ +# @malloy-publisher/skills + +The agent skills that ship with [Malloy Publisher](https://github.com/malloydata/publisher), as +files you can install. They are task-specific guides for writing Malloy, building and reviewing +models, exploring data, and authoring HTML data apps. + +Publisher already serves these skills over MCP, as prompts. This package exists for the cases MCP +does not cover: a host that reads skill files from disk (Claude Code and the Publisher plugin), a +scaffolder that installs them into a new project, or any tool that wants the `reference/` files that +the prompts point at but cannot carry. + +## Install + +```bash +npm install @malloy-publisher/skills +``` + +## Copy the skills into a project + +```js +import { cpSync } from "node:fs"; +import { skillsDir } from "@malloy-publisher/skills"; + +cpSync(skillsDir, ".claude/skills", { recursive: true }); +``` + +Copy the files rather than symlinking them. npm drops symlinks from a tarball, and a symlink into +`node_modules` breaks the moment the tree is pruned or the package is hoisted somewhere else. + +## List what is available + +```js +import { listSkills } from "@malloy-publisher/skills"; + +for (const skill of listSkills()) { + console.log(skill.name, skill.description, skill.dir); +} +``` + +`listSkills()` reads each skill's frontmatter from disk, so it cannot drift from the files it +describes. + +## What is in it + +Each skill is a directory holding a `SKILL.md`, and some also carry a `reference/` directory that the +skill points to for detail it does not inline. Start with `malloy-getting-started`. Use +`malloy-modeling` to build or change a model, `malloy-analysis` to explore and answer questions, and +`malloy-review` to check Malloy for correctness. + +A few skills (`malloy-modeling`, `malloy-publish`, `malloy-document`, `malloy-getting-started`, and +the `malloy` index) are written for a Publisher host and name Publisher's `malloy_*` MCP tools +directly. The rest describe Malloy itself and refer to tools by bare name (`get_context`, +`execute_query`, `search_malloy_docs`), because the prefix depends on the host. On Publisher those +are `malloy_getContext`, `malloy_executeQuery`, and `malloy_searchDocs`. + +## Versioning + +This package versions on its own line, separately from `@malloy-publisher/server`, `sdk`, and `app`. +A skill edit does not need a server release. + +The MCP prompt channel is a separate story: the server compiles the skill bodies into its own bundle +at build time, so installing a newer version of this package does not change what an already-built +server serves over MCP. + +## Contributing + +The skills live at [`skills/`](../../skills) in the Publisher repo, and this package copies that +directory in when it is packed. Edit them there. diff --git a/packages/skills/eslint.config.mjs b/packages/skills/eslint.config.mjs new file mode 100644 index 00000000..edddff96 --- /dev/null +++ b/packages/skills/eslint.config.mjs @@ -0,0 +1,33 @@ +import js from '@eslint/js'; +import tseslint from '@typescript-eslint/eslint-plugin'; +import tsparser from '@typescript-eslint/parser'; + +export default [ + { + ignores: ['dist/', 'node_modules/', 'skills/'], + }, + js.configs.recommended, + { + files: ['src/**/*.ts', 'scripts/**/*.ts'], + languageOptions: { + parser: tsparser, + parserOptions: { + ecmaVersion: 2022, + sourceType: 'module', + }, + }, + plugins: { + '@typescript-eslint': tseslint, + }, + rules: { + ...tseslint.configs.recommended.rules, + // TypeScript resolves globals and imports; no-undef only produces false + // positives here (typescript-eslint recommends turning it off). + 'no-undef': 'off', + '@typescript-eslint/no-unused-vars': [ + 'error', + { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }, + ], + }, + }, +]; diff --git a/packages/skills/package.json b/packages/skills/package.json new file mode 100644 index 00000000..d3f754b3 --- /dev/null +++ b/packages/skills/package.json @@ -0,0 +1,54 @@ +{ + "name": "@malloy-publisher/skills", + "description": "Agent skills for Malloy and the Malloy Publisher", + "version": "0.1.0", + "license": "MIT", + "type": "module", + "main": "dist/index.js", + "types": "dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + }, + "./package.json": "./package.json", + "./skills/*": "./skills/*" + }, + "files": [ + "skills", + "dist" + ], + "keywords": [ + "malloy", + "publisher", + "agent", + "skills" + ], + "repository": { + "type": "git", + "url": "https://github.com/malloydata/publisher.git" + }, + "publishConfig": { + "access": "public" + }, + "scripts": { + "copy-skills": "bun run scripts/copy-skills.ts", + "build": "bun run copy-skills && bunx tsc -p tsconfig.build.json", + "prepack": "bun run build", + "prepublishOnly": "bun run scripts/check-pack.ts", + "check-pack": "bun run build && bun run scripts/check-pack.ts", + "typecheck": "bunx tsc --noEmit", + "lint": "bunx eslint ./src ./scripts --fix", + "format": "bunx prettier --write --parser typescript '{src,scripts}/**/*.ts'", + "test": "bun run copy-skills && bun test src" + }, + "devDependencies": { + "@eslint/js": "^8.57.0", + "@types/bun": "^1.2.21", + "@types/node": "^24.10.0", + "@typescript-eslint/eslint-plugin": "^8.16.0", + "@typescript-eslint/parser": "^8.16.0", + "eslint": "^8.50.0", + "typescript": "^5.3.3" + } +} diff --git a/packages/skills/scripts/check-pack.ts b/packages/skills/scripts/check-pack.ts new file mode 100644 index 00000000..7fd4dfd5 --- /dev/null +++ b/packages/skills/scripts/check-pack.ts @@ -0,0 +1,128 @@ +/** + * Verify what `npm pack` really publishes, by packing and reading the tarball. + * + * The failure this exists to catch is silent: a symlinked or uncopied skills/ + * dir packs to package.json alone, with exit 0, no warning, and every other + * test still green. Nothing else in the repo would notice an empty tarball. + * + * It reads the real tarball rather than `npm pack --json`, whose stdout is + * shared with whatever the prepack script prints, and which reports what npm + * intends to pack rather than what it packed. + * + * Expectations are derived from the source skills/ tree, never hardcoded. The + * MCP bundle's own "at least 24 skills" floor sat 4 behind reality, which is + * what a hardcoded count buys you. + */ +import { execFileSync } from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; +import { isExcluded } from "./exclusions"; + +const packageDir = path.dirname(path.dirname(fileURLToPath(import.meta.url))); +const source = path.join(packageDir, "..", "..", "skills"); + +/** Every file under skills/, as forward-slash paths relative to skills/. */ +function sourceFiles(dir: string = source, prefix = ""): string[] { + const files: string[] = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const relative = prefix ? `${prefix}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + files.push(...sourceFiles(path.join(dir, entry.name), relative)); + } else { + files.push(relative); + } + } + return files; +} + +/** Pack for real and list the files that came out. */ +function pack(): Set { + const outDir = fs.mkdtempSync(path.join(os.tmpdir(), "skills-pack-")); + try { + execFileSync("npm", ["pack", "--pack-destination", outDir], { + cwd: packageDir, + stdio: ["ignore", "inherit", "inherit"], + // Clear an inherited dry-run: under `npm publish --dry-run` this pack + // would write no tarball, and the audit would report that as the + // catastrophic empty-pack it exists to catch. + env: { ...process.env, npm_config_dry_run: "false" }, + }); + const tarball = fs.readdirSync(outDir).find((f) => f.endsWith(".tgz")); + if (!tarball) + throw new Error(`npm pack produced no tarball in ${outDir}`); + return new Set( + execFileSync("tar", ["-tzf", path.join(outDir, tarball)], { + encoding: "utf8", + }) + .split("\n") + .filter(Boolean) + // Entries are prefixed with "package/", and some tars add "./". + .map((entry) => entry.replace(/^\.?\/?package\//, "")) + .filter((entry) => !entry.endsWith("/")), + ); + } finally { + fs.rmSync(outDir, { recursive: true, force: true }); + } +} + +const expected = sourceFiles() + .filter((file) => !isExcluded(file)) + .map((file) => `skills/${file}`); + +const packedPaths = pack(); +const manifest = JSON.parse( + fs.readFileSync(path.join(packageDir, "package.json"), "utf8"), +); +const failures: string[] = []; + +const missing = expected.filter((file) => !packedPaths.has(file)); +if (missing.length > 0) { + failures.push( + `${missing.length} of ${expected.length} skill files are missing from ` + + `the tarball, starting with: ${missing.slice(0, 5).join(", ")}`, + ); +} + +for (const required of ["dist/index.js", "dist/index.d.ts", "README.md"]) { + if (!packedPaths.has(required)) { + failures.push(`the tarball is missing ${required}`); + } +} + +// Same predicate as the copy, so the two cannot disagree about a given file. +const leaked = [...packedPaths] + .filter((file) => file.startsWith("skills/")) + .filter((file) => isExcluded(file.slice("skills/".length))); +if (leaked.length > 0) { + failures.push(`the tarball must not contain: ${leaked.join(", ")}`); +} + +/** + * npm ships a `workspace:*` range verbatim, with exit 0 and no warning, and the + * consumer's install then dies with EUNSUPPORTEDPROTOCOL. Only packages/app is + * de-workspaced before publishing, by a sed naming one file and one key, so a + * workspace dep added here would reach users unnoticed. + */ +const workspaceDeps = Object.entries({ + ...manifest.dependencies, + ...manifest.peerDependencies, +}).filter(([, range]) => String(range).startsWith("workspace:")); +if (workspaceDeps.length > 0) { + failures.push( + `a published package cannot depend on workspace:*: ` + + workspaceDeps.map(([name]) => name).join(", "), + ); +} + +if (failures.length > 0) { + console.error(`npm pack check failed for ${manifest.name}:`); + for (const failure of failures) console.error(` - ${failure}`); + process.exit(1); +} + +console.log( + `npm pack check passed: ${expected.length} skill files and dist/ present ` + + `in ${packedPaths.size} entries (${manifest.name}@${manifest.version})`, +); diff --git a/packages/skills/scripts/copy-skills.ts b/packages/skills/scripts/copy-skills.ts new file mode 100644 index 00000000..bf605a90 --- /dev/null +++ b/packages/skills/scripts/copy-skills.ts @@ -0,0 +1,38 @@ +/** + * Copy the repo's top-level skills/ into this package so npm can pack it. + * + * npm cannot pack above the package directory, and a symlink named in "files" + * packs as nothing at all (you get a tarball holding package.json, exit 0, no + * warning), so the files have to physically exist here at pack time. Runs from + * prepack; the copy is gitignored. + */ +import * as fs from "node:fs"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; +import { isExcluded } from "./exclusions"; + +const packageDir = path.dirname(path.dirname(fileURLToPath(import.meta.url))); +const source = path.join(packageDir, "..", "..", "skills"); +const destination = path.join(packageDir, "skills"); + +fs.rmSync(destination, { recursive: true, force: true }); +fs.cpSync(source, destination, { + recursive: true, + filter: (from: string) => { + const relative = path.relative(source, from); + return relative === "" || !isExcluded(relative); + }, +}); + +const copied = fs + .readdirSync(destination, { withFileTypes: true }) + .filter((entry) => + fs.existsSync(path.join(destination, entry.name, "SKILL.md")), + ); + +if (copied.length === 0) { + console.error(`No skills found in ${source}`); + process.exit(1); +} + +console.log(`Copied ${copied.length} skills to ${destination}`); diff --git a/packages/skills/scripts/exclusions.ts b/packages/skills/scripts/exclusions.ts new file mode 100644 index 00000000..55e4ec98 --- /dev/null +++ b/packages/skills/scripts/exclusions.ts @@ -0,0 +1,62 @@ +/** + * What must not reach the published package, in one place. + * + * The copy, the pack audit, and the tests all have to agree exactly. When they + * drift, one of them silently permits what another forbids: a filter that only + * the copy applies makes the audit demand a file that will never be there, and + * a filter only the audit applies lets the copy ship it. + */ +import * as path from "node:path"; + +/** Path segments of a forward-slash or platform-separated relative path. */ +function segments(relative: string): string[] { + return relative.split(/[\\/]/).filter(Boolean); +} + +/** + * `credible-*` skills are Credible-platform-specific and must never be + * published (see skills/README.md). + * + * Case-insensitive, and matched at every depth: an npm tarball cannot be + * unpublished, so this errs toward excluding a file it should not rather than + * shipping one it must not. + */ +export function isCredible(relative: string): boolean { + return segments(relative).some((segment) => + segment.toLowerCase().startsWith("credible-"), + ); +} + +/** + * Editor and OS debris. It must not enter the copy or the audit's + * expectations: npm strips .DS_Store and friends from every tarball whatever + * "files" says, so a stray one would be reported as a missing skill file, which + * is what the audit says when the copy has catastrophically broken. + * + * The dotfile rule is deliberately broader than npm's own list (npm packs + * .prettierrc and the like): skills are markdown, so no dotfile under skills/ + * is content. + */ +export function isNeverPacked(relative: string): boolean { + const name = path.basename(relative); + return ( + name.startsWith(".") || name.endsWith(".orig") || name === "npm-debug.log" + ); +} + +/** + * skills/README.md documents the internal upstream-sync process, which is not + * what a consumer of this package needs. The package ships its own README. + */ +export function isSourceReadme(relative: string): boolean { + return relative === "README.md"; +} + +/** True if a path relative to skills/ must not reach the package. */ +export function isExcluded(relative: string): boolean { + return ( + isCredible(relative) || + isNeverPacked(relative) || + isSourceReadme(relative) + ); +} diff --git a/packages/skills/src/index.spec.ts b/packages/skills/src/index.spec.ts new file mode 100644 index 00000000..50f0b6f3 --- /dev/null +++ b/packages/skills/src/index.spec.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from "bun:test"; +import { execFileSync } from "node:child_process"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { isCredible, isExcluded } from "../scripts/exclusions"; +import { listSkills, skillsDir } from "./index"; + +/** The repo's top-level skills/, which copy-skills.ts copies into this package. */ +const sourceDir = path.join(import.meta.dir, "..", "..", "..", "skills"); + +function skillNamesIn(dir: string): string[] { + return fs + .readdirSync(dir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .filter((name) => fs.existsSync(path.join(dir, name, "SKILL.md"))) + .sort(); +} + +describe("@malloy-publisher/skills", () => { + it("ships every publishable skill in the repo's skills/ directory", () => { + const publishable = skillNamesIn(sourceDir).filter( + (name) => !isExcluded(name), + ); + expect(skillNamesIn(skillsDir)).toEqual(publishable); + }); + + /** + * The copy filters `credible-*` out, but that is a backstop. A committed one + * is the defect itself (skills/README.md), and this package turns skills/ + * into a published artifact that npm cannot unpublish, so say so loudly and + * point at the stray rather than at the copy. + * + * Asks git, not the filesystem: an uncommitted credible-* skill under + * skills/ is a supported local state (`skills/credible-*` is gitignored as + * a local install target), and only a committed one is the defect. + */ + it("has no credible-* file committed to skills/", () => { + const committed = execFileSync("git", ["ls-files"], { + cwd: sourceDir, + encoding: "utf8", + }) + .split("\n") + .filter(Boolean); + expect(committed.filter(isCredible)).toEqual([]); + }); + + it("finds a skill for each one it ships", () => { + expect(listSkills().length).toBe(skillNamesIn(skillsDir).length); + }); + + it("reads a name and description for every skill", () => { + for (const skill of listSkills()) { + expect(skill.name.length).toBeGreaterThan(0); + expect(skill.description.length).toBeGreaterThan(0); + expect(fs.existsSync(path.join(skill.dir, "SKILL.md"))).toBe(true); + } + }); + + /** + * The frontmatter name is what MCP prompts and `skill:` cross-references use; + * the directory is what this package ships. If they drift, a consumer doing + * path.join(skillsDir, skill.name) gets ENOENT for a skill that exists. + */ + it("names each skill after its directory", () => { + for (const skill of listSkills()) { + expect(skill.name).toBe(path.basename(skill.dir)); + } + }); + + // The reason this package exists: reference/ files reach no npm consumer + // today, so the pointers to them in the MCP prompt bodies dangle. + it("brings each skill's reference/ files along", () => { + const withReference = skillNamesIn(sourceDir) + .filter((name) => !isExcluded(name)) + .filter((name) => + fs.existsSync(path.join(sourceDir, name, "reference")), + ); + expect(withReference.length).toBeGreaterThan(0); + for (const name of withReference) { + const shipped = fs.readdirSync( + path.join(skillsDir, name, "reference"), + ); + const original = fs + .readdirSync(path.join(sourceDir, name, "reference")) + .filter((file) => !isExcluded(`${name}/reference/${file}`)); + expect(shipped.sort()).toEqual(original.sort()); + } + }); + + it("leaves out the internal sync README and any credible-* skill", () => { + const shipped = fs.readdirSync(skillsDir); + expect(shipped).not.toContain("README.md"); + expect(shipped.filter(isCredible)).toEqual([]); + }); +}); diff --git a/packages/skills/src/index.ts b/packages/skills/src/index.ts new file mode 100644 index 00000000..982c33a6 --- /dev/null +++ b/packages/skills/src/index.ts @@ -0,0 +1,64 @@ +/** + * The skill files are this package's payload; these exports exist so a consumer + * can find them without hardcoding a node_modules layout. + */ +import * as fs from "node:fs"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; + +/** A skill's frontmatter, plus where its files live. */ +export interface Skill { + name: string; + description: string; + /** Absolute path to the skill's directory: SKILL.md and any reference/. */ + dir: string; +} + +/** + * Absolute path to the directory holding the skill directories. + * + * Resolved from this module's own URL, which works both from dist/ once + * installed and from src/ in the repo, since both sit one level under the + * package root. + */ +export const skillsDir: string = path.join( + path.dirname(fileURLToPath(import.meta.url)), + "..", + "skills", +); + +function unquote(value: string): string { + return value.trim().replace(/^["']|["']$/g, ""); +} + +/** + * Read every skill's frontmatter from the shipped files. + * + * Parsed on demand rather than baked into a generated manifest, so there is no + * second copy of this metadata to regenerate or to drift. + */ +export function listSkills(): Skill[] { + const skills: Skill[] = []; + for (const entry of fs.readdirSync(skillsDir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const dir = path.join(skillsDir, entry.name); + const skillFile = path.join(dir, "SKILL.md"); + if (!fs.existsSync(skillFile)) continue; + const markdown = fs + .readFileSync(skillFile, "utf8") + .replace(/\r\n/g, "\n"); + const frontmatter = markdown.match(/^---\n([\s\S]*?)\n---/)?.[1] ?? ""; + skills.push({ + name: unquote(frontmatter.match(/^name:\s*(.+)$/m)?.[1] ?? entry.name), + description: unquote( + frontmatter.match(/^description:\s*(.+)$/m)?.[1] ?? "", + ), + dir, + }); + } + // Codepoint order, not localeCompare: the result is compared across + // platforms and must not depend on the runtime's locale. + return skills.sort((a, b) => + a.name < b.name ? -1 : a.name > b.name ? 1 : 0, + ); +} diff --git a/packages/skills/tsconfig.build.json b/packages/skills/tsconfig.build.json new file mode 100644 index 00000000..102e2b8b --- /dev/null +++ b/packages/skills/tsconfig.build.json @@ -0,0 +1,12 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": false, + "declaration": true, + "outDir": "./dist", + "rootDir": "./src", + "types": ["node"] + }, + "include": ["src/**/*"], + "exclude": ["node_modules", "dist", "skills", "src/**/*.spec.ts"] +} diff --git a/packages/skills/tsconfig.json b/packages/skills/tsconfig.json new file mode 100644 index 00000000..79c52ed0 --- /dev/null +++ b/packages/skills/tsconfig.json @@ -0,0 +1,19 @@ +{ + "compilerOptions": { + "target": "ESNext", + "lib": ["ESNext"], + "module": "ESNext", + "moduleResolution": "bundler", + "types": ["node", "bun"], + "noEmit": true, + "strict": true, + "skipLibCheck": true, + "isolatedModules": true, + "forceConsistentCasingInFileNames": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noFallthroughCasesInSwitch": true + }, + "include": ["src/**/*", "scripts/**/*"], + "exclude": ["node_modules", "dist", "skills"] +} diff --git a/skills/README.md b/skills/README.md index 3e372686..168dda8c 100644 --- a/skills/README.md +++ b/skills/README.md @@ -2,6 +2,8 @@ Task-specific guides for working with Malloy through this Publisher deployment. Claude Code auto-discovers them via the `.claude/skills/` symlinks; other hosts pull the same content as MCP prompts from the Publisher endpoint. Start with [`malloy-getting-started`](malloy-getting-started/SKILL.md); use `malloy-modeling` to build a model, `malloy-analysis` to answer questions, and `malloy-review` to check Malloy for correctness. +[`packages/skills`](../packages/skills) publishes this directory to npm, for consumers that need the files themselves without cloning. That is the channel the `reference/` directories reach: the MCP prompts carry each `SKILL.md` body and nothing else. It copies this tree in when it is packed, so adding a skill here needs no extra step to ship it. + ## Where these come from Most of these skills are **shared, open-source Malloy skills** kept in sync with the Credible source-of-truth repo (`ms2data/agent-skills`, `skills/`). The intent is that the shared skills are the *same text* in both repos, copyable verbatim in either direction. Automation will come later; for now the copy is manual (`cp`). Two rules make it work: