diff --git a/.github/rulesets/main-protection.json b/.github/rulesets/main-protection.json index d18310f90..00d41cbfd 100644 --- a/.github/rulesets/main-protection.json +++ b/.github/rulesets/main-protection.json @@ -34,16 +34,14 @@ "parameters": { "strict_required_status_checks_policy": true, "required_status_checks": [ - { "context": "lint-tests" }, - { "context": "test (ubuntu-latest, 22)" }, - { "context": "test (macos-latest, 22)" }, - { "context": "test (windows-latest, 22)" }, - { "context": "test (ubuntu-latest, 24)" }, - { "context": "test (macos-latest, 24)" }, - { "context": "test (windows-latest, 24)" }, - { "context": "Changeset Required" }, - { "context": "require-issue-link" }, - { "context": "pr-template-format" } + { "context": "Required tests" }, + { "context": "size-check" }, + { "context": "check-branch" }, + { "context": "changeset-lint" }, + { "context": "docs-lint" }, + { "context": "validate-target" }, + { "context": "Issue link required" }, + { "context": "Pull request template format" } ] } } diff --git a/.github/rulesets/release-branches.json b/.github/rulesets/release-branches.json index 36caa35a6..28e0b24de 100644 --- a/.github/rulesets/release-branches.json +++ b/.github/rulesets/release-branches.json @@ -31,16 +31,14 @@ "parameters": { "strict_required_status_checks_policy": true, "required_status_checks": [ - { "context": "lint-tests" }, - { "context": "test (ubuntu-latest, 22)" }, - { "context": "test (macos-latest, 22)" }, - { "context": "test (windows-latest, 22)" }, - { "context": "test (ubuntu-latest, 24)" }, - { "context": "test (macos-latest, 24)" }, - { "context": "test (windows-latest, 24)" }, - { "context": "Changeset Required" }, - { "context": "require-issue-link" }, - { "context": "pr-template-format" } + { "context": "Required tests" }, + { "context": "size-check" }, + { "context": "check-branch" }, + { "context": "changeset-lint" }, + { "context": "docs-lint" }, + { "context": "validate-target" }, + { "context": "Issue link required" }, + { "context": "Pull request template format" } ] } } diff --git a/.github/workflows/install-smoke.yml b/.github/workflows/install-smoke.yml index a30c2c660..3708afa7b 100644 --- a/.github/workflows/install-smoke.yml +++ b/.github/workflows/install-smoke.yml @@ -16,16 +16,19 @@ name: Install Smoke on: pull_request: branches: + - next - main paths: - 'bin/install.js' - 'get-shit-done/bin/gsd-tools.cjs' + - 'get-shit-done/bin/**' - 'package.json' - 'package-lock.json' - 'scripts/release-tarball-smoke.cjs' - 'tests/release-tarball-smoke.install.test.cjs' - '.github/workflows/install-smoke.yml' - '.github/workflows/release.yml' + - '.github/workflows/hotfix.yml' push: branches: - main @@ -90,22 +93,24 @@ jobs: fetch-depth: 0 # The default `refs/pull/N/merge` ref GitHub produces for PRs is cached - # against the recorded merge-base, not current main. When main advances + # against the recorded merge-base, not the current target branch. When the + # target advances # after the PR was opened, the merge ref stays stale and CI can fail on - # issues that were already fixed upstream. Explicitly merge current - # origin/main into the PR head so smoke always tests the PR against the - # latest trunk. If the merge conflicts, emit a clear "rebase onto main" - # diagnostic instead of a downstream build error that looks unrelated. - - name: Rebase check — merge origin/main into PR head + # issues that were already fixed upstream. Explicitly merge the current + # target branch into the PR head so smoke always tests the PR against the + # latest target. If the merge conflicts, emit a clear rebase diagnostic + # instead of a downstream build error that looks unrelated. + - name: Rebase check — merge PR base branch into PR head if: steps.skip.outputs.skip != 'true' && github.event_name == 'pull_request' shell: bash run: | set -euo pipefail git config user.email "ci@gsd-redux" git config user.name "CI Rebase Check" - git fetch origin main - if ! git merge --no-edit --no-ff origin/main; then - echo "::error::This PR cannot cleanly merge origin/main. Rebase your branch onto current main and push again." + BASE_BRANCH="${GITHUB_BASE_REF:-main}" + git fetch origin "$BASE_BRANCH" + if ! git merge --no-edit --no-ff "origin/$BASE_BRANCH"; then + echo "::error::This PR cannot cleanly merge origin/$BASE_BRANCH. Rebase your branch onto current $BASE_BRANCH and push again." echo "::error::Conflicting files:" git diff --name-only --diff-filter=U git merge --abort @@ -216,18 +221,20 @@ jobs: fetch-depth: 0 # See the `smoke` job above for rationale — refs/pull/N/merge is cached - # against the recorded merge-base, not current main. Explicitly merge - # origin/main so smoke-unpacked also runs against the latest trunk. - - name: Rebase check — merge origin/main into PR head + # against the recorded merge-base, not the current target branch. + # Explicitly merge the PR base so smoke-unpacked also runs against the + # latest target. + - name: Rebase check — merge PR base branch into PR head if: github.event_name == 'pull_request' shell: bash run: | set -euo pipefail git config user.email "ci@gsd-redux" git config user.name "CI Rebase Check" - git fetch origin main - if ! git merge --no-edit --no-ff origin/main; then - echo "::error::This PR cannot cleanly merge origin/main. Rebase your branch onto current main and push again." + BASE_BRANCH="${GITHUB_BASE_REF:-main}" + git fetch origin "$BASE_BRANCH" + if ! git merge --no-edit --no-ff "origin/$BASE_BRANCH"; then + echo "::error::This PR cannot cleanly merge origin/$BASE_BRANCH. Rebase your branch onto current $BASE_BRANCH and push again." echo "::error::Conflicting files:" git diff --name-only --diff-filter=U git merge --abort diff --git a/.github/workflows/pr-template-format.yml b/.github/workflows/pr-template-format.yml index 92ff5553f..99b8b7874 100644 --- a/.github/workflows/pr-template-format.yml +++ b/.github/workflows/pr-template-format.yml @@ -74,7 +74,7 @@ jobs: } core.warning(policy.reason); - - name: Comment, close, and fail untrusted malformed PR + - name: Comment and fail untrusted malformed PR if: steps.policy.outputs.action == 'close' uses: actions/github-script@d746ffe35508b1917358783b479e04febd2b8f71 # v9.0.0 with: @@ -96,11 +96,11 @@ jobs: issue_number: pr.number, body: [ marker, - '## PR template required - PR auto-closed', + '## PR template required', '', 'This PR does not follow one of the required pull request templates from the contribution guidelines.', '', - 'Please reopen with the correct typed template:', + 'Please update the PR description to use the correct typed template:', '', `- [Fix PR template](${repoUrl}/blob/main/.github/PULL_REQUEST_TEMPLATE/fix.md)`, `- [Enhancement PR template](${repoUrl}/blob/main/.github/PULL_REQUEST_TEMPLATE/enhancement.md)`, @@ -112,10 +112,4 @@ jobs: ].join('\n'), }); } - await github.rest.pulls.update({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pr.number, - state: 'closed', - }); core.setFailed(policy.reason); diff --git a/.github/workflows/require-issue-link.yml b/.github/workflows/require-issue-link.yml index 60df98014..129093da0 100644 --- a/.github/workflows/require-issue-link.yml +++ b/.github/workflows/require-issue-link.yml @@ -9,7 +9,7 @@ concurrency: cancel-in-progress: true permissions: - pull-requests: write + issues: write jobs: check-issue-link: @@ -28,20 +28,18 @@ jobs: echo "found=false" >> "$GITHUB_OUTPUT" fi - - name: Comment, close, and fail if no issue link + - name: Comment and fail if no issue link if: steps.check.outputs.found == 'false' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: # Uses GitHub API SDK — no shell string interpolation of untrusted input script: | + const marker = ''; const repoUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}`; const prNumber = context.payload.pull_request.number; - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - body: [ - '## Missing issue link — PR auto-closed', + const body = [ + marker, + '## Missing issue link', '', 'This PR does not reference an issue. **All PRs must link to an open issue** using a closing keyword in the PR body:', '', @@ -51,13 +49,28 @@ jobs: '', `If no issue exists for this change, [open one first](${repoUrl}/issues/new/choose), then update this PR body with the reference.`, '', - 'To resume work after fixing the body: edit the PR description to add a valid `Closes #NNN`, `Fixes #NNN`, or `Resolves #NNN` line, then click **Reopen pull request**. The workflow will re-evaluate on reopen.', - ].join('\n') - }); - await github.rest.pulls.update({ + 'Edit the PR description to add a valid `Closes #NNN`, `Fixes #NNN`, or `Resolves #NNN` line. This check will re-evaluate on the next PR update.', + ].join('\n'); + const comments = await github.paginate(github.rest.issues.listComments, { owner: context.repo.owner, repo: context.repo.repo, - pull_number: prNumber, - state: 'closed', + issue_number: prNumber, + per_page: 100, }); - core.setFailed('PR body must contain a closing issue reference (e.g. "Closes #123") — PR closed.'); + const existing = comments.find(comment => comment.body && comment.body.includes(marker)); + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body, + }); + } + core.setFailed('PR body must contain a closing issue reference (e.g. "Closes #123").'); diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index c9ecf67fd..e6ba663ab 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -3,9 +3,20 @@ name: Security Scan on: pull_request: branches: + - next - main - 'release/**' - 'hotfix/**' + paths: + - 'get-shit-done/**' + - 'agents/**' + - 'commands/**' + - 'hooks/**' + - 'scripts/**' + - 'package.json' + - 'package-lock.json' + - '.github/workflows/security-scan.yml' + - '.secretscanignore' concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} diff --git a/.github/workflows/test-skip.yml b/.github/workflows/test-skip.yml deleted file mode 100644 index 75ad05a40..000000000 --- a/.github/workflows/test-skip.yml +++ /dev/null @@ -1,48 +0,0 @@ -name: Test (skip-noop) - -# Fires on pull_request events that touch NO code paths. -# Emits noop jobs with identical names/matrix to test.yml so the required -# status checks (lint-tests + 6x test (...)) are satisfied on doc-only PRs. -# -# IMPORTANT: paths-ignore here must be the exact inverse of the paths: list -# in test.yml. Keep both in sync whenever a new code path is added. - -on: - pull_request: - branches: - - next - - main - paths-ignore: - - 'bin/**' - - 'get-shit-done/**' - - 'agents/**' - - 'commands/**' - - 'hooks/**' - - 'tests/**' - - 'scripts/**' - - 'package.json' - - 'package-lock.json' - - 'tsconfig*.json' - - '.github/workflows/**' - -concurrency: - group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} - cancel-in-progress: true - -jobs: - lint-tests: - runs-on: ubuntu-latest - steps: - - run: "echo 'doc-only PR: skipping lint-tests'" - shell: bash - - test: - runs-on: ${{ matrix.os }} - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, macos-latest, windows-2025] - node-version: [22, 24] - steps: - - run: "echo 'doc-only PR: skipping test on node ${{ matrix.node-version }} / ${{ matrix.os }}'" - shell: bash diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ac9d00713..a4885c6ef 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,18 +11,6 @@ on: branches: - next - main - paths: - - 'bin/**' - - 'get-shit-done/**' - - 'agents/**' - - 'commands/**' - - 'hooks/**' - - 'tests/**' - - 'scripts/**' - - 'package.json' - - 'package-lock.json' - - 'tsconfig*.json' - - '.github/workflows/**' workflow_dispatch: concurrency: @@ -32,15 +20,70 @@ concurrency: permissions: contents: read -env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - jobs: - # Static lint: test-suite and PR-check contracts. - # Runs once (not per matrix node version) since it is a file-content check. - lint-tests: + changes: + name: Detect test scope runs-on: ubuntu-latest timeout-minutes: 2 + outputs: + code_changed: ${{ steps.scope.outputs.code_changed }} + full_matrix: ${{ steps.scope.outputs.full_matrix }} + targeted_tests: ${{ steps.scope.outputs.targeted_tests }} + windows_tests: ${{ steps.scope.outputs.windows_tests }} + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + persist-credentials: true + token: ${{ github.token }} + + - name: Classify changed paths + id: scope + shell: bash + env: + EVENT_NAME: ${{ github.event_name }} + BASE_SHA: ${{ github.event.pull_request.base.sha || '' }} + HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + run: | + set -euo pipefail + + if [ "$EVENT_NAME" != "pull_request" ]; then + echo "code_changed=true" >> "$GITHUB_OUTPUT" + echo "full_matrix=true" >> "$GITHUB_OUTPUT" + echo "targeted_tests=" >> "$GITHUB_OUTPUT" + echo "windows_tests=" >> "$GITHUB_OUTPUT" + { + echo "## Test scope" + echo "" + echo "- event: \`$EVENT_NAME\`" + echo "- code_changed: \`true\`" + echo "- full_matrix: \`true\`" + } >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + + node scripts/ci-test-scope.cjs --base "$BASE_SHA" --head "$HEAD_SHA" | tee ci-test-scope.json + node <<'NODE' >> "$GITHUB_STEP_SUMMARY" + const fs = require('fs'); + const result = JSON.parse(fs.readFileSync('ci-test-scope.json', 'utf8')); + console.log('## Test scope'); + console.log(''); + console.log(`- code_changed: \`${result.code_changed}\``); + console.log(`- full_matrix: \`${result.full_matrix}\``); + console.log(`- targeted_tests: \`${result.targeted_tests.length}\``); + console.log(`- windows_tests: \`${result.windows_tests.length}\``); + if (result.reasons.length > 0) { + console.log(''); + console.log('### Reasons'); + for (const reason of result.reasons) console.log(`- ${reason}`); + } + NODE + + # Static lint: test-suite and PR-check contracts. + # Runs once, not per matrix lane. + lint-tests: + runs-on: ubuntu-latest + timeout-minutes: 3 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -50,6 +93,9 @@ jobs: uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: node-version: 24 + - name: Lint — skill dependency graph + shell: bash + run: npm run lint:skill-deps - name: Lint — no source-grep tests shell: bash run: node scripts/lint-no-source-grep.cjs @@ -64,26 +110,38 @@ jobs: run: node scripts/lint-pr-check-project-dir.cjs test: + name: test (${{ matrix.os }}, ${{ matrix.node-version }}) + needs: changes + if: needs.changes.outputs.code_changed == 'true' runs-on: ${{ matrix.os }} timeout-minutes: 15 + env: + GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled strategy: - # fail-fast disabled so a single lane failure doesn't cancel the rest; - # makes OS-specific vs version-specific regressions easier to distinguish. fail-fast: false matrix: - os: [ubuntu-latest, macos-latest, windows-2025] - node-version: [22, 24] - # Windows path/separator coverage is also reinforced by hardcoded-paths.test.cjs - # and windows-robustness.test.cjs (static analysis). - # A dedicated windows-compat workflow runs on a weekly schedule. + include: + # Required default PR lanes: fast signal, minimum runtime, primary runtime, + # and one Windows shell/path lane. The non-primary OS/runtime lanes run + # scoped tests from scripts/ci-test-scope.cjs; Ubuntu/Node 24 runs the + # broader default suite. + - os: ubuntu-latest + node-version: 22 + scope: targeted + - os: ubuntu-latest + node-version: 24 + scope: full + - os: windows-latest + node-version: 24 + scope: windows steps: - # Windows lane stays on checkout v4 for reliable auth/header behavior. - # (v6's includeIf.gitdir flow has shown intermittent Windows auth drift.) + # actions/checkout@v6 uses includeIf.gitdir: to inject auth on Windows. + # On Windows git 2.54, the gitdir path comparison is unreliable, so use + # v4 on Windows and v6 elsewhere. - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 (Windows) if: runner.os == 'Windows' with: - # Fetch full history so we can merge origin/main for stale-base detection. fetch-depth: 0 persist-credentials: true token: ${{ github.token }} @@ -91,7 +149,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 (Linux/macOS) if: runner.os != 'Windows' with: - # Fetch full history so we can merge origin/main for stale-base detection. fetch-depth: 0 persist-credentials: true token: ${{ github.token }} @@ -105,8 +162,6 @@ jobs: exit 1 fi - # GitHub's `refs/pull/N/merge` is cached against the recorded merge-base. - # Explicitly merge the PR base branch so tests run against the latest target. - name: Rebase check — merge PR base branch into PR head if: github.event_name == 'pull_request' shell: bash @@ -142,10 +197,6 @@ jobs: node-version: ${{ matrix.node-version }} cache: 'npm' - # Environment check - validates Node/npm version against engines constraints - # and confirms package-lock.json is present before npm ci runs. - # Catches env drift in CI itself (contributor parity check, issue #117). - # Source: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#engines - name: Environment check shell: bash run: npm run check:env @@ -160,47 +211,177 @@ jobs: chmod +x scripts/check-npm-integrity.sh scripts/check-npm-integrity.sh - # PR lanes run affected tests only (changed tests + reverse dependencies - # + smoke fallback). Push lanes continue to run full named suites. - - name: Run affected tests (PR) - if: github.event_name == 'pull_request' + - name: Prepare scoped test list + if: matrix.scope != 'full' shell: bash - run: npm run test:affected + env: + TEST_SCOPE: ${{ matrix.scope }} + TARGETED_TESTS: ${{ needs.changes.outputs.targeted_tests }} + WINDOWS_TESTS: ${{ needs.changes.outputs.windows_tests }} + run: | + set -euo pipefail + case "$TEST_SCOPE" in + windows) + selected="$WINDOWS_TESTS" + ;; + targeted) + selected="$TARGETED_TESTS" + ;; + *) + echo "::error::Unknown test scope: $TEST_SCOPE" + exit 1 + ;; + esac + + if [ -z "${selected// }" ]; then + selected="tests/command-contract.test.cjs tests/commands.test.cjs tests/core.test.cjs tests/package-manifest.test.cjs" + fi + + printf '%s\n' "$selected" | tr ' ' '\n' | sed '/^$/d' > .ci-selected-tests.txt + echo "Scoped tests:" + cat .ci-selected-tests.txt + + - name: Run scoped tests + if: matrix.scope != 'full' + shell: bash + run: node scripts/run-tests.cjs --files-from .ci-selected-tests.txt - name: Run unit tests - if: github.event_name != 'pull_request' + if: matrix.scope == 'full' shell: bash run: npm run test:unit - name: Run integration tests - if: github.event_name != 'pull_request' + if: matrix.scope == 'full' shell: bash run: npm run test:integration - name: Run security tests - if: github.event_name != 'pull_request' + if: matrix.scope == 'full' shell: bash run: npm run test:security - # Install/slow remain push-only checks on the canonical lane. - name: Run install tests - if: github.event_name != 'pull_request' && matrix.os == 'ubuntu-latest' && matrix.node-version == 24 + if: matrix.scope == 'full' && needs.changes.outputs.full_matrix == 'true' shell: bash run: npm run test:install - name: Run slow tests - if: github.event_name != 'pull_request' && matrix.os == 'ubuntu-latest' && matrix.node-version == 24 + if: matrix.scope == 'full' && needs.changes.outputs.full_matrix == 'true' shell: bash run: npm run test:slow + test-full: + name: full test (${{ matrix.os }}, ${{ matrix.node-version }}) + needs: changes + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.full_matrix == 'true' + runs-on: ${{ matrix.os }} + timeout-minutes: 15 + env: + GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled + strategy: + fail-fast: false + matrix: + include: + - os: windows-latest + node-version: 22 + - os: macos-latest + node-version: 22 + - os: macos-latest + node-version: 24 + + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 (Windows) + if: runner.os == 'Windows' + with: + fetch-depth: 0 + persist-credentials: true + token: ${{ github.token }} + + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 (Linux/macOS) + if: runner.os != 'Windows' + with: + fetch-depth: 0 + persist-credentials: true + token: ${{ github.token }} + + - name: Guard — require GitHub-hosted runner + shell: bash + run: | + set -euo pipefail + if [ "${RUNNER_ENVIRONMENT:-}" != "github-hosted" ]; then + echo "::error::Expected github-hosted runner. RUNNER_ENVIRONMENT=${RUNNER_ENVIRONMENT:-unset}" + exit 1 + fi + + - name: Rebase check — merge PR base branch into PR head + if: github.event_name == 'pull_request' + shell: bash + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + git config user.email "ci@gsd-redux" + git config user.name "CI Rebase Check" + git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" + BASE_BRANCH="${GITHUB_BASE_REF:-main}" + for attempt in 1 2 3; do + if git fetch origin "$BASE_BRANCH"; then + break + fi + if [ "$attempt" -eq 3 ]; then + echo "::error::git fetch origin $BASE_BRANCH failed after 3 attempts." + exit 1 + fi + sleep $((attempt * 4)) + done + if ! git merge --no-edit --no-ff "origin/$BASE_BRANCH"; then + echo "::error::This PR cannot cleanly merge origin/$BASE_BRANCH. Rebase your branch onto current $BASE_BRANCH and push again." + echo "::error::Conflicting files:" + git diff --name-only --diff-filter=U + git merge --abort + exit 1 + fi + + - name: Set up Node.js ${{ matrix.node-version }} + uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + with: + node-version: ${{ matrix.node-version }} + cache: 'npm' + + - name: Environment check + shell: bash + run: npm run check:env + + - name: Install dependencies + shell: bash + run: npm ci + + - name: Dependency integrity gate + shell: bash + run: | + chmod +x scripts/check-npm-integrity.sh + scripts/check-npm-integrity.sh + + - name: Run unit tests + shell: bash + run: npm run test:unit + + - name: Run integration tests + shell: bash + run: npm run test:integration + + - name: Run security tests + shell: bash + run: npm run test:security - # Dedicated coverage job. Runs only on ubuntu/Node 24 (the canonical lane) - # because c8 coverage of one suite on one OS is enough signal — running it - # across the full matrix would 9x the cost for no extra coverage data. - # Replaces main's per-matrix `Run tests with coverage` step. coverage: + needs: changes + if: needs.changes.outputs.code_changed == 'true' runs-on: ubuntu-latest timeout-minutes: 15 + env: + GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -268,3 +449,65 @@ jobs: coverage/ .nyc_output/ if-no-files-found: ignore + + required-tests: + name: Required tests + needs: + - changes + - lint-tests + - test + - test-full + - coverage + if: always() + runs-on: ubuntu-latest + timeout-minutes: 1 + steps: + - name: Summarize required test gate + shell: bash + env: + CODE_CHANGED: ${{ needs.changes.outputs.code_changed }} + CHANGES_RESULT: ${{ needs.changes.result }} + LINT_RESULT: ${{ needs.lint-tests.result }} + TEST_RESULT: ${{ needs.test.result }} + FULL_TEST_RESULT: ${{ needs.test-full.result }} + COVERAGE_RESULT: ${{ needs.coverage.result }} + run: | + set -euo pipefail + echo "code_changed=$CODE_CHANGED" + echo "changes=$CHANGES_RESULT" + echo "lint-tests=$LINT_RESULT" + echo "test=$TEST_RESULT" + echo "test-full=$FULL_TEST_RESULT" + echo "coverage=$COVERAGE_RESULT" + + if [ "$CHANGES_RESULT" != "success" ]; then + echo "::error::test scope detection did not pass" + exit 1 + fi + + if [ "$LINT_RESULT" != "success" ]; then + echo "::error::lint-tests did not pass" + exit 1 + fi + + if [ "$CODE_CHANGED" != "true" ]; then + echo "No code paths changed; matrix tests intentionally skipped." + exit 0 + fi + + if [ "$TEST_RESULT" != "success" ]; then + echo "::error::test matrix did not pass" + exit 1 + fi + + if [ "$FULL_TEST_RESULT" != "success" ] && [ "$FULL_TEST_RESULT" != "skipped" ]; then + echo "::error::full parity matrix did not pass" + exit 1 + fi + + if [ "$COVERAGE_RESULT" != "success" ]; then + echo "::error::coverage did not pass" + exit 1 + fi + + echo "Required test gate passed." diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index 304d47975..6c3f787b9 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -49,25 +49,44 @@ Direct harness invocation also works: ```bash node scripts/run-tests.cjs --suite security node scripts/run-tests.cjs --suite=security +node scripts/run-tests.cjs --files "tests/command-contract.test.cjs tests/core.test.cjs" +node scripts/run-tests.cjs --files-from .ci-selected-tests.txt ``` Unknown suites exit non-zero with the list of valid suites. Empty suites (e.g. `--suite security` before any security-tagged file exists) exit `0` with a `no tests in suite "..."` notice on stderr so CI lanes don't go red while a suite is being populated. ## CI matrix -The `Tests` workflow runs on: +The `Tests` workflow runs every PR through a scoped gate generated by +`scripts/ci-test-scope.cjs`. -| OS | Node 22 | Node 24 | Node 26 | -|---|---|---|---| -| `ubuntu-latest` | gate | gate | forward-compat (`continue-on-error`) | -| `macos-latest` | gate | gate | forward-compat (`continue-on-error`) | -| `windows-latest` | gate | gate | forward-compat (`continue-on-error`) | +| Lane | Node 22 | Node 24 | +|---|---|---| +| `ubuntu-latest` | scoped tests | unit + integration + security | +| `windows-latest` | — | scoped Windows/path/shell tests | +| `macos-latest` | full parity when required | full parity when required | - **Node 22** is the `engines.node` floor (`>=22.0.0`) — must stay green. - **Node 24** is the default development lane. -- **Node 26** is forward-compat. The lane reports status but does not gate the workflow — `actions/setup-node` may not yet have a stable Node 26 image at any given moment. When it stabilises, flip `continue-on-error` off. +- **Scoped tests** are selected from the changed paths, plus a small CLI/package + smoke set. They are for confidence on the affected surface, not for counting + tests. -Each matrix cell runs `test:affected` on PRs (changed tests + reverse-import dependents + a smoke fallback), then runs full named suites on push lanes. `install` and `slow` remain `main`/release push-only checks on the canonical lane to keep PR CI fast. Coverage runs in a dedicated `coverage` job on `ubuntu-latest` / Node 24 — running coverage across the full matrix would 9x the cost for no extra coverage data. +The default PR gate runs the broad `unit`, `integration`, and `security` suites +once on Ubuntu / Node 24, scoped smoke on Ubuntu / Node 22, scoped +Windows/path/shell tests on Windows / Node 24, and unit coverage once on Ubuntu / +Node 24. PRs touching workflow, package, test-runner, install, release, or +Windows-sensitive surfaces also run the full parity matrix on macOS and the +older Windows runtime, plus `install` and `slow` on the primary Ubuntu lane. +Coverage stays single-lane because multiplying coverage across OS/runtime lanes +adds cost without improving the threshold signal. + +To inspect the scope locally: + +```bash +npm run ci:test-scope -- --files "commands/gsd/plan-phase.md" +node scripts/ci-test-scope.cjs --base origin/next --head HEAD +``` ## Best practices for forward-compat (Node 24/26) diff --git a/docs/branch-protection.md b/docs/branch-protection.md index f09063bb6..e4c1d286f 100644 --- a/docs/branch-protection.md +++ b/docs/branch-protection.md @@ -10,7 +10,9 @@ Targets `~DEFAULT_BRANCH` (main). Enforces: - No deletions or force pushes - Required linear history (no merge commits) - All changes via pull request (0 required approvals, stale-review dismissal, thread resolution required, squash/rebase only) -- 10 required status checks: lint, Node 22/24 matrix (ubuntu/mac/windows), Changeset Required, require-issue-link, pr-template-format +- Required status checks: the aggregate `Required tests` gate plus PR policy + checks for size, branch name, changeset, docs, target branch, issue link, and + PR template format ### `release-branches` Targets `refs/heads/release/**` and `refs/heads/hotfix/**`. Same rules as @@ -26,7 +28,7 @@ immutable once created. Tag creation is unrestricted. | PR | Branch | Action | |----|--------|--------| | PR-1 | `chore/branch-protection-specs` | Check in spec files; `enforcement: disabled` — no effect on repo | -| PR-2 | `chore/ci-skip-tests-on-docs` | Add path filter to `test.yml` + new `test-skip.yml` noop; doc-only PRs satisfy required checks in <30s | +| PR-2 | `chore/tiered-ci-gate` | Add tiered test scope detection to `test.yml`; doc-only PRs satisfy `Required tests` without macOS/Windows noop queues | | PR-3 | `chore/branch-protection-evaluate` | Run `sync-rulesets.sh` with `ENFORCEMENT=evaluate`; 1-week dry-run via rule-suite logs | | PR-4 | `chore/branch-protection-active` | Run `sync-rulesets.sh` with `ENFORCEMENT=active`; protection live | @@ -63,26 +65,31 @@ triggered. Use this to validate no legitimate workflows are broken before flipping to `active` in PR-3. -## Path filters +## Test scope detection -The `test.yml` workflow uses a `paths:` filter on its `pull_request:` trigger so -the 6-lane matrix only runs when code-touching files change. A companion workflow, -`test-skip.yml`, fires on the inverse (`paths-ignore:`) and emits instant noop -jobs with **identical job IDs and matrix dimensions**. This ensures the 7 required -status checks (`lint-tests` + 6× `test (...)`) are always satisfied — whether the -real matrix ran or the noop ran. +The `test.yml` workflow always runs, but its `changes` job classifies the PR +with `scripts/ci-test-scope.cjs` before starting expensive runners. The required +branch-protection context is the single aggregate `Required tests` job. -### Why dual workflow instead of paths-ignore alone? +Doc-only PRs run the lightweight lint and aggregate jobs only. Code-touching PRs +run the default required matrix: -GitHub required-status-checks expect a specific context string to appear as -"passed" on every PR. If `test.yml` is suppressed by `paths-ignore` on a doc-only -PR, those contexts never fire and the PR is permanently blocked. The noop workflow -produces the same context strings via matching job IDs + matrix, resolving the -deadlock. +- Ubuntu / Node 22 scoped tests +- Ubuntu / Node 24 unit, integration, and security suites +- Windows / Node 24 scoped Windows/path/shell tests +- Ubuntu / Node 24 coverage + +PRs that touch workflow, package, test-runner, install, release, or Windows +sensitive surfaces also run install/slow on the primary Ubuntu lane and the full +parity matrix: + +- Windows / Node 22 +- macOS / Node 22 +- macOS / Node 24 ### Canonical code-paths list -Both `test.yml` (`paths:`) and `test-skip.yml` (`paths-ignore:`) use this list: +The `changes` job treats these paths as code-touching: ``` bin/** @@ -90,29 +97,32 @@ get-shit-done/** agents/** commands/** hooks/** -sdk/** tests/** scripts/** package.json package-lock.json tsconfig*.json -.github/workflows/test.yml -.github/workflows/test-skip.yml +.github/workflows/** +.github/rulesets/** ``` -This list also mirrors `changeset-required.yml`'s path filter. Keep all three in sync. +This list should stay aligned with changeset/docs policy where those gates care +about the same user-facing surfaces. ### Adding a new code path When adding a directory or file that should trigger real tests: -1. Add the glob to `paths:` in `.github/workflows/test.yml` -2. Add the **same** glob to `paths-ignore:` in `.github/workflows/test-skip.yml` -3. Add the same glob to `changeset-required.yml` if changesets should be required +1. Add the path classifier to `scripts/ci-test-scope.cjs`. +2. Add targeted tests for that surface in the same classifier. +3. If it requires macOS/extra-Windows coverage, mark the classifier rule + `fullMatrix: true`. +4. Add the same glob to `changeset-required.yml` if changesets should be required for that path -Failure to update `test-skip.yml` means doc PRs that happen to touch the new -path will deadlock (real matrix never fires, noop never fires either). +The old `test-skip.yml` inverse-path workflow was removed. Do not add required +checks for individual matrix jobs; require the aggregate `Required tests` context +instead. ## Phase-2 TODO diff --git a/package.json b/package.json index a8e71cb94..a652cb15c 100644 --- a/package.json +++ b/package.json @@ -69,6 +69,7 @@ "lint:pr-checks": "node scripts/lint-pr-check-project-dir.cjs", "lint:changeset": "node scripts/changeset/lint.cjs", "lint:docs": "node scripts/lint-docs-required.cjs", + "ci:test-scope": "node scripts/ci-test-scope.cjs", "changeset": "node scripts/changeset/new.cjs", "changelog:render": "node scripts/changeset/cli.cjs render", "test": "node scripts/run-tests.cjs", diff --git a/scripts/ci-test-scope.cjs b/scripts/ci-test-scope.cjs new file mode 100644 index 000000000..56a21bba3 --- /dev/null +++ b/scripts/ci-test-scope.cjs @@ -0,0 +1,303 @@ +#!/usr/bin/env node +'use strict'; + +const { execFileSync } = require('child_process'); +const { existsSync, readdirSync, appendFileSync } = require('fs'); +const { join } = require('path'); + +const DEFAULT_SMOKE_TESTS = [ + 'tests/command-contract.test.cjs', + 'tests/commands.test.cjs', + 'tests/core.test.cjs', + 'tests/package-manifest.test.cjs', +]; + +const WINDOWS_SMOKE_TESTS = [ + 'tests/hardcoded-paths.test.cjs', + 'tests/windows-robustness.test.cjs', + 'tests/windows-test-parity-guard.test.cjs', + 'tests/workflow-shell-pinning.test.cjs', +]; + +const RULES = [ + { + name: 'workflow automation', + match: path => path.startsWith('.github/workflows/') || path.startsWith('.github/rulesets/'), + fullMatrix: true, + tests: [ + 'tests/workflow-shell-pinning.test.cjs', + 'tests/release-tarball-smoke-workflow.test.cjs', + 'tests/lint-pr-check-project-dir.test.cjs', + 'tests/pr-template-policy.test.cjs', + ], + }, + { + name: 'test harness', + match: path => path === 'scripts/run-tests.cjs', + fullMatrix: true, + tests: [ + 'tests/run-tests-harness.test.cjs', + 'tests/workflow-shell-pinning.test.cjs', + ], + }, + { + name: 'environment and dependency gates', + match: path => [ + 'scripts/check-env.sh', + 'scripts/check-npm-integrity.sh', + 'package.json', + 'package-lock.json', + ].includes(path), + fullMatrix: true, + tests: [ + 'tests/check-env.test.cjs', + 'tests/npm-integrity-gate.test.cjs', + 'tests/package-manifest.test.cjs', + 'tests/bug-3588-npm-audit-clean.test.cjs', + ], + }, + { + name: 'installer and package layout', + match: path => path.startsWith('bin/') || + path.startsWith('get-shit-done/bin/') || + path.includes('install') || + path.includes('release-tarball-smoke'), + fullMatrix: true, + tests: [ + 'tests/install.test.cjs', + 'tests/install-regressions.test.cjs', + 'tests/install-runtime-artifacts.test.cjs', + 'tests/install-path-detection.test.cjs', + 'tests/release-tarball-smoke.install.test.cjs', + 'tests/runtime-artifact-layout.test.cjs', + ], + }, + { + name: 'hooks', + match: path => path.startsWith('hooks/'), + fullMatrix: true, + tests: [ + 'tests/hook-validation.test.cjs', + 'tests/managed-hooks.test.cjs', + 'tests/hooks-opt-in.test.cjs', + 'tests/sh-hook-paths.test.cjs', + 'tests/precommit-alias-drift-hook.test.cjs', + 'tests/prepush-enterprise-email-hook.test.cjs', + ], + }, + { + name: 'changeset tooling', + match: path => path.startsWith('scripts/changeset/') || path.startsWith('.changeset/'), + tests: [ + 'tests/changeset-cli.test.cjs', + 'tests/changeset-lint.test.cjs', + 'tests/changeset-new.test.cjs', + 'tests/changeset-parse.test.cjs', + 'tests/changeset-render.test.cjs', + 'tests/changeset-serialize.test.cjs', + 'tests/changeset-github-release-notes.test.cjs', + ], + }, + { + name: 'security scanners', + match: path => path.includes('secret-scan') || + path.includes('base64-scan') || + path.includes('prompt-injection-scan') || + path.startsWith('tests/fixtures/adversarial/security/'), + tests: [ + 'tests/secret-scan-lint.test.cjs', + 'tests/prompt-injection-scan.test.cjs', + 'tests/security-prompt-injection.test.cjs', + 'tests/read-injection-scanner.test.cjs', + 'tests/security-scan.test.cjs', + ], + }, + { + name: 'command definitions', + match: path => path.startsWith('commands/'), + tests: [ + 'tests/command-contract.test.cjs', + 'tests/command-routing-hub.test.cjs', + 'tests/commands.test.cjs', + 'tests/phase-command-router.test.cjs', + 'tests/roadmap-command-router.test.cjs', + ], + }, + { + name: 'workflow prompts', + match: path => path.startsWith('get-shit-done/workflows/'), + tests: [ + 'tests/workflow-compat.test.cjs', + 'tests/workflow-size-budget.test.cjs', + 'tests/workflow-guard-registration.test.cjs', + 'tests/commands.test.cjs', + 'tests/bug-3683-workflow-colon-namespace-leak.test.cjs', + ], + }, + { + name: 'agent prompts', + match: path => path.startsWith('agents/'), + tests: [ + 'tests/agent-frontmatter.test.cjs', + 'tests/agent-size-budget.test.cjs', + 'tests/agent-skills.test.cjs', + 'tests/agent-skills-awareness.test.cjs', + 'tests/agent-required-reading-consistency.test.cjs', + ], + }, + { + name: 'configuration', + match: path => /config|configuration|model-catalog|model-profile/.test(path), + tests: [ + 'tests/config.test.cjs', + 'tests/config-get-default.test.cjs', + 'tests/configuration-migrate-config.test.cjs', + 'tests/model-catalog-runtime-defaults.test.cjs', + 'tests/model-profiles.test.cjs', + ], + }, +]; + +function usage() { + return [ + 'Usage:', + ' node scripts/ci-test-scope.cjs --base --head ', + ' node scripts/ci-test-scope.cjs --files ', + '', + 'Prints JSON by default. With GITHUB_OUTPUT set, also writes workflow outputs.', + ].join('\n'); +} + +function parseArgs(argv) { + const out = { base: null, head: null, files: null }; + for (let i = 0; i < argv.length; i++) { + const arg = argv[i]; + if (arg === '--base') { + out.base = argv[++i]; + if (!out.base || out.base.startsWith('--')) throw new Error('--base requires a value'); + } else if (arg.startsWith('--base=')) { + out.base = arg.slice('--base='.length); + if (!out.base) throw new Error('--base requires a value'); + } else if (arg === '--head') { + out.head = argv[++i]; + if (!out.head || out.head.startsWith('--')) throw new Error('--head requires a value'); + } else if (arg.startsWith('--head=')) { + out.head = arg.slice('--head='.length); + if (!out.head) throw new Error('--head requires a value'); + } else if (arg === '--files') { + out.files = argv[++i]; + if (!out.files || out.files.startsWith('--')) throw new Error('--files requires a value'); + } else if (arg.startsWith('--files=')) { + out.files = arg.slice('--files='.length); + if (!out.files) throw new Error('--files requires a value'); + } else if (arg === '--help' || arg === '-h') { + console.log(usage()); + process.exit(0); + } else { + throw new Error(`unknown argument: ${arg}`); + } + } + return out; +} + +function splitFiles(value) { + if (!value) return []; + return value.split(/[,\s]+/).map(v => v.trim()).filter(Boolean); +} + +function changedFiles(args) { + if (args.files) return splitFiles(args.files); + if (!args.base || !args.head) { + throw new Error('--base/--head or --files is required'); + } + const stdout = execFileSync('git', ['diff', '--name-only', args.base, args.head], { + encoding: 'utf8', + }); + return splitFiles(stdout); +} + +function existingTests(files) { + const all = new Set(readdirSync('tests').filter(f => f.endsWith('.test.cjs')).map(f => `tests/${f}`)); + return files.filter(file => all.has(file) && existsSync(file)); +} + +function addAll(set, values) { + for (const value of values) set.add(value); +} + +function classify(files) { + const targeted = new Set(); + const windows = new Set(); + const reasons = []; + let codeChanged = false; + let fullMatrix = false; + + for (const file of files) { + if (/^(bin|get-shit-done|agents|commands|hooks|tests|scripts)\//.test(file) || + /^package(-lock)?\.json$/.test(file) || + /^tsconfig.*\.json$/.test(file) || + file.startsWith('.github/workflows/') || + file.startsWith('.github/rulesets/')) { + codeChanged = true; + } + + if (file.startsWith('tests/') && file.endsWith('.test.cjs')) { + targeted.add(file); + if (/windows|path|shell|workflow|install|hook/i.test(file)) { + windows.add(file); + } + } + + for (const rule of RULES) { + if (rule.match(file)) { + addAll(targeted, rule.tests); + reasons.push(`${file}: ${rule.name}`); + if (rule.fullMatrix) fullMatrix = true; + } + } + } + + if (codeChanged) { + addAll(targeted, DEFAULT_SMOKE_TESTS); + addAll(windows, WINDOWS_SMOKE_TESTS); + } + + const targetedTests = existingTests([...targeted].sort()); + const windowsTests = existingTests([...new Set([...windows, ...targetedTests.filter(t => /windows|path|shell|workflow|install|hook/i.test(t))])].sort()); + + return { + code_changed: codeChanged, + full_matrix: fullMatrix, + targeted_tests: targetedTests, + windows_tests: windowsTests, + reasons: [...new Set(reasons)].sort(), + }; +} + +function writeOutputs(result) { + if (!process.env.GITHUB_OUTPUT) return; + const lines = [ + `code_changed=${result.code_changed}`, + `full_matrix=${result.full_matrix}`, + `targeted_tests=${result.targeted_tests.join(' ')}`, + `windows_tests=${result.windows_tests.join(' ')}`, + ]; + appendFileSync(process.env.GITHUB_OUTPUT, `${lines.join('\n')}\n`); +} + +function main() { + try { + const args = parseArgs(process.argv.slice(2)); + const files = changedFiles(args); + const result = classify(files); + result.changed_files = files; + writeOutputs(result); + console.log(JSON.stringify(result, null, 2)); + } catch (error) { + console.error(`ci-test-scope: ${error.message}`); + console.error(usage()); + process.exit(2); + } +} + +main(); diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 53f4a8964..43b31cb4c 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -11,6 +11,8 @@ // node scripts/run-tests.cjs --suite integration # *.integration.test.cjs // node scripts/run-tests.cjs --suite install # *.install.test.cjs // node scripts/run-tests.cjs --suite slow # *.slow.test.cjs +// node scripts/run-tests.cjs --files "a.test.cjs b.test.cjs" +// node scripts/run-tests.cjs --files-from /tmp/selected-tests.txt // // Suite grouping convention: filename suffix marker before `.test.cjs`. // A file named `foo.security.test.cjs` belongs to the `security` suite. @@ -28,6 +30,8 @@ const MARKED_SUITES = ['integration', 'install', 'security', 'slow']; function parseArgs(argv) { let suite = null; let seen = false; + let files = null; + let filesFrom = null; for (let i = 0; i < argv.length; i++) { const a = argv[i]; if (a === '--suite') { @@ -50,11 +54,50 @@ function parseArgs(argv) { if (!suite) { return { error: '--suite requires a value' }; } + } else if (a === '--files') { + if (files !== null) { + return { error: 'duplicate --files flag' }; + } + const v = argv[i + 1]; + if (!v || v.startsWith('--')) { + return { error: '--files requires a value' }; + } + files = v; + i++; + } else if (a.startsWith('--files=')) { + if (files !== null) { + return { error: 'duplicate --files flag' }; + } + files = a.slice('--files='.length); + if (!files) { + return { error: '--files requires a value' }; + } + } else if (a === '--files-from') { + if (filesFrom !== null) { + return { error: 'duplicate --files-from flag' }; + } + const v = argv[i + 1]; + if (!v || v.startsWith('--')) { + return { error: '--files-from requires a value' }; + } + filesFrom = v; + i++; + } else if (a.startsWith('--files-from=')) { + if (filesFrom !== null) { + return { error: 'duplicate --files-from flag' }; + } + filesFrom = a.slice('--files-from='.length); + if (!filesFrom) { + return { error: '--files-from requires a value' }; + } } else { return { error: `unknown argument: ${a}` }; } } - return { suite }; + if (files !== null && filesFrom !== null) { + return { error: '--files and --files-from cannot be combined' }; + } + return { suite, files, filesFrom }; } // Return the marked suite name embedded in a filename, or null if it's unmarked. @@ -79,6 +122,38 @@ function selectFiles(allFiles, suite) { return allFiles.filter(f => suiteOf(f) === suite); } +function splitFileList(value) { + if (!value) return []; + return value + .split(/[,\s]+/) + .map(v => v.trim()) + .filter(Boolean) + .map(v => v.replace(/^tests[\\/]/, '')); +} + +function selectExplicitFiles(allFiles, filesValue, filesFrom) { + const fs = require('fs'); + const requested = filesFrom + ? splitFileList(fs.readFileSync(filesFrom, 'utf8')) + : splitFileList(filesValue); + const available = new Set(allFiles); + const selected = []; + const missing = []; + for (const file of requested) { + if (available.has(file)) { + selected.push(file); + } else { + missing.push(file); + } + } + if (missing.length > 0) { + return { + error: `requested test file(s) not found: ${missing.join(', ')}`, + }; + } + return { files: [...new Set(selected)] }; +} + function main() { const args = process.argv.slice(2); const parsed = parseArgs(args); @@ -107,7 +182,18 @@ function main() { process.exit(1); } - const selected = selectFiles(allFiles, suite).map(f => join(testDir, f)); + let selectedNames; + if (parsed.files !== null || parsed.filesFrom !== null) { + const explicit = selectExplicitFiles(allFiles, parsed.files, parsed.filesFrom); + if (explicit.error) { + console.error(`run-tests: ${explicit.error}`); + process.exit(2); + } + selectedNames = explicit.files; + } else { + selectedNames = selectFiles(allFiles, suite); + } + const selected = selectedNames.map(f => join(testDir, f)); if (selected.length === 0) { // Empty suite: report and exit 0 so empty lanes (e.g. `security` before diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs new file mode 100644 index 000000000..cb7a8c05c --- /dev/null +++ b/tests/ci-test-scope.test.cjs @@ -0,0 +1,71 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const { spawnSync } = require('child_process'); +const path = require('path'); + +const ROOT = path.join(__dirname, '..'); +const SCRIPT = path.join(ROOT, 'scripts', 'ci-test-scope.cjs'); + +function scopeFor(files) { + const r = spawnSync(process.execPath, [SCRIPT, '--files', files.join(' ')], { + cwd: ROOT, + encoding: 'utf8', + }); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); + return JSON.parse(r.stdout); +} + +describe('ci-test-scope.cjs', () => { + test('docs-only changes do not request code matrix work', () => { + const result = scopeFor(['docs/usage.md']); + assert.strictEqual(result.code_changed, false); + assert.strictEqual(result.full_matrix, false); + assert.deepStrictEqual(result.targeted_tests, []); + assert.deepStrictEqual(result.windows_tests, []); + }); + + test('workflow changes request full matrix and workflow contract tests', () => { + const result = scopeFor(['.github/workflows/test.yml']); + assert.strictEqual(result.code_changed, true); + assert.strictEqual(result.full_matrix, true); + assert.ok(result.targeted_tests.includes('tests/workflow-shell-pinning.test.cjs')); + assert.ok(result.targeted_tests.includes('tests/release-tarball-smoke-workflow.test.cjs')); + assert.ok(result.windows_tests.includes('tests/workflow-shell-pinning.test.cjs')); + }); + + test('command changes request command tests without full parity matrix', () => { + const result = scopeFor(['commands/gsd/plan-phase.md']); + assert.strictEqual(result.code_changed, true); + assert.strictEqual(result.full_matrix, false); + assert.ok(result.targeted_tests.includes('tests/command-contract.test.cjs')); + assert.ok(result.targeted_tests.includes('tests/commands.test.cjs')); + }); + + test('changed test files are selected directly', () => { + const result = scopeFor(['tests/run-tests-harness.test.cjs']); + assert.strictEqual(result.code_changed, true); + assert.ok(result.targeted_tests.includes('tests/run-tests-harness.test.cjs')); + }); + + test('installer-sensitive changes request full matrix and install tests', () => { + const result = scopeFor(['bin/gsd']); + assert.strictEqual(result.code_changed, true); + assert.strictEqual(result.full_matrix, true); + assert.ok(result.targeted_tests.includes('tests/install.test.cjs')); + assert.ok(result.targeted_tests.includes('tests/release-tarball-smoke.install.test.cjs')); + }); + + test('missing required CLI values fail with usage', () => { + const r = spawnSync(process.execPath, [SCRIPT, '--files'], { + cwd: ROOT, + encoding: 'utf8', + }); + assert.notStrictEqual(r.status, 0); + // allow-test-rule: CLI usage failure text is user-facing contract for this parser guard. + assert.match(r.stderr, /--files requires a value/); + // allow-test-rule: CLI usage banner presence is a user-facing contract. + assert.match(r.stderr, /Usage:/); + }); +}); diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index 75cb12155..c819000de 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -93,6 +93,29 @@ describe('run-tests.cjs harness (issue #3597)', () => { const r = runHarness(tmpDir, ['--suite=security']); assert.strictEqual(r.status, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); }); + + test('missing --files value exits non-zero', () => { + seed(tmpDir, ['a.test.cjs']); + const r = runHarness(tmpDir, ['--files']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /--files requires a value/i); + }); + + test('duplicate --files flag is rejected', () => { + seed(tmpDir, ['a.test.cjs']); + const r = runHarness(tmpDir, ['--files', 'a.test.cjs', '--files', 'a.test.cjs']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /duplicate --files/i); + }); + + test('--files and --files-from cannot be combined', () => { + seed(tmpDir, ['a.test.cjs']); + const listPath = path.join(tmpDir, 'selected-tests.txt'); + fs.writeFileSync(listPath, 'a.test.cjs\n', 'utf8'); + const r = runHarness(tmpDir, ['--files', 'a.test.cjs', '--files-from', listPath]); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /cannot be combined/i); + }); }); describe('suite filtering', () => { @@ -192,6 +215,35 @@ describe('run-tests.cjs harness (issue #3597)', () => { }); }); + describe('explicit file selection', () => { + test('--files runs only the named tests', () => { + seed(tmpDir, ['a.test.cjs', 'b.security.test.cjs', 'c.test.cjs']); + const r = runHarness(tmpDir, ['--files', 'a.test.cjs tests/c.test.cjs']); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + assert.ok(r.stderr.includes('a.test.cjs')); + assert.ok(r.stderr.includes('c.test.cjs')); + assert.ok(!r.stderr.includes('b.security.test.cjs')); + }); + + test('--files-from runs tests listed in a file', () => { + seed(tmpDir, ['a.test.cjs', 'b.security.test.cjs', 'c.test.cjs']); + const listPath = path.join(tmpDir, 'selected-tests.txt'); + fs.writeFileSync(listPath, 'a.test.cjs\nb.security.test.cjs\n', 'utf8'); + const r = runHarness(tmpDir, ['--files-from', listPath]); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + assert.ok(r.stderr.includes('a.test.cjs')); + assert.ok(r.stderr.includes('b.security.test.cjs')); + assert.ok(!r.stderr.includes('c.test.cjs')); + }); + + test('missing explicit test file exits non-zero', () => { + seed(tmpDir, ['a.test.cjs']); + const r = runHarness(tmpDir, ['--files', 'a.test.cjs missing.test.cjs']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /requested test file\(s\) not found: missing\.test\.cjs/i); + }); + }); + describe('failure propagation', () => { test('non-zero from node:test propagates through harness', () => { const FAIL = `'use strict';