From d6124753ec8ec20b3337ebaef7d7352d09cbbd41 Mon Sep 17 00:00:00 2001 From: Monty Lennie Date: Thu, 16 Jul 2026 20:54:49 -0600 Subject: [PATCH 1/3] feat(skills): publish the agent skills as their own npm package The npm publish root is packages/server, so skills/ sits above it and cannot reach the tarball. Skill bodies do reach npx users, inlined into the MCP prompt bundle, but no skill files do, and the 25 reference/ files reach nobody: 19 live reference/ pointers inside those prompt bodies name files the host cannot open. A skills content edit also has to ride a joint sdk+app+server version bump, because the only thing that carries it is the regenerated bundle. @malloy-publisher/skills packs the 28 skills and their reference/ directories as real files, on its own version line. It exports skillsDir and listSkills() so a consumer can locate them without hardcoding a node_modules layout; listSkills() parses frontmatter from the shipped files, so there is no generated manifest to drift. The exports map keeps ./package.json and ./skills/* reachable, without which require.resolve('pkg/package.json'), the usual way to find a package root, fails outright. skills/ stays at the repo root and prepack copies it in. A symlink named in "files" packs to package.json alone, with exit 0 and no warning, so the copy is load-bearing rather than a convenience, and check-pack packs for real and compares the tarball against the source tree because that failure is otherwise silent. The same shared predicate drives the copy, the audit and the tests, so they cannot disagree about what may ship. This does not make MCP any fresher. The server statically imports the bundle and the bundler inlines it, so newer skills still need a server build. The package buys the file channel and its own semver, nothing more. Also replaces skills_bundle.spec.ts's "at least 24 skills" floor, which sat 4 behind reality and would pass a four-skill regression, with a regenerate-and- compare against skills/. Publishing is a separate manual dispatch and needs an npm trusted-publisher entry, which cannot exist until the package does; the workflow documents the order. Signed-off-by: Monty Lennie --- .github/workflows/cross-platform-tests.yml | 6 + .github/workflows/skills-npm.yml | 110 +++++++++++++++ .gitignore | 1 + bun.lock | 20 +++ package.json | 10 +- .../src/mcp/skills/build_skills_bundle.ts | 23 ++-- .../src/mcp/skills/skills_bundle.spec.ts | 33 ++++- packages/skills/.gitignore | 5 + packages/skills/.prettierrc | 4 + packages/skills/LICENSE | 21 +++ packages/skills/README.md | 68 ++++++++++ packages/skills/eslint.config.mjs | 33 +++++ packages/skills/package.json | 54 ++++++++ packages/skills/scripts/check-pack.ts | 128 ++++++++++++++++++ packages/skills/scripts/copy-skills.ts | 38 ++++++ packages/skills/scripts/exclusions.ts | 62 +++++++++ packages/skills/src/index.spec.ts | 99 ++++++++++++++ packages/skills/src/index.ts | 64 +++++++++ packages/skills/tsconfig.build.json | 12 ++ packages/skills/tsconfig.json | 19 +++ skills/README.md | 2 + 21 files changed, 798 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/skills-npm.yml create mode 100644 packages/skills/.gitignore create mode 100644 packages/skills/.prettierrc create mode 100644 packages/skills/LICENSE create mode 100644 packages/skills/README.md create mode 100644 packages/skills/eslint.config.mjs create mode 100644 packages/skills/package.json create mode 100644 packages/skills/scripts/check-pack.ts create mode 100644 packages/skills/scripts/copy-skills.ts create mode 100644 packages/skills/scripts/exclusions.ts create mode 100644 packages/skills/src/index.spec.ts create mode 100644 packages/skills/src/index.ts create mode 100644 packages/skills/tsconfig.build.json create mode 100644 packages/skills/tsconfig.json diff --git a/.github/workflows/cross-platform-tests.yml b/.github/workflows/cross-platform-tests.yml index 65ce7fba8..cdb68494c 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 000000000..365b085c2 --- /dev/null +++ b/.github/workflows/skills-npm.yml @@ -0,0 +1,110 @@ +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 + 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 78f0e31f5..2e887c580 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 34f0fa840..0bfc054c0 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 09c682758..4aa5de432 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 1671b8a51..c9b07a42f 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,18 @@ export function parseSkill(md: string, dirName: string): SkillEntry { return { name, description, body }; } +/** 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()) 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 +59,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 c105b2620..c0ce81d2b 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 000000000..3bbb9e2d9 --- /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 000000000..7e76f63c7 --- /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 000000000..db145fd39 --- /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 000000000..b51f707b3 --- /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 000000000..edddff968 --- /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 000000000..d3f754b3a --- /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 000000000..7fd4dfd5b --- /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 000000000..bf605a905 --- /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 000000000..55e4ec988 --- /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 000000000..7338c857d --- /dev/null +++ b/packages/skills/src/index.spec.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from "bun:test"; +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"); + +/** Every file under a dir, as forward-slash paths relative to it. */ +function filesIn(dir: string, 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(...filesIn(path.join(dir, entry.name), relative)); + } else { + files.push(relative); + } + } + return files; +} + +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. + */ + it("has no credible-* file committed to skills/", () => { + expect(filesIn(sourceDir).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 000000000..982c33a6e --- /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 000000000..102e2b8be --- /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 000000000..79c52ed00 --- /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 3e3726863..168dda8c1 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: From 4f24dcb02f070f75a5ca57f0753d69244fad3c6e Mon Sep 17 00:00:00 2001 From: kylenesbit <148360916+kylenesbit@users.noreply.github.com> Date: Sun, 19 Jul 2026 20:23:21 -0600 Subject: [PATCH 2/3] Potential fix for pull request finding 'CodeQL / Workflow does not contain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .github/workflows/skills-npm.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/skills-npm.yml b/.github/workflows/skills-npm.yml index 365b085c2..f169166bf 100644 --- a/.github/workflows/skills-npm.yml +++ b/.github/workflows/skills-npm.yml @@ -26,6 +26,8 @@ jobs: check_pack: name: Verify the published tarball runs-on: ubuntu-latest + permissions: + contents: read steps: - uses: actions/checkout@v4 From 6949d583dfb7e81ec34fa7f041067612c75a6efa Mon Sep 17 00:00:00 2001 From: Kyle Nesbit Date: Sun, 19 Jul 2026 23:10:33 -0600 Subject: [PATCH 3/3] fix(skills): tolerate gitignored local credible-* installs in the new gates skills/credible-*/ is gitignored as a local install target, but two of the new gates failed on exactly that state on an untouched checkout: - skills_bundle.spec.ts's regenerate-and-compare read every skill directory off disk, so an uncommitted local credible-* skill made the committed bundle look stale. buildSkills() now skips credible-* directories, which also closes a regeneration leak: rebuilding the bundle on a machine with Credible installed would have baked the local skills into the committed artifact, and the sync test would then have blessed the leak. - index.spec.ts's "no credible-* committed" guard scanned the filesystem, so it fired on the gitignored local install it exists to permit. It now asks git ls-files, and only a committed stray trips it. Verified both directions: with a gitignored credible-* skill present, both suites pass and bundle regeneration is byte-identical to the committed bundle; with the same skill force-staged, the committed guard still fails and names the stray file. Signed-off-by: Kyle Nesbit Co-authored-by: Cursor --- .../src/mcp/skills/build_skills_bundle.ts | 17 +++++++++++- packages/skills/src/index.spec.ts | 27 +++++++++---------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/packages/server/src/mcp/skills/build_skills_bundle.ts b/packages/server/src/mcp/skills/build_skills_bundle.ts index c9b07a42f..db1211529 100644 --- a/packages/server/src/mcp/skills/build_skills_bundle.ts +++ b/packages/server/src/mcp/skills/build_skills_bundle.ts @@ -36,11 +36,26 @@ 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()) continue; + 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)); diff --git a/packages/skills/src/index.spec.ts b/packages/skills/src/index.spec.ts index 7338c857d..50f0b6f3b 100644 --- a/packages/skills/src/index.spec.ts +++ b/packages/skills/src/index.spec.ts @@ -1,4 +1,5 @@ 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"; @@ -7,20 +8,6 @@ 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"); -/** Every file under a dir, as forward-slash paths relative to it. */ -function filesIn(dir: string, 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(...filesIn(path.join(dir, entry.name), relative)); - } else { - files.push(relative); - } - } - return files; -} - function skillNamesIn(dir: string): string[] { return fs .readdirSync(dir, { withFileTypes: true }) @@ -43,9 +30,19 @@ describe("@malloy-publisher/skills", () => { * 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/", () => { - expect(filesIn(sourceDir).filter(isCredible)).toEqual([]); + 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", () => {