tests(skills E2E): drive checks from skills/ as the source of truth#717
Conversation
…g skill to manifest Agent-Logs-Url: https://github.com/ClickHouse/clickhouse-js/sessions/760e6c33-bd92-406b-bac0-e4e1aa20a31d Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
…er check Agent-Logs-Url: https://github.com/ClickHouse/clickhouse-js/sessions/760e6c33-bd92-406b-bac0-e4e1aa20a31d Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
|
copilot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
@copilot resolve the merge conflicts in this pull request |
…ts-for-skills # Conflicts: # tests/e2e/skills/check.js Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
Resolved in 346e342. The only conflict was in |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Improves the Skills E2E packaging check to treat the repo-root skills/ directory as the source of truth, and validates that the installed @clickhouse/client package’s agents.skills manifest and skills-npm symlinks match what’s in skills/.
Changes:
- Discover expected skills by scanning
skills/for subdirectories containingSKILL.md. - Assert
@clickhouse/client’sagents.skillsmatches that discovered set and each entry resolves to an installed skill containingSKILL.md. - Generalize the
skills-npmsymlink checks to validate symlinks for every declared skill (instead of a single hardcoded skill), and keep@clickhouse/client-webskill-free.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Address unresolved review comments from #714 on
tests/e2e/skills/check.js: the hardcoded skill list and single-skill symlink assertion let a missing/misdeclared skill (or wrongpath) slip through. Make the repo-rootskills/directory the source of truth and verify the fullagents.skillsmanifest.packages/client-node/package.json— addclickhouse-js-node-codingtoagents.skillsso both shipped skills are discoverable. (Also landed independently onmainvia docs(AGENTS.md): refresh based on last 4 weeks of PRs #714 and reconciled during the merge.)tests/e2e/skills/check.js— discover skills fromskills/(subdirs containingSKILL.md) and assert, for the installed@clickhouse/clienttarball:agents.skillsdeclares exactly that set (sorted-namedeepStrictEqual— catches both missing and stale entries).path === ./skills/<name>, the path resolves inside the installed package, and containsSKILL.md..claude/skills/byskills-npmand the symlink target containsSKILL.md(no longer hardcoded toclickhouse-js-node-troubleshooting).@clickhouse/client-webstill ships noskills/dir.skills/is missing;npm-*link list is re-read inside each check to avoid stale snapshots.mainand resolved a conflict intests/e2e/skills/check.jsby keeping this PR's source-of-truth-driven implementation over the simpler hardcodednodeSkillsarray introduced by docs(AGENTS.md): refresh based on last 4 weeks of PRs #714.Checklist