From 4d2a8031f39c65435df61c4dce9ca26d31111b11 Mon Sep 17 00:00:00 2001 From: Antonio Bonet Date: Fri, 10 Apr 2026 03:57:11 +0700 Subject: [PATCH 1/5] test: validate skills map entries and add contributing guide Add structural validation test for SKILLS_MAP and COMBO_SKILLS_MAP with 12 tests verifying entry shape, required fields, detect signals, skill refs, and combo references. Error messages are actionable with format hints. Rewrite CONTRIBUTING.md with AutoSkills-specific guidance: how to add a technology, combo, or fix detection. Includes detection signal priority, skill ref format, common mistakes, and separate checklists. Closes #62. Implements the pragmatic path: validate the catalog contract and document the contribution workflow without splitting the architecture. --- CONTRIBUTING.md | 275 ++++++++++++-- .../tests/skills-map.validation.test.mjs | 354 ++++++++++++++++++ 2 files changed, 600 insertions(+), 29 deletions(-) create mode 100644 packages/autoskills/tests/skills-map.validation.test.mjs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9a74cc46..10ff016f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,61 +1,278 @@ # Contributing to AutoSkills -First off, thank you for considering contributing to AutoSkills! It's people like you that make open source such a great community. +Thank you for contributing to AutoSkills. This guide covers the full workflow, from finding an issue to opening a clean PR. -## 1. Where do I go from here? +## 1. Find or open an issue first -If you've noticed a bug or have a feature request, please [open an issue](../../issues) on GitHub. It's the best way to get things started. +Before writing code, [open an issue](../../issues) or find an existing one. -## 2. Fork & create a branch +Describe what you want to add or fix and why. Maintainers use issues to coordinate work, avoid duplicates, and give early feedback before you invest time in a PR. -If this is something you think you can fix, then [fork AutoSkills](../../fork) and create a branch with a descriptive name. +## 2. Fork and create a branch -A good branch name would be (where issue #325 is the ticket you're working on): +[Fork AutoSkills](../../fork), then create a descriptive branch: ```sh -git checkout -b 325-add-new-feature +git checkout -b 325-add-tailwind-detection +git checkout -b fix/ruby-gems-empty-array +git checkout -b docs/improve-contributing-guide ``` -Or for a general fix: +## 3. Make your changes + +Make the smallest change that fully solves the problem. Ask for help in the PR — you don't need to wait until it's perfect. + +## 4. Follow the style + +- Lint and format before pushing +- Add or update tests for every behavior change +- Make sure all tests pass locally before opening a PR + +## 5. Open a pull request + +Sync with main before pushing: ```sh -git checkout -b fix/typo-in-readme +git remote add upstream https://github.com/midudev/autoskills.git +git checkout main +git pull upstream main +git checkout your-branch +git rebase main +git push --set-upstream origin your-branch ``` -## 3. Implement your fix or feature +Keep the PR title short and in the imperative mood: `feat: add Bun detection`, `fix: empty gems array in Ruby entry`, `test: validate skills map entries`. -At this point, you're ready to make your changes! Feel free to ask for help on your PR if you get stuck. +## 6. Keep your PR up to date -## 4. Get the style right +When a maintainer asks you to rebase, the base branch has moved. Update your branch: -Your patch should follow the same coding conventions and style as the rest of the project. Please ensure: +```sh +git fetch upstream +git rebase upstream/main +git push --force-with-lease +``` -- Your code is properly linted and formatted before pushing. -- All new features and bug fixes include relevant tests. -- Existing tests pass locally before committing. +## 7. AutoSkills-specific contributions -## 5. Make a Pull Request +Most AutoSkills changes fall into one of these: -At this point, you should switch back to your master branch and make sure it's up to date with the main repository: +- Add detection for a new technology +- Fix a detection edge case +- Add or update skills for an existing technology +- Add a combo that provides value beyond individual detections -```sh -git remote add upstream https://github.com/midudev/autoskills.git -git checkout main -git pull upstream main +**Keep each PR to one concern.** One technology, one fix, one combo. Mixing unrelated changes slows review. + +--- + +### Adding a technology + +Add one entry to `SKILLS_MAP` in `packages/autoskills/skills-map.mjs`: + +```js +{ + id: "bun", + name: "Bun", + detect: { + configFiles: ["bun.lockb", "bunfig.toml"], + }, + skills: [ + "owner/repo/bun-best-practices", + ], +} +``` + +Every entry requires these four fields: + +| Field | Type | Description | +|-------|------|-------------| +| `id` | `string` | Stable, unique, lowercase. Use `-` as separator. | +| `name` | `string` | User-facing display name. | +| `detect` | `object` | At least one detection signal (see below). | +| `skills` | `string[]` | One or more skill references. | + +#### Detection signals — choose the simplest one that works + +Use signals in this order of preference: + +**1. `packages`** — match npm/pip/cargo package names + +```js +detect: { + packages: ["react", "react-dom"], +} +``` + +**2. `configFiles`** — match specific filenames in the project root + +```js +detect: { + configFiles: ["next.config.js", "next.config.mjs", "next.config.ts"], +} +``` + +**3. `gems`** — match Ruby gem names + +```js +detect: { + gems: ["rails", "sinatra"], +} +``` + +**4. `packagePatterns`** — match packages by RegExp when a prefix covers multiple packages + +```js +detect: { + packagePatterns: [/^@aws-sdk\//, /^aws-cdk/], +} +``` + +**5. `configFileContent`** — search file contents when names and packages are not enough + +```js +// Standard form — search inside specific files +detect: { + configFileContent: { + files: ["wrangler.json", "wrangler.toml"], + patterns: ["durable_objects"], + }, +} + +// Gradle form — let the scanner find build.gradle files automatically +detect: { + configFileContent: { + scanGradleLayout: true, + patterns: ["com.android.application", "com.android.library"], + }, +} +``` + +You can combine signals — the entry matches when any signal is found: + +```js +detect: { + packages: ["tailwindcss"], + configFiles: ["tailwind.config.js", "tailwind.config.ts"], +} +``` + +#### Skill reference format + +``` +owner/repo/skill-name # GitHub-hosted skill +https://example.com/skill # URL-based skill ``` -Then update your feature branch from your local copy of main, and push it! +Real example: `"vercel-labs/agent-skills/vercel-react-best-practices"` + +Verify that the referenced skill actually exists before opening a PR. + +#### What the validator catches + +Run the tests and you'll see exactly what needs fixing: + +``` +✗ [my-tech] detect.packages is empty — add at least one package name (e.g. ["react", "react-dom"]) +✗ [my-tech] skill "my-best-practices" is not a valid reference — use "owner/repo/skill-name" or an https:// URL +✗ [my-tech] detect has no signals — define at least one of: packages, configFiles, gems, packagePatterns, or configFileContent +``` + +Common mistakes: + +```js +// ✗ packages is empty +detect: { packages: [] } + +// ✗ skill ref is missing owner/repo +skills: ["react-best-practices"] + +// ✗ packagePattern is a string, not a RegExp +detect: { packagePatterns: ["^@aws-sdk/"] } + +// ✓ correct +detect: { packages: ["react"] } +skills: ["vercel-labs/agent-skills/vercel-react-best-practices"] +detect: { packagePatterns: [/^@aws-sdk\//] } +``` + +--- + +### Adding a combo + +Add one entry to `COMBO_SKILLS_MAP` in `packages/autoskills/skills-map.mjs`: + +```js +{ + id: "nextjs-supabase", + name: "Next.js + Supabase", + requires: ["nextjs", "supabase"], + skills: [ + "supabase/agent-skills/supabase-postgres-best-practices", + ], +} +``` + +Add a combo only when it provides skills that are not already covered by the individual technology entries. A combo that just duplicates standalone skills adds noise. + +| Field | Requirement | +|-------|-------------| +| `id` | Unique, lowercase, hyphen-separated. | +| `name` | User-facing, e.g. `"Next.js + Supabase"`. | +| `requires` | Array of two or more existing `SKILLS_MAP` ids. | +| `skills` | At least one valid skill reference. | + +If you reference an id that doesn't exist in `SKILLS_MAP`, the test fails: + +``` +✗ [nextjs-supabase] requires "supbase" which is not in SKILLS_MAP — fix the typo or add the technology first +``` + +--- + +### Run the tests ```sh -git checkout 325-add-new-feature -git rebase main -git push --set-upstream origin 325-add-new-feature +cd packages/autoskills +node --test 'tests/*.test.mjs' ``` -Finally, go to GitHub and create a Pull Request on the main repository. +Expected output when everything is correct: + +``` +✔ SKILLS_MAP validation › has unique technology ids +✔ SKILLS_MAP validation › validates every technology entry shape +✔ COMBO_SKILLS_MAP validation › has unique combo ids +✔ COMBO_SKILLS_MAP validation › validates every combo entry shape +``` + +--- + +### PR checklists + +#### Adding a technology + +- [ ] One entry added to `SKILLS_MAP` in `packages/autoskills/skills-map.mjs` +- [ ] Detection uses the simplest reliable signal +- [ ] All skill refs follow the `owner/repo/skill-name` or `https://` format +- [ ] Tests pass locally (`node --test 'tests/*.test.mjs'`) +- [ ] PR title uses the imperative mood: `feat: add Bun detection` + +#### Adding a combo + +- [ ] One entry added to `COMBO_SKILLS_MAP` in `packages/autoskills/skills-map.mjs` +- [ ] `requires` contains at least two ids that exist in `SKILLS_MAP` +- [ ] The combo adds skills not already covered by the individual entries +- [ ] Tests pass locally +- [ ] PR title uses the imperative mood: `feat: add Next.js + Supabase combo` + +#### Fixing detection -## 6. Keeping your Pull Request updated +- [ ] Existing entry updated in `packages/autoskills/skills-map.mjs` +- [ ] A test in `packages/autoskills/tests/detect.test.mjs` covers the fixed case +- [ ] Tests pass locally +- [ ] PR title describes the fix: `fix: detect Tailwind from @tailwindcss/vite` -If an maintainer asks you to rebase, they're saying that a lot of code has changed, and that you need to update your branch to easily merge it into the main project. +--- Thank you for contributing! diff --git a/packages/autoskills/tests/skills-map.validation.test.mjs b/packages/autoskills/tests/skills-map.validation.test.mjs new file mode 100644 index 00000000..4ed69947 --- /dev/null +++ b/packages/autoskills/tests/skills-map.validation.test.mjs @@ -0,0 +1,354 @@ +import { describe, it } from "node:test" +import { ok, strictEqual, throws } from "node:assert/strict" + +import { SKILLS_MAP, COMBO_SKILLS_MAP } from "../skills-map.mjs" + +// ── Helpers ─────────────────────────────────────────────────── + +function findDuplicates(values) { + const counts = new Map() + + for (const value of values) { + counts.set(value, (counts.get(value) ?? 0) + 1) + } + + return [...counts.entries()] + .filter(([, count]) => count > 1) + .map(([value]) => value) + .sort() +} + +function isNonEmptyString(value) { + return typeof value === "string" && value.trim().length > 0 +} + +function isValidSkillRef(skill) { + if (/^https?:\/\/\S+$/u.test(skill)) return true + + const parts = skill.split("/") + if (parts.length < 3) return false + + const [owner, repo, ...rest] = parts + return isNonEmptyString(owner) && isNonEmptyString(repo) && isNonEmptyString(rest.join("/")) +} + +function hasAtLeastOneDetectSignal(detect) { + return Boolean( + detect.packages?.length || + detect.packagePatterns?.length || + detect.configFiles?.length || + detect.gems?.length || + detect.configFileContent + ) +} + +function toConfigBlocks(value) { + if (!value) return [] + return Array.isArray(value) ? value : [value] +} + +// ── Per-entry validators ────────────────────────────────────── + +function validateDetectConfig(tech) { + const { detect } = tech + + ok(detect && typeof detect === "object", `[${tech.id}] detect must exist`) + ok( + hasAtLeastOneDetectSignal(detect), + `[${tech.id}] detect has no signals — define at least one of: packages, configFiles, gems, packagePatterns, or configFileContent` + ) + + if (detect.packages !== undefined) { + ok(Array.isArray(detect.packages), `[${tech.id}] detect.packages must be an array`) + ok( + detect.packages.length > 0, + `[${tech.id}] detect.packages is empty — add at least one package name (e.g. ["react", "react-dom"])` + ) + for (const pkg of detect.packages) { + ok(isNonEmptyString(pkg), `[${tech.id}] detect.packages contains an empty or non-string value`) + } + } + + if (detect.packagePatterns !== undefined) { + ok(Array.isArray(detect.packagePatterns), `[${tech.id}] detect.packagePatterns must be an array`) + ok( + detect.packagePatterns.length > 0, + `[${tech.id}] detect.packagePatterns is empty — add at least one RegExp (e.g. [/^@aws-sdk\\//])` + ) + for (const pattern of detect.packagePatterns) { + ok( + pattern instanceof RegExp, + `[${tech.id}] detect.packagePatterns contains a non-RegExp value — use /pattern/ syntax, not a string` + ) + } + } + + if (detect.configFiles !== undefined) { + ok(Array.isArray(detect.configFiles), `[${tech.id}] detect.configFiles must be an array`) + ok( + detect.configFiles.length > 0, + `[${tech.id}] detect.configFiles is empty — add at least one filename (e.g. ["next.config.js", "next.config.ts"])` + ) + for (const file of detect.configFiles) { + ok(isNonEmptyString(file), `[${tech.id}] detect.configFiles contains an empty or non-string value`) + } + } + + if (detect.gems !== undefined) { + ok(Array.isArray(detect.gems), `[${tech.id}] detect.gems must be an array`) + ok( + detect.gems.length > 0, + `[${tech.id}] detect.gems is empty — add at least one gem name (e.g. ["rails", "sinatra"])` + ) + for (const gem of detect.gems) { + ok(isNonEmptyString(gem), `[${tech.id}] detect.gems contains an empty or non-string value`) + } + } + + for (const block of toConfigBlocks(detect.configFileContent)) { + ok(block && typeof block === "object", `[${tech.id}] configFileContent block must be an object`) + ok(Array.isArray(block.patterns), `[${tech.id}] configFileContent.patterns must be an array`) + ok( + block.patterns.length > 0, + `[${tech.id}] configFileContent.patterns is empty — add at least one string pattern to search for` + ) + for (const pattern of block.patterns) { + ok( + isNonEmptyString(pattern), + `[${tech.id}] configFileContent.patterns contains an empty or non-string value` + ) + } + + if (!block.scanGradleLayout) { + ok( + Array.isArray(block.files), + `[${tech.id}] configFileContent.files must be an array (or set scanGradleLayout: true for Gradle projects)` + ) + ok( + block.files.length > 0, + `[${tech.id}] configFileContent.files is empty — add at least one filename to search in` + ) + for (const file of block.files) { + ok( + isNonEmptyString(file), + `[${tech.id}] configFileContent.files contains an empty or non-string value` + ) + } + } + } +} + +function validateSkillList(ownerId, skills) { + ok(Array.isArray(skills), `[${ownerId}] skills must be an array`) + + for (const skill of skills) { + ok(isNonEmptyString(skill), `[${ownerId}] skills contains an empty or non-string value`) + ok( + isValidSkillRef(skill), + `[${ownerId}] skill "${skill}" is not a valid reference — use "owner/repo/skill-name" (e.g. "vercel-labs/agent-skills/vercel-react-best-practices") or an https:// URL` + ) + } + + const duplicateSkills = findDuplicates(skills) + strictEqual( + duplicateSkills.length, + 0, + `[${ownerId}] duplicate skill references: ${duplicateSkills.join(", ")} — each skill can appear only once per entry` + ) +} + +function validateTechnologyEntry(tech) { + ok( + isNonEmptyString(tech.id), + `technology.id is missing or empty — every SKILLS_MAP entry needs a stable unique id (e.g. "react", "nextjs", "prisma")` + ) + ok( + isNonEmptyString(tech.name), + `[${tech.id ?? "?"}] name is missing or empty — add a user-facing display name (e.g. "React", "Next.js")` + ) + validateDetectConfig(tech) + validateSkillList(tech.id, tech.skills) +} + +function validateComboEntry(combo, technologyIds) { + ok( + isNonEmptyString(combo.id), + `combo.id is missing or empty — every COMBO_SKILLS_MAP entry needs a stable unique id (e.g. "nextjs-supabase")` + ) + ok( + isNonEmptyString(combo.name), + `[${combo.id ?? "?"}] name is missing or empty — add a user-facing name (e.g. "Next.js + Supabase")` + ) + + ok(Array.isArray(combo.requires), `[${combo.id}] requires must be an array`) + ok( + combo.requires.length >= 2, + `[${combo.id}] requires must list at least two technology ids — combos need a minimum of two technologies` + ) + + for (const requiredId of combo.requires) { + ok(isNonEmptyString(requiredId), `[${combo.id}] requires contains an empty or non-string id`) + ok( + technologyIds.has(requiredId), + `[${combo.id}] requires "${requiredId}" which is not in SKILLS_MAP — fix the typo or add the technology first` + ) + } + + const duplicateRequires = findDuplicates(combo.requires) + strictEqual( + duplicateRequires.length, + 0, + `[${combo.id}] duplicate ids in requires: ${duplicateRequires.join(", ")} — list each technology id once` + ) + + validateSkillList(combo.id, combo.skills) +} + +// ── SKILLS_MAP ──────────────────────────────────────────────── + +describe("SKILLS_MAP validation", () => { + it("has unique technology ids", () => { + const duplicates = findDuplicates(SKILLS_MAP.map((tech) => tech.id)) + strictEqual(duplicates.length, 0, `duplicate technology ids: ${duplicates.join(", ")}`) + }) + + it("validates every technology entry shape", () => { + for (const tech of SKILLS_MAP) { + validateTechnologyEntry(tech) + } + }) +}) + +// ── COMBO_SKILLS_MAP ────────────────────────────────────────── + +describe("COMBO_SKILLS_MAP validation", () => { + const technologyIds = new Set(SKILLS_MAP.map((tech) => tech.id)) + + it("has unique combo ids", () => { + const duplicates = findDuplicates(COMBO_SKILLS_MAP.map((combo) => combo.id)) + strictEqual(duplicates.length, 0, `duplicate combo ids: ${duplicates.join(", ")}`) + }) + + it("validates every combo entry shape", () => { + for (const combo of COMBO_SKILLS_MAP) { + validateComboEntry(combo, technologyIds) + } + }) +}) + +// ── Feedback quality: error messages must be actionable ─────── +// +// TDD contract: when a contributor submits a malformed entry, +// the assertion message must tell them exactly what to fix. + +describe("validation gives actionable feedback on bad entries", () => { + const VALID_SKILL = "owner/repo/skill-name" + const VALID_DETECT = { packages: ["some-package"] } + + function expectsError(fn, pattern) { + throws(fn, (err) => { + ok( + pattern.test(err.message), + `\nExpected error message matching: ${pattern}\nGot: "${err.message}"` + ) + return true + }) + } + + it("reports missing technology id with a usage hint", () => { + expectsError( + () => validateTechnologyEntry({ name: "My Tech", detect: VALID_DETECT, skills: [VALID_SKILL] }), + /e\.g\.|stable.*id|unique.*id/i + ) + }) + + it("reports empty detect by listing all available signal types", () => { + expectsError( + () => + validateTechnologyEntry({ id: "my-tech", name: "My Tech", detect: {}, skills: [VALID_SKILL] }), + /packages.*configFiles.*gems.*packagePatterns.*configFileContent/ + ) + }) + + it("reports empty detect.packages with an example array", () => { + expectsError( + () => + validateTechnologyEntry({ + id: "my-tech", + name: "My Tech", + detect: { packages: [] }, + skills: [VALID_SKILL], + }), + /empty|at least one/i + ) + }) + + it("reports non-RegExp in packagePatterns with usage hint", () => { + expectsError( + () => + validateTechnologyEntry({ + id: "my-tech", + name: "My Tech", + detect: { packagePatterns: ["not-a-regexp"] }, + skills: [VALID_SKILL], + }), + /RegExp|\/pattern\// + ) + }) + + it("reports invalid skill reference with expected format and example", () => { + expectsError( + () => + validateTechnologyEntry({ + id: "my-tech", + name: "My Tech", + detect: VALID_DETECT, + skills: ["not-valid"], + }), + /owner\/repo\/skill-name/ + ) + }) + + it("reports duplicate skills within an entry", () => { + expectsError( + () => + validateTechnologyEntry({ + id: "my-tech", + name: "My Tech", + detect: VALID_DETECT, + skills: [VALID_SKILL, VALID_SKILL], + }), + /duplicate/i + ) + }) + + it("reports unknown combo requires id with actionable message", () => { + const technologyIds = new Set(["react", "nextjs"]) + expectsError( + () => + validateComboEntry( + { + id: "my-combo", + name: "My Combo", + requires: ["react", "nonexistent-tech"], + skills: [VALID_SKILL], + }, + technologyIds + ), + /"nonexistent-tech".*SKILLS_MAP|SKILLS_MAP.*"nonexistent-tech"/ + ) + }) + + it("reports missing configFileContent.files with scanGradleLayout hint", () => { + expectsError( + () => + validateTechnologyEntry({ + id: "my-tech", + name: "My Tech", + detect: { configFileContent: { patterns: ["some-pattern"] } }, + skills: [VALID_SKILL], + }), + /files.*array|scanGradleLayout/ + ) + }) +}) From 80564883e3a830ea8c25ce8c356a263c8c999eff Mon Sep 17 00:00:00 2001 From: Antonio Bonet Date: Wed, 22 Apr 2026 01:12:31 +0700 Subject: [PATCH 2/5] chore: apply formatting fixes --- CONTRIBUTING.md | 42 +-- .../tests/skills-map.validation.test.mjs | 272 ++++++++++-------- 2 files changed, 170 insertions(+), 144 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 10ff016f..c9532ee5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -85,12 +85,12 @@ Add one entry to `SKILLS_MAP` in `packages/autoskills/skills-map.mjs`: Every entry requires these four fields: -| Field | Type | Description | -|-------|------|-------------| -| `id` | `string` | Stable, unique, lowercase. Use `-` as separator. | -| `name` | `string` | User-facing display name. | -| `detect` | `object` | At least one detection signal (see below). | -| `skills` | `string[]` | One or more skill references. | +| Field | Type | Description | +| -------- | ---------- | ------------------------------------------------ | +| `id` | `string` | Stable, unique, lowercase. Use `-` as separator. | +| `name` | `string` | User-facing display name. | +| `detect` | `object` | At least one detection signal (see below). | +| `skills` | `string[]` | One or more skill references. | #### Detection signals — choose the simplest one that works @@ -182,18 +182,26 @@ Common mistakes: ```js // ✗ packages is empty -detect: { packages: [] } +detect: { + packages: []; +} // ✗ skill ref is missing owner/repo -skills: ["react-best-practices"] +skills: ["react-best-practices"]; // ✗ packagePattern is a string, not a RegExp -detect: { packagePatterns: ["^@aws-sdk/"] } +detect: { + packagePatterns: ["^@aws-sdk/"]; +} // ✓ correct -detect: { packages: ["react"] } -skills: ["vercel-labs/agent-skills/vercel-react-best-practices"] -detect: { packagePatterns: [/^@aws-sdk\//] } +detect: { + packages: ["react"]; +} +skills: ["vercel-labs/agent-skills/vercel-react-best-practices"]; +detect: { + packagePatterns: [/^@aws-sdk\//]; +} ``` --- @@ -215,12 +223,12 @@ Add one entry to `COMBO_SKILLS_MAP` in `packages/autoskills/skills-map.mjs`: Add a combo only when it provides skills that are not already covered by the individual technology entries. A combo that just duplicates standalone skills adds noise. -| Field | Requirement | -|-------|-------------| -| `id` | Unique, lowercase, hyphen-separated. | -| `name` | User-facing, e.g. `"Next.js + Supabase"`. | +| Field | Requirement | +| ---------- | ----------------------------------------------- | +| `id` | Unique, lowercase, hyphen-separated. | +| `name` | User-facing, e.g. `"Next.js + Supabase"`. | | `requires` | Array of two or more existing `SKILLS_MAP` ids. | -| `skills` | At least one valid skill reference. | +| `skills` | At least one valid skill reference. | If you reference an id that doesn't exist in `SKILLS_MAP`, the test fails: diff --git a/packages/autoskills/tests/skills-map.validation.test.mjs b/packages/autoskills/tests/skills-map.validation.test.mjs index 4ed69947..c6a4fb17 100644 --- a/packages/autoskills/tests/skills-map.validation.test.mjs +++ b/packages/autoskills/tests/skills-map.validation.test.mjs @@ -1,240 +1,252 @@ -import { describe, it } from "node:test" -import { ok, strictEqual, throws } from "node:assert/strict" +import { describe, it } from "node:test"; +import { ok, strictEqual, throws } from "node:assert/strict"; -import { SKILLS_MAP, COMBO_SKILLS_MAP } from "../skills-map.mjs" +import { SKILLS_MAP, COMBO_SKILLS_MAP } from "../skills-map.mjs"; // ── Helpers ─────────────────────────────────────────────────── function findDuplicates(values) { - const counts = new Map() + const counts = new Map(); for (const value of values) { - counts.set(value, (counts.get(value) ?? 0) + 1) + counts.set(value, (counts.get(value) ?? 0) + 1); } return [...counts.entries()] .filter(([, count]) => count > 1) .map(([value]) => value) - .sort() + .sort(); } function isNonEmptyString(value) { - return typeof value === "string" && value.trim().length > 0 + return typeof value === "string" && value.trim().length > 0; } function isValidSkillRef(skill) { - if (/^https?:\/\/\S+$/u.test(skill)) return true + if (/^https?:\/\/\S+$/u.test(skill)) return true; - const parts = skill.split("/") - if (parts.length < 3) return false + const parts = skill.split("/"); + if (parts.length < 3) return false; - const [owner, repo, ...rest] = parts - return isNonEmptyString(owner) && isNonEmptyString(repo) && isNonEmptyString(rest.join("/")) + const [owner, repo, ...rest] = parts; + return isNonEmptyString(owner) && isNonEmptyString(repo) && isNonEmptyString(rest.join("/")); } function hasAtLeastOneDetectSignal(detect) { return Boolean( detect.packages?.length || - detect.packagePatterns?.length || - detect.configFiles?.length || - detect.gems?.length || - detect.configFileContent - ) + detect.packagePatterns?.length || + detect.configFiles?.length || + detect.gems?.length || + detect.configFileContent, + ); } function toConfigBlocks(value) { - if (!value) return [] - return Array.isArray(value) ? value : [value] + if (!value) return []; + return Array.isArray(value) ? value : [value]; } // ── Per-entry validators ────────────────────────────────────── function validateDetectConfig(tech) { - const { detect } = tech + const { detect } = tech; - ok(detect && typeof detect === "object", `[${tech.id}] detect must exist`) + ok(detect && typeof detect === "object", `[${tech.id}] detect must exist`); ok( hasAtLeastOneDetectSignal(detect), - `[${tech.id}] detect has no signals — define at least one of: packages, configFiles, gems, packagePatterns, or configFileContent` - ) + `[${tech.id}] detect has no signals — define at least one of: packages, configFiles, gems, packagePatterns, or configFileContent`, + ); if (detect.packages !== undefined) { - ok(Array.isArray(detect.packages), `[${tech.id}] detect.packages must be an array`) + ok(Array.isArray(detect.packages), `[${tech.id}] detect.packages must be an array`); ok( detect.packages.length > 0, - `[${tech.id}] detect.packages is empty — add at least one package name (e.g. ["react", "react-dom"])` - ) + `[${tech.id}] detect.packages is empty — add at least one package name (e.g. ["react", "react-dom"])`, + ); for (const pkg of detect.packages) { - ok(isNonEmptyString(pkg), `[${tech.id}] detect.packages contains an empty or non-string value`) + ok( + isNonEmptyString(pkg), + `[${tech.id}] detect.packages contains an empty or non-string value`, + ); } } if (detect.packagePatterns !== undefined) { - ok(Array.isArray(detect.packagePatterns), `[${tech.id}] detect.packagePatterns must be an array`) + ok( + Array.isArray(detect.packagePatterns), + `[${tech.id}] detect.packagePatterns must be an array`, + ); ok( detect.packagePatterns.length > 0, - `[${tech.id}] detect.packagePatterns is empty — add at least one RegExp (e.g. [/^@aws-sdk\\//])` - ) + `[${tech.id}] detect.packagePatterns is empty — add at least one RegExp (e.g. [/^@aws-sdk\\//])`, + ); for (const pattern of detect.packagePatterns) { ok( pattern instanceof RegExp, - `[${tech.id}] detect.packagePatterns contains a non-RegExp value — use /pattern/ syntax, not a string` - ) + `[${tech.id}] detect.packagePatterns contains a non-RegExp value — use /pattern/ syntax, not a string`, + ); } } if (detect.configFiles !== undefined) { - ok(Array.isArray(detect.configFiles), `[${tech.id}] detect.configFiles must be an array`) + ok(Array.isArray(detect.configFiles), `[${tech.id}] detect.configFiles must be an array`); ok( detect.configFiles.length > 0, - `[${tech.id}] detect.configFiles is empty — add at least one filename (e.g. ["next.config.js", "next.config.ts"])` - ) + `[${tech.id}] detect.configFiles is empty — add at least one filename (e.g. ["next.config.js", "next.config.ts"])`, + ); for (const file of detect.configFiles) { - ok(isNonEmptyString(file), `[${tech.id}] detect.configFiles contains an empty or non-string value`) + ok( + isNonEmptyString(file), + `[${tech.id}] detect.configFiles contains an empty or non-string value`, + ); } } if (detect.gems !== undefined) { - ok(Array.isArray(detect.gems), `[${tech.id}] detect.gems must be an array`) + ok(Array.isArray(detect.gems), `[${tech.id}] detect.gems must be an array`); ok( detect.gems.length > 0, - `[${tech.id}] detect.gems is empty — add at least one gem name (e.g. ["rails", "sinatra"])` - ) + `[${tech.id}] detect.gems is empty — add at least one gem name (e.g. ["rails", "sinatra"])`, + ); for (const gem of detect.gems) { - ok(isNonEmptyString(gem), `[${tech.id}] detect.gems contains an empty or non-string value`) + ok(isNonEmptyString(gem), `[${tech.id}] detect.gems contains an empty or non-string value`); } } for (const block of toConfigBlocks(detect.configFileContent)) { - ok(block && typeof block === "object", `[${tech.id}] configFileContent block must be an object`) - ok(Array.isArray(block.patterns), `[${tech.id}] configFileContent.patterns must be an array`) + ok( + block && typeof block === "object", + `[${tech.id}] configFileContent block must be an object`, + ); + ok(Array.isArray(block.patterns), `[${tech.id}] configFileContent.patterns must be an array`); ok( block.patterns.length > 0, - `[${tech.id}] configFileContent.patterns is empty — add at least one string pattern to search for` - ) + `[${tech.id}] configFileContent.patterns is empty — add at least one string pattern to search for`, + ); for (const pattern of block.patterns) { ok( isNonEmptyString(pattern), - `[${tech.id}] configFileContent.patterns contains an empty or non-string value` - ) + `[${tech.id}] configFileContent.patterns contains an empty or non-string value`, + ); } if (!block.scanGradleLayout) { ok( Array.isArray(block.files), - `[${tech.id}] configFileContent.files must be an array (or set scanGradleLayout: true for Gradle projects)` - ) + `[${tech.id}] configFileContent.files must be an array (or set scanGradleLayout: true for Gradle projects)`, + ); ok( block.files.length > 0, - `[${tech.id}] configFileContent.files is empty — add at least one filename to search in` - ) + `[${tech.id}] configFileContent.files is empty — add at least one filename to search in`, + ); for (const file of block.files) { ok( isNonEmptyString(file), - `[${tech.id}] configFileContent.files contains an empty or non-string value` - ) + `[${tech.id}] configFileContent.files contains an empty or non-string value`, + ); } } } } function validateSkillList(ownerId, skills) { - ok(Array.isArray(skills), `[${ownerId}] skills must be an array`) + ok(Array.isArray(skills), `[${ownerId}] skills must be an array`); for (const skill of skills) { - ok(isNonEmptyString(skill), `[${ownerId}] skills contains an empty or non-string value`) + ok(isNonEmptyString(skill), `[${ownerId}] skills contains an empty or non-string value`); ok( isValidSkillRef(skill), - `[${ownerId}] skill "${skill}" is not a valid reference — use "owner/repo/skill-name" (e.g. "vercel-labs/agent-skills/vercel-react-best-practices") or an https:// URL` - ) + `[${ownerId}] skill "${skill}" is not a valid reference — use "owner/repo/skill-name" (e.g. "vercel-labs/agent-skills/vercel-react-best-practices") or an https:// URL`, + ); } - const duplicateSkills = findDuplicates(skills) + const duplicateSkills = findDuplicates(skills); strictEqual( duplicateSkills.length, 0, - `[${ownerId}] duplicate skill references: ${duplicateSkills.join(", ")} — each skill can appear only once per entry` - ) + `[${ownerId}] duplicate skill references: ${duplicateSkills.join(", ")} — each skill can appear only once per entry`, + ); } function validateTechnologyEntry(tech) { ok( isNonEmptyString(tech.id), - `technology.id is missing or empty — every SKILLS_MAP entry needs a stable unique id (e.g. "react", "nextjs", "prisma")` - ) + `technology.id is missing or empty — every SKILLS_MAP entry needs a stable unique id (e.g. "react", "nextjs", "prisma")`, + ); ok( isNonEmptyString(tech.name), - `[${tech.id ?? "?"}] name is missing or empty — add a user-facing display name (e.g. "React", "Next.js")` - ) - validateDetectConfig(tech) - validateSkillList(tech.id, tech.skills) + `[${tech.id ?? "?"}] name is missing or empty — add a user-facing display name (e.g. "React", "Next.js")`, + ); + validateDetectConfig(tech); + validateSkillList(tech.id, tech.skills); } function validateComboEntry(combo, technologyIds) { ok( isNonEmptyString(combo.id), - `combo.id is missing or empty — every COMBO_SKILLS_MAP entry needs a stable unique id (e.g. "nextjs-supabase")` - ) + `combo.id is missing or empty — every COMBO_SKILLS_MAP entry needs a stable unique id (e.g. "nextjs-supabase")`, + ); ok( isNonEmptyString(combo.name), - `[${combo.id ?? "?"}] name is missing or empty — add a user-facing name (e.g. "Next.js + Supabase")` - ) + `[${combo.id ?? "?"}] name is missing or empty — add a user-facing name (e.g. "Next.js + Supabase")`, + ); - ok(Array.isArray(combo.requires), `[${combo.id}] requires must be an array`) + ok(Array.isArray(combo.requires), `[${combo.id}] requires must be an array`); ok( combo.requires.length >= 2, - `[${combo.id}] requires must list at least two technology ids — combos need a minimum of two technologies` - ) + `[${combo.id}] requires must list at least two technology ids — combos need a minimum of two technologies`, + ); for (const requiredId of combo.requires) { - ok(isNonEmptyString(requiredId), `[${combo.id}] requires contains an empty or non-string id`) + ok(isNonEmptyString(requiredId), `[${combo.id}] requires contains an empty or non-string id`); ok( technologyIds.has(requiredId), - `[${combo.id}] requires "${requiredId}" which is not in SKILLS_MAP — fix the typo or add the technology first` - ) + `[${combo.id}] requires "${requiredId}" which is not in SKILLS_MAP — fix the typo or add the technology first`, + ); } - const duplicateRequires = findDuplicates(combo.requires) + const duplicateRequires = findDuplicates(combo.requires); strictEqual( duplicateRequires.length, 0, - `[${combo.id}] duplicate ids in requires: ${duplicateRequires.join(", ")} — list each technology id once` - ) + `[${combo.id}] duplicate ids in requires: ${duplicateRequires.join(", ")} — list each technology id once`, + ); - validateSkillList(combo.id, combo.skills) + validateSkillList(combo.id, combo.skills); } // ── SKILLS_MAP ──────────────────────────────────────────────── describe("SKILLS_MAP validation", () => { it("has unique technology ids", () => { - const duplicates = findDuplicates(SKILLS_MAP.map((tech) => tech.id)) - strictEqual(duplicates.length, 0, `duplicate technology ids: ${duplicates.join(", ")}`) - }) + const duplicates = findDuplicates(SKILLS_MAP.map((tech) => tech.id)); + strictEqual(duplicates.length, 0, `duplicate technology ids: ${duplicates.join(", ")}`); + }); it("validates every technology entry shape", () => { for (const tech of SKILLS_MAP) { - validateTechnologyEntry(tech) + validateTechnologyEntry(tech); } - }) -}) + }); +}); // ── COMBO_SKILLS_MAP ────────────────────────────────────────── describe("COMBO_SKILLS_MAP validation", () => { - const technologyIds = new Set(SKILLS_MAP.map((tech) => tech.id)) + const technologyIds = new Set(SKILLS_MAP.map((tech) => tech.id)); it("has unique combo ids", () => { - const duplicates = findDuplicates(COMBO_SKILLS_MAP.map((combo) => combo.id)) - strictEqual(duplicates.length, 0, `duplicate combo ids: ${duplicates.join(", ")}`) - }) + const duplicates = findDuplicates(COMBO_SKILLS_MAP.map((combo) => combo.id)); + strictEqual(duplicates.length, 0, `duplicate combo ids: ${duplicates.join(", ")}`); + }); it("validates every combo entry shape", () => { for (const combo of COMBO_SKILLS_MAP) { - validateComboEntry(combo, technologyIds) + validateComboEntry(combo, technologyIds); } - }) -}) + }); +}); // ── Feedback quality: error messages must be actionable ─────── // @@ -242,33 +254,39 @@ describe("COMBO_SKILLS_MAP validation", () => { // the assertion message must tell them exactly what to fix. describe("validation gives actionable feedback on bad entries", () => { - const VALID_SKILL = "owner/repo/skill-name" - const VALID_DETECT = { packages: ["some-package"] } + const VALID_SKILL = "owner/repo/skill-name"; + const VALID_DETECT = { packages: ["some-package"] }; function expectsError(fn, pattern) { throws(fn, (err) => { ok( pattern.test(err.message), - `\nExpected error message matching: ${pattern}\nGot: "${err.message}"` - ) - return true - }) + `\nExpected error message matching: ${pattern}\nGot: "${err.message}"`, + ); + return true; + }); } it("reports missing technology id with a usage hint", () => { expectsError( - () => validateTechnologyEntry({ name: "My Tech", detect: VALID_DETECT, skills: [VALID_SKILL] }), - /e\.g\.|stable.*id|unique.*id/i - ) - }) + () => + validateTechnologyEntry({ name: "My Tech", detect: VALID_DETECT, skills: [VALID_SKILL] }), + /e\.g\.|stable.*id|unique.*id/i, + ); + }); it("reports empty detect by listing all available signal types", () => { expectsError( () => - validateTechnologyEntry({ id: "my-tech", name: "My Tech", detect: {}, skills: [VALID_SKILL] }), - /packages.*configFiles.*gems.*packagePatterns.*configFileContent/ - ) - }) + validateTechnologyEntry({ + id: "my-tech", + name: "My Tech", + detect: {}, + skills: [VALID_SKILL], + }), + /packages.*configFiles.*gems.*packagePatterns.*configFileContent/, + ); + }); it("reports empty detect.packages with an example array", () => { expectsError( @@ -279,9 +297,9 @@ describe("validation gives actionable feedback on bad entries", () => { detect: { packages: [] }, skills: [VALID_SKILL], }), - /empty|at least one/i - ) - }) + /empty|at least one/i, + ); + }); it("reports non-RegExp in packagePatterns with usage hint", () => { expectsError( @@ -292,9 +310,9 @@ describe("validation gives actionable feedback on bad entries", () => { detect: { packagePatterns: ["not-a-regexp"] }, skills: [VALID_SKILL], }), - /RegExp|\/pattern\// - ) - }) + /RegExp|\/pattern\//, + ); + }); it("reports invalid skill reference with expected format and example", () => { expectsError( @@ -305,9 +323,9 @@ describe("validation gives actionable feedback on bad entries", () => { detect: VALID_DETECT, skills: ["not-valid"], }), - /owner\/repo\/skill-name/ - ) - }) + /owner\/repo\/skill-name/, + ); + }); it("reports duplicate skills within an entry", () => { expectsError( @@ -318,12 +336,12 @@ describe("validation gives actionable feedback on bad entries", () => { detect: VALID_DETECT, skills: [VALID_SKILL, VALID_SKILL], }), - /duplicate/i - ) - }) + /duplicate/i, + ); + }); it("reports unknown combo requires id with actionable message", () => { - const technologyIds = new Set(["react", "nextjs"]) + const technologyIds = new Set(["react", "nextjs"]); expectsError( () => validateComboEntry( @@ -333,11 +351,11 @@ describe("validation gives actionable feedback on bad entries", () => { requires: ["react", "nonexistent-tech"], skills: [VALID_SKILL], }, - technologyIds + technologyIds, ), - /"nonexistent-tech".*SKILLS_MAP|SKILLS_MAP.*"nonexistent-tech"/ - ) - }) + /"nonexistent-tech".*SKILLS_MAP|SKILLS_MAP.*"nonexistent-tech"/, + ); + }); it("reports missing configFileContent.files with scanGradleLayout hint", () => { expectsError( @@ -348,7 +366,7 @@ describe("validation gives actionable feedback on bad entries", () => { detect: { configFileContent: { patterns: ["some-pattern"] } }, skills: [VALID_SKILL], }), - /files.*array|scanGradleLayout/ - ) - }) -}) + /files.*array|scanGradleLayout/, + ); + }); +}); From cc2e9d62e0b0acdc7a236b1387cbb62959faf270 Mon Sep 17 00:00:00 2001 From: Antonio Bonet Date: Wed, 22 Apr 2026 01:18:19 +0700 Subject: [PATCH 3/5] chore: apply formatting fixes --- packages/autoskills/skills-map.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/autoskills/skills-map.ts b/packages/autoskills/skills-map.ts index 55348b77..061c91e7 100644 --- a/packages/autoskills/skills-map.ts +++ b/packages/autoskills/skills-map.ts @@ -788,7 +788,7 @@ export const SKILLS_MAP: Technology[] = [ skills: [ "github/awesome-copilot/dotnet-best-practices", "github/awesome-copilot/dotnet-design-pattern-review", - "github/awesome-copilot/dotnet-upgrade" + "github/awesome-copilot/dotnet-upgrade", ], }, { @@ -819,10 +819,7 @@ export const SKILLS_MAP: Technology[] = [ patterns: ["Microsoft.NET.Sdk.Web"], }, }, - skills: [ - "github/awesome-copilot/containerize-aspnetcore", - "openai/skills/aspnet-core", - ], + skills: ["github/awesome-copilot/containerize-aspnetcore", "openai/skills/aspnet-core"], }, { id: "aspnet-blazor", @@ -833,9 +830,7 @@ export const SKILLS_MAP: Technology[] = [ patterns: ["Microsoft.NET.Sdk.BlazorWebAssembly", "Microsoft.AspNetCore.Components"], }, }, - skills: [ - "github/awesome-copilot/fluentui-blazor" - ], + skills: ["github/awesome-copilot/fluentui-blazor"], }, { id: "aspnet-minimal-api", @@ -849,7 +844,7 @@ export const SKILLS_MAP: Technology[] = [ }, skills: [ "github/awesome-copilot/aspnet-minimal-api-openapi", - "dotnet/skills/minimal-api-file-upload" + "dotnet/skills/minimal-api-file-upload", ], }, { From 827a721dfa4afae4efac20d50912812e39682019 Mon Sep 17 00:00:00 2001 From: Antonio Bonet Date: Wed, 29 Apr 2026 21:01:21 +0700 Subject: [PATCH 4/5] refactor(tests): convert skills-map validation to TypeScript and update CONTRIBUTING.md references --- CONTRIBUTING.md | 58 ++++++++------- ...test.mjs => skills-map.validation.test.ts} | 72 ++++++++++--------- 2 files changed, 72 insertions(+), 58 deletions(-) rename packages/autoskills/tests/{skills-map.validation.test.mjs => skills-map.validation.test.ts} (85%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c9532ee5..b1613a75 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,13 +22,23 @@ git checkout -b docs/improve-contributing-guide Make the smallest change that fully solves the problem. Ask for help in the PR — you don't need to wait until it's perfect. -## 4. Follow the style +## 4. Install Git hooks -- Lint and format before pushing -- Add or update tests for every behavior change -- Make sure all tests pass locally before opening a PR +Before committing, install the repository hooks: -## 5. Open a pull request +```sh +pnpm run hooks:install +``` + +This configures Git to use the versioned hooks in `.githooks/`. The pre-commit hook formats staged files and adds the formatted result to the commit automatically. + +## 5. Follow the style + +- Lint and format before pushing. +- Add or update tests for every behavior change. +- Make sure all tests pass locally before opening a PR. + +## 6. Open a pull request Sync with main before pushing: @@ -43,7 +53,7 @@ git push --set-upstream origin your-branch Keep the PR title short and in the imperative mood: `feat: add Bun detection`, `fix: empty gems array in Ruby entry`, `test: validate skills map entries`. -## 6. Keep your PR up to date +## 7. Keep your PR up to date When a maintainer asks you to rebase, the base branch has moved. Update your branch: @@ -53,7 +63,7 @@ git rebase upstream/main git push --force-with-lease ``` -## 7. AutoSkills-specific contributions +## 8. AutoSkills-specific contributions Most AutoSkills changes fall into one of these: @@ -68,9 +78,9 @@ Most AutoSkills changes fall into one of these: ### Adding a technology -Add one entry to `SKILLS_MAP` in `packages/autoskills/skills-map.mjs`: +Add one entry to `SKILLS_MAP` in `packages/autoskills/skills-map.ts`: -```js +```ts { id: "bun", name: "Bun", @@ -98,7 +108,7 @@ Use signals in this order of preference: **1. `packages`** — match npm/pip/cargo package names -```js +```ts detect: { packages: ["react", "react-dom"], } @@ -106,7 +116,7 @@ detect: { **2. `configFiles`** — match specific filenames in the project root -```js +```ts detect: { configFiles: ["next.config.js", "next.config.mjs", "next.config.ts"], } @@ -114,7 +124,7 @@ detect: { **3. `gems`** — match Ruby gem names -```js +```ts detect: { gems: ["rails", "sinatra"], } @@ -122,7 +132,7 @@ detect: { **4. `packagePatterns`** — match packages by RegExp when a prefix covers multiple packages -```js +```ts detect: { packagePatterns: [/^@aws-sdk\//, /^aws-cdk/], } @@ -130,7 +140,7 @@ detect: { **5. `configFileContent`** — search file contents when names and packages are not enough -```js +```ts // Standard form — search inside specific files detect: { configFileContent: { @@ -150,7 +160,7 @@ detect: { You can combine signals — the entry matches when any signal is found: -```js +```ts detect: { packages: ["tailwindcss"], configFiles: ["tailwind.config.js", "tailwind.config.ts"], @@ -180,7 +190,7 @@ Run the tests and you'll see exactly what needs fixing: Common mistakes: -```js +```ts // ✗ packages is empty detect: { packages: []; @@ -208,9 +218,9 @@ detect: { ### Adding a combo -Add one entry to `COMBO_SKILLS_MAP` in `packages/autoskills/skills-map.mjs`: +Add one entry to `COMBO_SKILLS_MAP` in `packages/autoskills/skills-map.ts`: -```js +```ts { id: "nextjs-supabase", name: "Next.js + Supabase", @@ -242,7 +252,7 @@ If you reference an id that doesn't exist in `SKILLS_MAP`, the test fails: ```sh cd packages/autoskills -node --test 'tests/*.test.mjs' +node --test 'tests/*.test.ts' ``` Expected output when everything is correct: @@ -260,15 +270,15 @@ Expected output when everything is correct: #### Adding a technology -- [ ] One entry added to `SKILLS_MAP` in `packages/autoskills/skills-map.mjs` +- [ ] One entry added to `SKILLS_MAP` in `packages/autoskills/skills-map.ts` - [ ] Detection uses the simplest reliable signal - [ ] All skill refs follow the `owner/repo/skill-name` or `https://` format -- [ ] Tests pass locally (`node --test 'tests/*.test.mjs'`) +- [ ] Tests pass locally (`node --test 'tests/*.test.ts'`) - [ ] PR title uses the imperative mood: `feat: add Bun detection` #### Adding a combo -- [ ] One entry added to `COMBO_SKILLS_MAP` in `packages/autoskills/skills-map.mjs` +- [ ] One entry added to `COMBO_SKILLS_MAP` in `packages/autoskills/skills-map.ts` - [ ] `requires` contains at least two ids that exist in `SKILLS_MAP` - [ ] The combo adds skills not already covered by the individual entries - [ ] Tests pass locally @@ -276,8 +286,8 @@ Expected output when everything is correct: #### Fixing detection -- [ ] Existing entry updated in `packages/autoskills/skills-map.mjs` -- [ ] A test in `packages/autoskills/tests/detect.test.mjs` covers the fixed case +- [ ] Existing entry updated in `packages/autoskills/skills-map.ts` +- [ ] A test in `packages/autoskills/tests/detect.test.ts` covers the fixed case - [ ] Tests pass locally - [ ] PR title describes the fix: `fix: detect Tailwind from @tailwindcss/vite` diff --git a/packages/autoskills/tests/skills-map.validation.test.mjs b/packages/autoskills/tests/skills-map.validation.test.ts similarity index 85% rename from packages/autoskills/tests/skills-map.validation.test.mjs rename to packages/autoskills/tests/skills-map.validation.test.ts index c6a4fb17..e2e4e1f8 100644 --- a/packages/autoskills/tests/skills-map.validation.test.mjs +++ b/packages/autoskills/tests/skills-map.validation.test.ts @@ -1,55 +1,57 @@ import { describe, it } from "node:test"; import { ok, strictEqual, throws } from "node:assert/strict"; - -import { SKILLS_MAP, COMBO_SKILLS_MAP } from "../skills-map.mjs"; +import { SKILLS_MAP, COMBO_SKILLS_MAP } from "../skills-map.ts"; +import type { Technology, ComboSkill, DetectConfig } from "../skills-map.ts"; // ── Helpers ─────────────────────────────────────────────────── -function findDuplicates(values) { - const counts = new Map(); - +function findDuplicates(values: string[]): string[] { + const counts = new Map(); for (const value of values) { counts.set(value, (counts.get(value) ?? 0) + 1); } - return [...counts.entries()] .filter(([, count]) => count > 1) .map(([value]) => value) .sort(); } -function isNonEmptyString(value) { +function isNonEmptyString(value: unknown): value is string { return typeof value === "string" && value.trim().length > 0; } -function isValidSkillRef(skill) { +function isValidSkillRef(skill: string): boolean { if (/^https?:\/\/\S+$/u.test(skill)) return true; - const parts = skill.split("/"); if (parts.length < 3) return false; - const [owner, repo, ...rest] = parts; - return isNonEmptyString(owner) && isNonEmptyString(repo) && isNonEmptyString(rest.join("/")); + return ( + isNonEmptyString(owner) && + isNonEmptyString(repo) && + isNonEmptyString(rest.join("/")) + ); } -function hasAtLeastOneDetectSignal(detect) { +function hasAtLeastOneDetectSignal(detect: DetectConfig): boolean { return Boolean( detect.packages?.length || - detect.packagePatterns?.length || - detect.configFiles?.length || - detect.gems?.length || - detect.configFileContent, + detect.packagePatterns?.length || + detect.configFiles?.length || + detect.gems?.length || + detect.configFileContent, ); } -function toConfigBlocks(value) { +function toConfigBlocks( + value: DetectConfig["configFileContent"], +): Array> { if (!value) return []; return Array.isArray(value) ? value : [value]; } // ── Per-entry validators ────────────────────────────────────── -function validateDetectConfig(tech) { +function validateDetectConfig(tech: Technology): void { const { detect } = tech; ok(detect && typeof detect === "object", `[${tech.id}] detect must exist`); @@ -115,10 +117,7 @@ function validateDetectConfig(tech) { } for (const block of toConfigBlocks(detect.configFileContent)) { - ok( - block && typeof block === "object", - `[${tech.id}] configFileContent block must be an object`, - ); + ok(block && typeof block === "object", `[${tech.id}] configFileContent block must be an object`); ok(Array.isArray(block.patterns), `[${tech.id}] configFileContent.patterns must be an array`); ok( block.patterns.length > 0, @@ -131,10 +130,10 @@ function validateDetectConfig(tech) { ); } - if (!block.scanGradleLayout) { + if (!block.scanGradleLayout && !block.scanDotNetLayout) { ok( Array.isArray(block.files), - `[${tech.id}] configFileContent.files must be an array (or set scanGradleLayout: true for Gradle projects)`, + `[${tech.id}] configFileContent.files must be an array (or set scanGradleLayout: true for Gradle projects, scanDotNetLayout: true for .NET projects)`, ); ok( block.files.length > 0, @@ -150,7 +149,7 @@ function validateDetectConfig(tech) { } } -function validateSkillList(ownerId, skills) { +function validateSkillList(ownerId: string, skills: string[]): void { ok(Array.isArray(skills), `[${ownerId}] skills must be an array`); for (const skill of skills) { @@ -169,7 +168,7 @@ function validateSkillList(ownerId, skills) { ); } -function validateTechnologyEntry(tech) { +function validateTechnologyEntry(tech: Technology): void { ok( isNonEmptyString(tech.id), `technology.id is missing or empty — every SKILLS_MAP entry needs a stable unique id (e.g. "react", "nextjs", "prisma")`, @@ -182,7 +181,7 @@ function validateTechnologyEntry(tech) { validateSkillList(tech.id, tech.skills); } -function validateComboEntry(combo, technologyIds) { +function validateComboEntry(combo: ComboSkill, technologyIds: Set): void { ok( isNonEmptyString(combo.id), `combo.id is missing or empty — every COMBO_SKILLS_MAP entry needs a stable unique id (e.g. "nextjs-supabase")`, @@ -255,13 +254,14 @@ describe("COMBO_SKILLS_MAP validation", () => { describe("validation gives actionable feedback on bad entries", () => { const VALID_SKILL = "owner/repo/skill-name"; - const VALID_DETECT = { packages: ["some-package"] }; + const VALID_DETECT: DetectConfig = { packages: ["some-package"] }; - function expectsError(fn, pattern) { - throws(fn, (err) => { + function expectsError(fn: () => void, pattern: RegExp): void { + throws(fn, (err: unknown) => { + const message = err instanceof Error ? err.message : String(err); ok( - pattern.test(err.message), - `\nExpected error message matching: ${pattern}\nGot: "${err.message}"`, + pattern.test(message), + `\nExpected error message matching: ${pattern}\nGot: "${message}"`, ); return true; }); @@ -270,7 +270,11 @@ describe("validation gives actionable feedback on bad entries", () => { it("reports missing technology id with a usage hint", () => { expectsError( () => - validateTechnologyEntry({ name: "My Tech", detect: VALID_DETECT, skills: [VALID_SKILL] }), + validateTechnologyEntry({ + name: "My Tech", + detect: VALID_DETECT, + skills: [VALID_SKILL], + } as Technology), /e\.g\.|stable.*id|unique.*id/i, ); }); @@ -307,7 +311,7 @@ describe("validation gives actionable feedback on bad entries", () => { validateTechnologyEntry({ id: "my-tech", name: "My Tech", - detect: { packagePatterns: ["not-a-regexp"] }, + detect: { packagePatterns: ["not-a-regexp"] as unknown as RegExp[] }, skills: [VALID_SKILL], }), /RegExp|\/pattern\//, From 1983ea503feaecb50355e1ac1ea6e0ba7c84b902 Mon Sep 17 00:00:00 2001 From: Antonio Bonet Date: Wed, 29 Apr 2026 21:04:11 +0700 Subject: [PATCH 5/5] fix(skills-map): remove duplicate python, django, and fastapi entries --- packages/autoskills/skills-map.ts | 38 ++++--------------------------- 1 file changed, 4 insertions(+), 34 deletions(-) diff --git a/packages/autoskills/skills-map.ts b/packages/autoskills/skills-map.ts index d39c0416..c18fbf41 100644 --- a/packages/autoskills/skills-map.ts +++ b/packages/autoskills/skills-map.ts @@ -891,14 +891,6 @@ export const SKILLS_MAP: Technology[] = [ }, skills: [], }, - { - id: "python", - name: "Python", - detect: { - configFiles: ["pyproject.toml", "requirements.txt", "setup.py", "setup.cfg", "Pipfile"], - }, - skills: [], - }, { id: "sorbet", name: "Sorbet", @@ -919,18 +911,6 @@ export const SKILLS_MAP: Technology[] = [ }, skills: [], }, - { - id: "django", - name: "Django", - detect: { - configFiles: ["manage.py"], - configFileContent: { - files: ["pyproject.toml", "requirements.txt", "setup.py", "setup.cfg", "Pipfile"], - patterns: ["django", "Django"], - }, - }, - skills: [], - }, { id: "devise", name: "Devise", @@ -939,17 +919,6 @@ export const SKILLS_MAP: Technology[] = [ }, skills: [], }, - { - id: "fastapi", - name: "FastAPI", - detect: { - configFileContent: { - files: ["pyproject.toml", "requirements.txt", "setup.py", "setup.cfg", "Pipfile"], - patterns: ["fastapi", "FastAPI"], - }, - }, - skills: [], - }, { id: "sidekiq", name: "Sidekiq", @@ -1007,7 +976,7 @@ export const SKILLS_MAP: Technology[] = [ id: "python", name: "Python", detect: { - configFiles: ["pyproject.toml", "requirements.txt", "setup.py", "Pipfile"], + configFiles: ["pyproject.toml", "requirements.txt", "setup.py", "setup.cfg", "Pipfile"], }, skills: ["inferen-sh/skills/python-executor", "wshobson/agents/python-testing-patterns"], }, @@ -1016,7 +985,7 @@ export const SKILLS_MAP: Technology[] = [ name: "FastAPI", detect: { configFileContent: { - files: ["pyproject.toml", "requirements.txt", "setup.py", "Pipfile"], + files: ["pyproject.toml", "requirements.txt", "setup.py", "setup.cfg", "Pipfile"], patterns: ["fastapi", "FastAPI"], }, }, @@ -1026,8 +995,9 @@ export const SKILLS_MAP: Technology[] = [ id: "django", name: "Django", detect: { + configFiles: ["manage.py"], configFileContent: { - files: ["pyproject.toml", "requirements.txt", "setup.py", "Pipfile"], + files: ["pyproject.toml", "requirements.txt", "setup.py", "setup.cfg", "Pipfile"], patterns: ["django", "Django"], }, },