From 6c2a1137d3cedc5468d0855eeb1e9f07d2072a1b Mon Sep 17 00:00:00 2001 From: James Date: Thu, 25 Jun 2026 19:01:40 +0000 Subject: [PATCH] ci(skills): run skills/**/*.test.mjs in CI skills/**/*.test.mjs files (e.g. skills/media-use/scripts/resolve.test.mjs and skills/media-use/scripts/lib/manifest.test.mjs) are bare `node --test` files with only `node:` built-in imports. They aren't part of any workspace package, and the existing `Test` job's path filter (the `code` filter in the `changes` job) excludes `skills/**`, so even on PRs that touch only skills/ those tests never run. This matters for regression guards. The shell-injection probe test added in HF#1723 feeds probe() a filename containing `clip"; touch INJECTED; echo ".mp4` and asserts no marker file is created. The test passes locally but under the current job graph it would never run in CI on a follow-up skills/ change that re-introduces the bug. Closing the gap with a dedicated `Test: skills` job rather than relaxing the `code` filter. The existing `Test` job's steps run `bun run test:scripts` (hardcoded file list) and `bun run --filter '*' test` (workspace packages only), neither of which would actually execute skills tests even if the filter let `skills/**` through. The dedicated job needs no `bun install`, just node 22, since the tests only import from `node:` and relative paths. The discovery step shells out to `find` and fails loudly when zero test files match, so a future rename or layout change can't silently turn this into a no-op pass. Spotted by Via in HF#1723 review thread, confirmed by James as a separate follow-up rather than a blocker for HF#1723. -- Jerrai (https://claude.com/claude-code) --- .github/workflows/ci.yml | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 464fb00c9f..69221d5fd1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -229,11 +229,16 @@ jobs: - run: bun run --cwd packages/core build:hyperframes-runtime - run: bun run --filter '!@hyperframes/producer' test - # Runs the skills' own node:test suites (e.g. media-use), which live outside - # the workspace packages and are not covered by the `test` job above. Gated on - # the `skills` filter so a skills-only PR actually exercises them in CI. + # Tests under skills/**/*.test.mjs are bare `node --test` files with only + # `node:` built-in imports. They aren't part of any workspace package, and + # the main `Test` job's `code` path filter excludes `skills/**`, so without + # this dedicated job they'd never run in CI. Examples: + # * skills/media-use/scripts/resolve.test.mjs + # * skills/media-use/scripts/lib/manifest.test.mjs + # Several of these are regression guards (e.g. shell-injection cases), so + # the whole point is that they fire on PRs that touch skills/. test-skills: - name: Test (skills) + name: "Test: skills" needs: changes if: needs.changes.outputs.skills == 'true' runs-on: ubuntu-latest @@ -243,7 +248,20 @@ jobs: - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22 - - run: node --test 'skills/**/*.test.mjs' + - name: Discover and run skills tests + # We expand the test list via bash so the job fails loudly when the + # matcher comes back empty, rather than silently no-op'ing (which + # would defeat the whole point of this job). + run: | + set -euo pipefail + mapfile -t SKILLS_TESTS < <(find skills -type f -name "*.test.mjs" | sort) + if [ "${#SKILLS_TESTS[@]}" -eq 0 ]; then + echo "::error::No skills/**/*.test.mjs files found. Did the layout change?" + exit 1 + fi + printf 'Running %d skills test file(s):\n' "${#SKILLS_TESTS[@]}" + printf ' * %s\n' "${SKILLS_TESTS[@]}" + node --test "${SKILLS_TESTS[@]}" cli-npx-shim: name: "CLI: npx shim (${{ matrix.os }})"