From ec61201547d2081d1e3de17485e88420abcbde4c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 10:54:31 +0000 Subject: [PATCH 01/10] chore: replace peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/a76f3f0b-859f-486a-b72d-f2c7c9b48477 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- .github/workflows/deploy.yml | 57 ++++++++++++++++++++------ .github/workflows/e2e-tests.yml | 30 +++++++++----- .github/workflows/preview.yml | 68 +++++++++++++++++++++++++------- .github/workflows/unit-tests.yml | 30 +++++++++----- docs/LEARNINGS.md | 10 +++++ 5 files changed, 151 insertions(+), 44 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 340107e..40fc3ab 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -27,16 +27,51 @@ jobs: run: npm run build - name: Deploy to GitHub Pages (gh-pages branch) - uses: peaceiris/actions-gh-pages@4f9cc6602d3f66b9c108549d475ec49e8ef4d45e # v4.0.0 - with: - github_token: ${{ secrets.GITHUB_TOKEN }} - publish_dir: . - # Preserve any existing PR preview directories across production deploys - keep_files: true - # Exclude non-site files from the deployment. - # Keep this list in sync with the exclude_assets in preview.yml + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + + # Fetch the existing gh-pages branch (silently skip if it doesn't exist yet) + git fetch origin gh-pages:refs/remotes/origin/gh-pages 2>/dev/null || true + + # Check out gh-pages into a worktree so we can update it in-place. + # This preserves any existing content (e.g. previews/) — equivalent to keep_files: true. + if git rev-parse --verify refs/remotes/origin/gh-pages >/dev/null 2>&1; then + git branch --force gh-pages refs/remotes/origin/gh-pages + git worktree add /tmp/gh-pages gh-pages + else + # First-ever deploy: create an orphan gh-pages branch + git worktree add --orphan -b gh-pages /tmp/gh-pages + fi + + # Ensure Jekyll processing is disabled + touch /tmp/gh-pages/.nojekyll + + # Sync the built site into the worktree root. + # Excluded paths keep non-site files out of the deployment. + # Keep this list in sync with the exclude list in preview.yml # (screenshots is an additional preview-only exclusion). - # .gitignore is excluded here too: if it lands in peaceiris's gh-pages temp - # clone, `git add --all` would silently skip script.js, styles.css and the + # .gitignore is excluded so it never lands in gh-pages; if it did, + # `git add -A` would silently skip script.js, styles.css and the # generated data files (all listed in .gitignore). - exclude_assets: '.github,.gitignore,node_modules,tests,scripts,package-lock.json,package.json,milestones.yaml,project-stats.yaml' + rsync -a \ + --exclude='.git' \ + --exclude='.github' \ + --exclude='.gitignore' \ + --exclude='node_modules' \ + --exclude='tests' \ + --exclude='scripts' \ + --exclude='package-lock.json' \ + --exclude='package.json' \ + --exclude='milestones.yaml' \ + --exclude='project-stats.yaml' \ + ./ /tmp/gh-pages/ + + cd /tmp/gh-pages + git add -A + if git diff --cached --quiet; then + echo "Nothing to deploy — gh-pages is already up to date" + else + git commit -m "chore: deploy $(git rev-parse --short HEAD) to gh-pages" + git push origin gh-pages + fi diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index b45fca9..31e55be 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -18,18 +18,30 @@ jobs: steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 - name: Detect relevant file changes - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 id: filter - with: - filters: | - code: - - '**/*.js' - - '**/*.ts' - - '**/*.css' - - 'tests/**' - - 'package*.json' + run: | + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + BASE="${{ github.event.pull_request.base.sha }}" + elif [[ "${{ github.event_name }}" == "push" ]]; then + BASE="${{ github.event.before }}" + else + BASE="${{ github.event.merge_group.base_sha }}" + fi + # On a new branch (no prior commit) or unknown base, always run tests + if [[ -z "$BASE" || "$BASE" == "0000000000000000000000000000000000000000" ]]; then + echo "code=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + if git diff --name-only "$BASE" "${{ github.sha }}" \ + | grep -qE '\.(js|ts|css)$|^tests/|^package[^/]*\.json$'; then + echo "code=true" >> "$GITHUB_OUTPUT" + else + echo "code=false" >> "$GITHUB_OUTPUT" + fi test-e2e: needs: changes diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index 818c126..f547c61 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -4,6 +4,11 @@ on: pull_request: types: [opened, synchronize, reopened] +# Serialize gh-pages pushes per PR; cancel in-progress runs when a new push arrives. +concurrency: + group: "pages-preview-${{ github.event.number }}" + cancel-in-progress: true + permissions: contents: write pull-requests: write @@ -25,21 +30,54 @@ jobs: run: npm run screenshots - name: Deploy PR preview to gh-pages branch - uses: peaceiris/actions-gh-pages@4f9cc6602d3f66b9c108549d475ec49e8ef4d45e # v4.0.0 - with: - github_token: ${{ secrets.GITHUB_TOKEN }} - publish_dir: . - destination_dir: previews/pr-${{ github.event.number }} - # Preserve existing previews and production files - keep_files: true - # Exclude non-site files (keep in sync with deploy.yml). - # .gitignore is excluded so it is never deployed: if it lands in the gh-pages - # temp clone, peaceiris's `git add --all` would silently skip script.js, - # styles.css and the generated data files (all listed in .gitignore). - # Screenshots are excluded here because they are uploaded via the Content API - # in the next step; uploading before deploy would cause peaceiris to delete them. - # NOTE: deploy.yml shares the same base list minus .gitignore and screenshots. - exclude_assets: '.github,.gitignore,node_modules,tests,scripts,package-lock.json,package.json,milestones.yaml,project-stats.yaml,screenshots' + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + + # Fetch the existing gh-pages branch (silently skip if it doesn't exist yet) + git fetch origin gh-pages:refs/remotes/origin/gh-pages 2>/dev/null || true + + # Check out gh-pages into a worktree so we can update it in-place. + # This preserves production files and other previews — equivalent to keep_files: true. + if git rev-parse --verify refs/remotes/origin/gh-pages >/dev/null 2>&1; then + git branch --force gh-pages refs/remotes/origin/gh-pages + git worktree add /tmp/gh-pages gh-pages + else + git worktree add --orphan -b gh-pages /tmp/gh-pages + fi + + DEST="/tmp/gh-pages/previews/pr-${{ github.event.number }}" + mkdir -p "$DEST" + + # Sync built files into the preview subdirectory. + # Keep the exclude list in sync with deploy.yml (screenshots is an additional + # preview-only exclusion: they are uploaded via the Content API in the next step, + # so rsync must not overwrite them before that step runs). + # .gitignore is excluded so it never lands in gh-pages; if it did, + # `git add -A` would silently skip script.js, styles.css and the + # generated data files (all listed in .gitignore). + rsync -a \ + --exclude='.git' \ + --exclude='.github' \ + --exclude='.gitignore' \ + --exclude='node_modules' \ + --exclude='tests' \ + --exclude='scripts' \ + --exclude='package-lock.json' \ + --exclude='package.json' \ + --exclude='milestones.yaml' \ + --exclude='project-stats.yaml' \ + --exclude='screenshots' \ + ./ "$DEST/" + + cd /tmp/gh-pages + git add -A + if git diff --cached --quiet; then + echo "Nothing to deploy — preview is already up to date" + else + git commit -m "chore: deploy preview for PR #${{ github.event.number }}" + git push origin gh-pages + fi - name: Upload screenshots to GitHub id: upload-screenshots diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 71fdb00..2ce6b79 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -22,19 +22,31 @@ jobs: steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 - name: Detect relevant file changes - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 if: github.event_name != 'schedule' id: filter - with: - filters: | - code: - - '**/*.js' - - '**/*.ts' - - '**/*.css' - - 'tests/**' - - 'package*.json' + run: | + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + BASE="${{ github.event.pull_request.base.sha }}" + elif [[ "${{ github.event_name }}" == "push" ]]; then + BASE="${{ github.event.before }}" + else + BASE="${{ github.event.merge_group.base_sha }}" + fi + # On a new branch (no prior commit) or unknown base, always run tests + if [[ -z "$BASE" || "$BASE" == "0000000000000000000000000000000000000000" ]]; then + echo "code=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + if git diff --name-only "$BASE" "${{ github.sha }}" \ + | grep -qE '\.(js|ts|css)$|^tests/|^package[^/]*\.json$'; then + echo "code=true" >> "$GITHUB_OUTPUT" + else + echo "code=false" >> "$GITHUB_OUTPUT" + fi test: needs: changes diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 75b4ea4..f86cc8e 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -112,6 +112,7 @@ Every PR description (written by a human or agent) must follow this structure: | S1 | All dynamic strings inserted via `innerHTML` must be escaped with `escHtml()` in `src/js/05-security.js`. Never assign untrusted data directly to `innerHTML`. | AGENTS.md | | S2 | GitHub Actions `uses:` references must be pinned to a full commit SHA with the semver tag as an inline comment (`@abc1234 # v3.1.0`). Mutable tags (`@v3`) can be silently redirected, creating a supply-chain risk. | AGENTS.md | | S3 | Dependabot is configured to open weekly PRs for GitHub Actions SHA bumps. Do not skip or dismiss those PRs. | AGENTS.md | +| S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | #— | --- @@ -143,6 +144,15 @@ Entries are grouped by release. Add new entries at the top of the appropriate re ### v1.7.x +#### PR #— chore: replace peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell + +- **Problem:** Two third-party GitHub Actions (`peaceiris/actions-gh-pages` and `dorny/paths-filter`) added supply-chain risk from less-known organisations when native equivalents exist. +- **Approach:** Replaced `peaceiris/actions-gh-pages` in `deploy.yml` and `preview.yml` with native `git worktree` + `rsync` shell steps that replicate `keep_files: true`, `destination_dir`, and `exclude_assets`. Replaced `dorny/paths-filter` in `unit-tests.yml` and `e2e-tests.yml` with a native `git diff --name-only` shell step and `fetch-depth: 0` checkout. Also added a per-PR `concurrency` group to `preview.yml` to serialize gh-pages pushes. +- **Learning:** `git worktree add` is the cleanest way to check out a second branch (e.g. `gh-pages`) alongside the current checkout without a separate clone. Use `git branch --force refs/remotes/origin/` first so the worktree has a local tracking branch to push from. Always use `touch .nojekyll` in the gh-pages worktree to prevent Jekyll processing — peaceiris did this automatically. (→ S4) +- **Key files:** `.github/workflows/deploy.yml`, `.github/workflows/preview.yml`, `.github/workflows/unit-tests.yml`, `.github/workflows/e2e-tests.yml` + +--- + #### PR #103 — feat: implement Token Horoscope daily satirical AI horoscope (Phase 3 PRD #1) - **Problem:** The site had no daily-rotating content to drive return visits; Phase 3 PRD #1 (Token Horoscope) was the highest-impact lowest-effort unimplemented feature. From b0bc6b7185b7868f5314a49270489b1ca6a13ffe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:21:34 +0000 Subject: [PATCH 02/10] refactor: extract reusable detect-changes workflow, fix injection, add shell tests Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/4f60af0f-87e2-4078-a371-9a2ec9f09868 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- .github/scripts/detect-changes.sh | 49 +++++ .github/workflows/detect-changes.yml | 84 +++++++ .github/workflows/e2e-tests.yml | 34 +-- .github/workflows/unit-tests.yml | 39 +--- docs/LEARNINGS.md | 12 +- package.json | 1 + tests/detect-changes.test.sh | 317 +++++++++++++++++++++++++++ 7 files changed, 474 insertions(+), 62 deletions(-) create mode 100644 .github/scripts/detect-changes.sh create mode 100644 .github/workflows/detect-changes.yml create mode 100644 tests/detect-changes.test.sh diff --git a/.github/scripts/detect-changes.sh b/.github/scripts/detect-changes.sh new file mode 100644 index 0000000..4db6b1c --- /dev/null +++ b/.github/scripts/detect-changes.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# detect-changes.sh — determines whether relevant source files changed between BASE and HEAD. +# +# All GitHub context is read from environment variables (never interpolated from +# ${{ }} expressions inside a run: block) so the script is immune to +# GitHub Actions expression injection. +# +# Inputs via environment variables: +# GH_EVENT_NAME — github.event_name from the calling workflow +# GH_PR_BASE_SHA — github.event.pull_request.base.sha (for pull_request events) +# GH_PUSH_BEFORE — github.event.before (for push events) +# GH_MERGE_BASE_SHA — github.event.merge_group.base_sha (for merge_group events) +# GH_SHA — github.sha (HEAD commit) +# GITHUB_OUTPUT — output file path (set automatically by the GitHub Actions runner) +# +# Output written to $GITHUB_OUTPUT: +# code=true relevant source files changed — proceed with tests / CI +# code=false no relevant changes detected — skip tests / CI + +set -euo pipefail + +case "${GH_EVENT_NAME}" in + pull_request) BASE="${GH_PR_BASE_SHA}" ;; + push) BASE="${GH_PUSH_BEFORE}" ;; + *) BASE="${GH_MERGE_BASE_SHA:-}" ;; +esac + +# An empty or all-zeros SHA means there is no prior commit to compare against +# (e.g. first push on a brand-new branch). Treat as "changed" so CI always runs. +if [[ -z "${BASE}" || "${BASE}" == "0000000000000000000000000000000000000000" ]]; then + echo "code=true" >> "${GITHUB_OUTPUT}" + exit 0 +fi + +# Capture the changed file list. If git fails (e.g. the ref is unknown or was +# tampered with) log the error and fall back to code=true so CI is never silently +# skipped. +diff_files=$(git diff --name-only "${BASE}" "${GH_SHA}") || { + echo "detect-changes: git diff failed for BASE '${BASE}' — defaulting to code=true" >&2 + echo "code=true" >> "${GITHUB_OUTPUT}" + exit 0 +} + +if echo "${diff_files}" | grep -qE '\.(js|ts|css)$|^tests/|^package[^/]*\.json$'; then + echo "code=true" >> "${GITHUB_OUTPUT}" +else + echo "code=false" >> "${GITHUB_OUTPUT}" +fi + diff --git a/.github/workflows/detect-changes.yml b/.github/workflows/detect-changes.yml new file mode 100644 index 0000000..a2c6158 --- /dev/null +++ b/.github/workflows/detect-changes.yml @@ -0,0 +1,84 @@ +name: Detect Relevant Changes + +# Reusable workflow — determines whether relevant source files (JS/TS/CSS, tests, +# package manifests) were modified between the base and HEAD commits. +# +# Usage in a calling workflow: +# +# jobs: +# changes: +# uses: ./.github/workflows/detect-changes.yml +# with: +# event_name: ${{ github.event_name }} +# always_run: ${{ github.event_name == 'schedule' }} # optional +# +# test: +# needs: changes +# if: needs.changes.outputs.code == 'true' +# +# Why event_name is a required input: +# Inside a reusable workflow triggered via workflow_call, github.event_name is +# always "workflow_call", not the original triggering event (push/pull_request/…). +# The caller must pass the real event name so the script can resolve the correct +# base SHA from the inherited github.event payload. + +on: + workflow_call: + inputs: + event_name: + description: > + The event name from the calling workflow (github.event_name). + Required because github.event_name is always "workflow_call" inside a + reusable workflow, not the original triggering event. + required: true + type: string + always_run: + description: > + When true, skip the diff check and unconditionally output code=true. + Use this for scheduled runs where there is no diff to compare against. + required: false + type: boolean + default: false + outputs: + code: + description: > + "true" when relevant source files changed (or always_run is set); + "false" when no relevant changes were detected. + value: ${{ jobs.detect.outputs.code }} + +permissions: + contents: read + +jobs: + detect: + runs-on: ubuntu-latest + outputs: + code: ${{ steps.result.outputs.code }} + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + # Full history is required so the base commit is reachable for git diff. + fetch-depth: 0 + + - name: Detect relevant file changes + id: result + # GitHub context values are set as env vars here in the env: block. + # ${{ }} expressions in env: are evaluated by the GitHub Actions runner + # before the shell starts, so they are never interpreted as shell code. + # This is the correct pattern for passing context to shell scripts; by + # contrast, interpolating ${{ }} directly inside run: text would allow + # a malicious value to break out of quotes and execute arbitrary commands. + env: + ALWAYS_RUN: ${{ inputs.always_run }} + GH_EVENT_NAME: ${{ inputs.event_name }} + GH_PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + GH_PUSH_BEFORE: ${{ github.event.before }} + GH_MERGE_BASE_SHA: ${{ github.event.merge_group.base_sha }} + GH_SHA: ${{ github.sha }} + run: | + if [[ "${ALWAYS_RUN}" == "true" ]]; then + echo "code=true" >> "$GITHUB_OUTPUT" + else + bash .github/scripts/detect-changes.sh + fi diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 31e55be..cb80e5f 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -12,36 +12,9 @@ permissions: jobs: changes: - runs-on: ubuntu-latest - outputs: - code: ${{ steps.filter.outputs.code }} - steps: - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - fetch-depth: 0 - - - name: Detect relevant file changes - id: filter - run: | - if [[ "${{ github.event_name }}" == "pull_request" ]]; then - BASE="${{ github.event.pull_request.base.sha }}" - elif [[ "${{ github.event_name }}" == "push" ]]; then - BASE="${{ github.event.before }}" - else - BASE="${{ github.event.merge_group.base_sha }}" - fi - # On a new branch (no prior commit) or unknown base, always run tests - if [[ -z "$BASE" || "$BASE" == "0000000000000000000000000000000000000000" ]]; then - echo "code=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - if git diff --name-only "$BASE" "${{ github.sha }}" \ - | grep -qE '\.(js|ts|css)$|^tests/|^package[^/]*\.json$'; then - echo "code=true" >> "$GITHUB_OUTPUT" - else - echo "code=false" >> "$GITHUB_OUTPUT" - fi + uses: ./.github/workflows/detect-changes.yml + with: + event_name: ${{ github.event_name }} test-e2e: needs: changes @@ -72,3 +45,4 @@ jobs: exit 1 fi echo "E2E tests passed or were skipped (no relevant files changed)" + diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 2ce6b79..525678b 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -14,39 +14,12 @@ permissions: jobs: changes: - runs-on: ubuntu-latest - outputs: + uses: ./.github/workflows/detect-changes.yml + with: + event_name: ${{ github.event_name }} # On scheduled runs there is no diff to compare — always run tests. # On push/PR, only run when relevant files changed. - code: ${{ github.event_name == 'schedule' || steps.filter.outputs.code == 'true' }} - steps: - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - fetch-depth: 0 - - - name: Detect relevant file changes - if: github.event_name != 'schedule' - id: filter - run: | - if [[ "${{ github.event_name }}" == "pull_request" ]]; then - BASE="${{ github.event.pull_request.base.sha }}" - elif [[ "${{ github.event_name }}" == "push" ]]; then - BASE="${{ github.event.before }}" - else - BASE="${{ github.event.merge_group.base_sha }}" - fi - # On a new branch (no prior commit) or unknown base, always run tests - if [[ -z "$BASE" || "$BASE" == "0000000000000000000000000000000000000000" ]]; then - echo "code=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - if git diff --name-only "$BASE" "${{ github.sha }}" \ - | grep -qE '\.(js|ts|css)$|^tests/|^package[^/]*\.json$'; then - echo "code=true" >> "$GITHUB_OUTPUT" - else - echo "code=false" >> "$GITHUB_OUTPUT" - fi + always_run: ${{ github.event_name == 'schedule' }} test: needs: changes @@ -65,6 +38,9 @@ jobs: - name: Run tests with coverage run: npm run test:ci + - name: Run shell script tests + run: npm run test:shell + - name: Upload coverage to Codecov uses: codecov/codecov-action@57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 # v6.0.0 with: @@ -85,3 +61,4 @@ jobs: exit 1 fi echo "Unit tests passed or were skipped (no relevant files changed)" + diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index f86cc8e..d9b290c 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -113,6 +113,7 @@ Every PR description (written by a human or agent) must follow this structure: | S2 | GitHub Actions `uses:` references must be pinned to a full commit SHA with the semver tag as an inline comment (`@abc1234 # v3.1.0`). Mutable tags (`@v3`) can be silently redirected, creating a supply-chain risk. | AGENTS.md | | S3 | Dependabot is configured to open weekly PRs for GitHub Actions SHA bumps. Do not skip or dismiss those PRs. | AGENTS.md | | S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | #— | +| S5 | Always pass GitHub context values to shell scripts via `env:` vars (e.g. `GH_SHA: ${{ github.sha }}`), never by interpolating `${{ }}` directly inside `run:`. Inline interpolation allows expression injection if an attacker controls the context value. | #— | --- @@ -144,7 +145,16 @@ Entries are grouped by release. Add new entries at the top of the appropriate re ### v1.7.x -#### PR #— chore: replace peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell +#### PR #— refactor: extract reusable detect-changes workflow + fix injection + shell tests + +- **Problem:** The `git diff` path-filter logic was duplicated verbatim in both `unit-tests.yml` and `e2e-tests.yml`, and each copy interpolated GitHub context values (`${{ github.event_name }}`, `${{ github.sha }}`, etc.) directly into `run:` scripts — an expression-injection anti-pattern. +- **Approach:** Extracted the logic into `.github/scripts/detect-changes.sh` (reads all GitHub context from env vars, never from `${{ }}` interpolation). Wrapped it in a reusable `workflow_call` workflow (`.github/workflows/detect-changes.yml`) with `event_name` and `always_run` inputs and a `code` output. Both callers now use `uses: ./.github/workflows/detect-changes.yml`. Added 19 bash unit tests in `tests/detect-changes.test.sh` and an `npm run test:shell` script. The `test` job in `unit-tests.yml` runs shell tests in CI. The git failure fallback now outputs `code=true` (safe: run CI) rather than `code=false` (unsafe: silently skip CI). +- **Learning:** Pass `event_name` as an explicit `string` input to reusable workflows because `github.event_name` inside a `workflow_call` callee is always `"workflow_call"`, not the original triggering event. The `github.event` payload and `github.sha` are inherited correctly. Always default to the "run CI" safe side when git diff fails on an unresolvable ref. (→ S4, S5) +- **Key files:** `.github/scripts/detect-changes.sh`, `.github/workflows/detect-changes.yml`, `tests/detect-changes.test.sh`, `package.json`, `.github/workflows/unit-tests.yml`, `.github/workflows/e2e-tests.yml` + +--- + + - **Problem:** Two third-party GitHub Actions (`peaceiris/actions-gh-pages` and `dorny/paths-filter`) added supply-chain risk from less-known organisations when native equivalents exist. - **Approach:** Replaced `peaceiris/actions-gh-pages` in `deploy.yml` and `preview.yml` with native `git worktree` + `rsync` shell steps that replicate `keep_files: true`, `destination_dir`, and `exclude_assets`. Replaced `dorny/paths-filter` in `unit-tests.yml` and `e2e-tests.yml` with a native `git diff --name-only` shell step and `fetch-depth: 0` checkout. Also added a per-PR `concurrency` group to `preview.yml` to serialize gh-pages pushes. diff --git a/package.json b/package.json index cbd785b..ec5e167 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "test": "jest --coverage", "pretest:ci": "npm run build:milestones && npm run build:js", "test:ci": "jest --ci --coverage", + "test:shell": "bash tests/detect-changes.test.sh", "test:e2e": "playwright test", "test:e2e:ui": "playwright test --ui", "build:milestones": "node scripts/build-milestones.js", diff --git a/tests/detect-changes.test.sh b/tests/detect-changes.test.sh new file mode 100644 index 0000000..3eb2499 --- /dev/null +++ b/tests/detect-changes.test.sh @@ -0,0 +1,317 @@ +#!/usr/bin/env bash +# tests/detect-changes.test.sh +# +# Unit tests for .github/scripts/detect-changes.sh +# +# Run with: bash tests/detect-changes.test.sh +# +# Tests are self-contained: git-based cases create an isolated temporary +# repository in /tmp so they never depend on the real repo's history. + +set -uo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SCRIPT="${REPO_ROOT}/.github/scripts/detect-changes.sh" + +PASS=0 +FAIL=0 +TMPGIT="" + +# Ensure the temp git repo is always cleaned up, even on unexpected exit. +trap 'rm -rf "${TMPGIT:-}"' EXIT + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +# assert_output +assert_output() { + local desc="$1" expected="$2" tmpout="$3" + local actual + actual=$(grep '^code=' "${tmpout}" 2>/dev/null | tail -1 | cut -d= -f2 || true) + if [[ "${actual}" == "${expected}" ]]; then + echo " ✅ ${desc}" + PASS=$((PASS + 1)) + else + echo " ❌ ${desc}" + echo " expected : '${expected}'" + echo " got : '${actual}'" + FAIL=$((FAIL + 1)) + fi +} + +# run_script [KEY=VAL …] +# Runs detect-changes.sh in with the given env vars plus GITHUB_OUTPUT. +run_script() { + local tmpout="$1" dir="$2" + shift 2 + (cd "${dir}" && env "$@" GITHUB_OUTPUT="${tmpout}" bash "${SCRIPT}") 2>/dev/null || true +} + +# make_git_repo — creates a temp git repo and sets TMPGIT to its path +make_git_repo() { + TMPGIT=$(mktemp -d) + git -C "${TMPGIT}" init -q + git -C "${TMPGIT}" config user.email "test@example.com" + git -C "${TMPGIT}" config user.name "Test" +} + +# git_commit [file content pairs …] +# Creates or updates files then commits. Pairs: path content +git_commit() { + local repo="$1" msg="$2" + shift 2 + while [[ $# -ge 2 ]]; do + local path="$1" content="$2" + shift 2 + mkdir -p "${repo}/$(dirname "${path}")" + printf '%s\n' "${content}" > "${repo}/${path}" + done + git -C "${repo}" add -A + git -C "${repo}" commit -qm "${msg}" +} + +# --------------------------------------------------------------------------- +# Early-exit tests (no git diff needed) +# --------------------------------------------------------------------------- + +echo "" +echo "=== detect-changes.sh — early-exit (no diff) ===" +echo "" + +tmpout=$(mktemp) + +# 1. Empty BASE — push event with missing GH_PUSH_BEFORE +run_script "${tmpout}" "${REPO_ROOT}" \ + GH_EVENT_NAME="push" GH_PUSH_BEFORE="" \ + GH_SHA="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" \ + GH_PR_BASE_SHA="" GH_MERGE_BASE_SHA="" +assert_output "push: empty BASE → code=true" "true" "${tmpout}" +: > "${tmpout}" + +# 2. All-zeros BASE — first push on a new branch +run_script "${tmpout}" "${REPO_ROOT}" \ + GH_EVENT_NAME="push" \ + GH_PUSH_BEFORE="0000000000000000000000000000000000000000" \ + GH_SHA="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" \ + GH_PR_BASE_SHA="" GH_MERGE_BASE_SHA="" +assert_output "push: all-zeros BASE (new branch) → code=true" "true" "${tmpout}" +: > "${tmpout}" + +# 3. Unexpected event_name with no resolvable SHAs → safe fallback +run_script "${tmpout}" "${REPO_ROOT}" \ + GH_EVENT_NAME="workflow_dispatch" \ + GH_PR_BASE_SHA="" GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" \ + GH_SHA="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +assert_output "unknown event + empty SHA → code=true" "true" "${tmpout}" +: > "${tmpout}" + +rm -f "${tmpout}" + +# --------------------------------------------------------------------------- +# Git diff tests — pull_request event +# --------------------------------------------------------------------------- + +echo "" +echo "=== detect-changes.sh — pull_request events ===" +echo "" + +make_git_repo + +# BASE commit — only a README +git_commit "${TMPGIT}" "init" "README.md" "initial" +BASE_SHA=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD A — adds a .js file +git_commit "${TMPGIT}" "add js" "app.js" "console.log('hi')" +HEAD_JS=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD B — adds a .ts file (built on HEAD_JS) +git_commit "${TMPGIT}" "add ts" "lib.ts" "export const x = 1" +HEAD_TS=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD C — adds a .css file +git_commit "${TMPGIT}" "add css" "style.css" "body{}" +HEAD_CSS=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD D — changes tests/ +git_commit "${TMPGIT}" "add test" "tests/foo.test.js" "test('x',()=>{})" +HEAD_TESTS=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD E — changes package.json +git_commit "${TMPGIT}" "pkg" "package.json" '{"name":"x"}' +HEAD_PKG=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD F — changes package-lock.json +git_commit "${TMPGIT}" "lock" "package-lock.json" '{"lockfileVersion":3}' +HEAD_LOCK=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD G — only a docs change (irrelevant) +git_commit "${TMPGIT}" "docs" "README.md" "updated" +HEAD_DOCS=$(git -C "${TMPGIT}" rev-parse HEAD) + +# HEAD H — only a YAML change (irrelevant) +git_commit "${TMPGIT}" "yaml" "config.yaml" "key: value" +HEAD_YAML=$(git -C "${TMPGIT}" rev-parse HEAD) + +tmpout=$(mktemp) + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${BASE_SHA}" GH_SHA="${HEAD_JS}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: .js change → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${HEAD_JS}" GH_SHA="${HEAD_TS}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: .ts change → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${HEAD_TS}" GH_SHA="${HEAD_CSS}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: .css change → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${HEAD_CSS}" GH_SHA="${HEAD_TESTS}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: tests/ change → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${HEAD_TESTS}" GH_SHA="${HEAD_PKG}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: package.json change → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${HEAD_PKG}" GH_SHA="${HEAD_LOCK}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: package-lock.json change → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${HEAD_LOCK}" GH_SHA="${HEAD_DOCS}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: only README change → code=false" "false" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="pull_request" GH_PR_BASE_SHA="${HEAD_DOCS}" GH_SHA="${HEAD_YAML}" \ + GH_PUSH_BEFORE="" GH_MERGE_BASE_SHA="" +assert_output "pull_request: only .yaml change → code=false" "false" "${tmpout}"; : > "${tmpout}" + +# --------------------------------------------------------------------------- +# Push event — uses GH_PUSH_BEFORE (not GH_PR_BASE_SHA) +# --------------------------------------------------------------------------- + +echo "" +echo "=== detect-changes.sh — push events ===" +echo "" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="push" GH_PUSH_BEFORE="${BASE_SHA}" GH_SHA="${HEAD_JS}" \ + GH_PR_BASE_SHA="" GH_MERGE_BASE_SHA="" +assert_output "push: .js change via GH_PUSH_BEFORE → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="push" GH_PUSH_BEFORE="${HEAD_DOCS}" GH_SHA="${HEAD_YAML}" \ + GH_PR_BASE_SHA="" GH_MERGE_BASE_SHA="" +assert_output "push: only .yaml change via GH_PUSH_BEFORE → code=false" "false" "${tmpout}"; : > "${tmpout}" + +# GH_PR_BASE_SHA is ignored for push events — if we set it to something that +# WOULD trigger code=true, but it should NOT because the event is "push". +git_commit "${TMPGIT}" "another readme" "README.md" "again" +HEAD_EXTRA=$(git -C "${TMPGIT}" rev-parse HEAD) + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="push" \ + GH_PUSH_BEFORE="${HEAD_YAML}" GH_SHA="${HEAD_EXTRA}" \ + GH_PR_BASE_SHA="${BASE_SHA}" GH_MERGE_BASE_SHA="" +assert_output "push: ignores GH_PR_BASE_SHA (only README changed) → code=false" "false" "${tmpout}"; : > "${tmpout}" + +# --------------------------------------------------------------------------- +# merge_group event — uses GH_MERGE_BASE_SHA +# --------------------------------------------------------------------------- + +echo "" +echo "=== detect-changes.sh — merge_group events ===" +echo "" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="merge_group" GH_MERGE_BASE_SHA="${BASE_SHA}" GH_SHA="${HEAD_JS}" \ + GH_PR_BASE_SHA="" GH_PUSH_BEFORE="" +assert_output "merge_group: .js change → code=true" "true" "${tmpout}"; : > "${tmpout}" + +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="merge_group" GH_MERGE_BASE_SHA="${HEAD_DOCS}" GH_SHA="${HEAD_YAML}" \ + GH_PR_BASE_SHA="" GH_PUSH_BEFORE="" +assert_output "merge_group: only .yaml change → code=false" "false" "${tmpout}"; : > "${tmpout}" + +# merge_group ignores GH_PR_BASE_SHA and GH_PUSH_BEFORE +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="merge_group" \ + GH_MERGE_BASE_SHA="${HEAD_EXTRA}" GH_SHA="${HEAD_EXTRA}" \ + GH_PR_BASE_SHA="${BASE_SHA}" GH_PUSH_BEFORE="${BASE_SHA}" +assert_output "merge_group: ignores PR/push SHAs (no diff HEAD..HEAD) → code=false" "false" "${tmpout}"; : > "${tmpout}" + +# --------------------------------------------------------------------------- +# Injection safety test — malformed BASE does not execute arbitrary code +# --------------------------------------------------------------------------- + +echo "" +echo "=== detect-changes.sh — injection safety ===" +echo "" + +INJECTION_MARKER_FILE="/tmp/detect-changes-injection-$$" +rm -f "${INJECTION_MARKER_FILE}" + +# Craft a value that contains an unescaped command substitution pattern. +# The \$ below prevents the TEST SCRIPT from expanding the $(…) — it is NOT +# a backslash that ends up in the payload. The actual value stored in +# INJECTION_PAYLOAD is the literal string "$(touch /tmp/...)" (no backslash), +# which is a real command substitution pattern. When passed to the script via +# an env var, bash uses it as a literal argument to git ("${BASE}" is properly +# quoted) so the command substitution is never evaluated. +INJECTION_PAYLOAD="\$(touch ${INJECTION_MARKER_FILE})" + +# Run the script; it is expected to exit non-zero (invalid git ref) — || true so +# the test script itself doesn't abort. +run_script "${tmpout}" "${TMPGIT}" \ + GH_EVENT_NAME="push" \ + GH_PUSH_BEFORE="${INJECTION_PAYLOAD}" \ + GH_SHA="${HEAD_JS}" \ + GH_PR_BASE_SHA="" GH_MERGE_BASE_SHA="" + +# 1. The injected command must NOT have run. +if [[ ! -f "${INJECTION_MARKER_FILE}" ]]; then + echo " ✅ injection: command was not executed" + PASS=$((PASS + 1)) +else + echo " ❌ injection: marker file was created — injection succeeded!" + FAIL=$((FAIL + 1)) + rm -f "${INJECTION_MARKER_FILE}" +fi + +# 2. The output file must contain code=true (safe fallback when git ref is unresolvable). +actual_code=$(grep '^code=' "${tmpout}" 2>/dev/null | tail -1 | cut -d= -f2 || true) +if [[ "${actual_code}" == "true" ]]; then + echo " ✅ injection: safe fallback code=true written (CI not silently skipped)" + PASS=$((PASS + 1)) +else + echo " ❌ injection: expected code=true safe fallback, got '${actual_code}'" + FAIL=$((FAIL + 1)) +fi +: > "${tmpout}" + +rm -f "${tmpout}" +rm -rf "${TMPGIT}" + +# --------------------------------------------------------------------------- +# Summary +# --------------------------------------------------------------------------- + +echo "" +echo "=======================================" +echo "Results: ${PASS} passed, ${FAIL} failed" +echo "=======================================" +echo "" + +if [[ ${FAIL} -gt 0 ]]; then + exit 1 +fi From 1be3ded56e0b02ad5d437d10797a2b015a4d0199 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:49:14 +0000 Subject: [PATCH 03/10] feat: composite actions for gh-pages-deploy and paths-filter with codecov coverage Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/0047cdb7-30cf-42d6-b536-50e08c386248 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- .github/actions/gh-pages-deploy/action.yml | 106 ++++ .github/actions/gh-pages-deploy/deploy.sh | 111 ++++ .github/actions/paths-filter/action.yml | 121 +++++ .github/actions/paths-filter/filter.js | 306 +++++++++++ .github/workflows/deploy.yml | 64 +-- .github/workflows/detect-changes.yml | 37 +- .github/workflows/preview-cleanup.yml | 6 +- .github/workflows/preview.yml | 66 +-- docs/LEARNINGS.md | 12 +- package.json | 11 +- tests/gh-pages-deploy.test.sh | 369 +++++++++++++ tests/paths-filter.test.js | 576 +++++++++++++++++++++ 12 files changed, 1669 insertions(+), 116 deletions(-) create mode 100644 .github/actions/gh-pages-deploy/action.yml create mode 100644 .github/actions/gh-pages-deploy/deploy.sh create mode 100644 .github/actions/paths-filter/action.yml create mode 100644 .github/actions/paths-filter/filter.js create mode 100644 tests/gh-pages-deploy.test.sh create mode 100644 tests/paths-filter.test.js diff --git a/.github/actions/gh-pages-deploy/action.yml b/.github/actions/gh-pages-deploy/action.yml new file mode 100644 index 0000000..5c89c9b --- /dev/null +++ b/.github/actions/gh-pages-deploy/action.yml @@ -0,0 +1,106 @@ +name: 'Deploy to GitHub Pages' +description: > + Publish a local directory to a GitHub Pages branch using git worktree and + rsync. Drop-in composite-action replacement for peaceiris/actions-gh-pages, + maintaining the same core inputs. + +# Usage example: +# +# - uses: actions/checkout@... +# with: +# fetch-depth: 0 +# +# - uses: ./.github/actions/gh-pages-deploy +# with: +# github_token: ${{ secrets.GITHUB_TOKEN }} +# publish_dir: ./dist +# destination_dir: '' +# exclude_assets: | +# .github +# node_modules + +inputs: + github_token: + description: > + GitHub token used to authenticate git push. When provided, the action + rewrites the origin remote URL to embed the token so push works without + relying on credentials configured by actions/checkout. + required: true + + publish_branch: + description: 'Target branch to deploy to.' + required: false + default: 'gh-pages' + + publish_dir: + description: > + Local directory whose contents are published to the branch. + required: false + default: 'public' + + destination_dir: + description: > + Subdirectory within publish_branch to deploy into. + Leave empty to deploy to the branch root. + required: false + default: '' + + keep_files: + description: > + When "true", preserve existing files in the destination directory that + are not present in publish_dir. + When "false" (default), the destination directory is cleared before + syncing — only applies to subdirectory deployments (destination_dir set). + Root deployments always preserve other content in the branch (e.g. the + previews/ directory). + required: false + default: 'false' + + exclude_assets: + description: > + Newline- or comma-separated list of file/directory names to exclude from + the deployment (passed as --exclude flags to rsync). + required: false + default: '.github' + + user_name: + description: 'Git commit author name.' + required: false + default: 'github-actions[bot]' + + user_email: + description: 'Git commit author email.' + required: false + default: 'github-actions[bot]@users.noreply.github.com' + + commit_message: + description: 'Commit message for the deployment commit.' + required: false + default: 'chore: deploy to gh-pages' + + disable_nojekyll: + description: > + Set to "true" to skip creating the .nojekyll file, enabling Jekyll + processing on the published branch. + required: false + default: 'false' + +runs: + using: composite + steps: + - name: Deploy to branch + # All inputs are assigned to env vars so the shell script never receives + # user-controlled data via command-line interpolation. + env: + INPUT_GITHUB_TOKEN: ${{ inputs.github_token }} + INPUT_PUBLISH_BRANCH: ${{ inputs.publish_branch }} + INPUT_PUBLISH_DIR: ${{ inputs.publish_dir }} + INPUT_DESTINATION_DIR: ${{ inputs.destination_dir }} + INPUT_KEEP_FILES: ${{ inputs.keep_files }} + INPUT_EXCLUDE_ASSETS: ${{ inputs.exclude_assets }} + INPUT_USER_NAME: ${{ inputs.user_name }} + INPUT_USER_EMAIL: ${{ inputs.user_email }} + INPUT_COMMIT_MESSAGE: ${{ inputs.commit_message }} + INPUT_DISABLE_NOJEKYLL: ${{ inputs.disable_nojekyll }} + run: bash "$GITHUB_ACTION_PATH/deploy.sh" + shell: bash diff --git a/.github/actions/gh-pages-deploy/deploy.sh b/.github/actions/gh-pages-deploy/deploy.sh new file mode 100644 index 0000000..dd2653a --- /dev/null +++ b/.github/actions/gh-pages-deploy/deploy.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# deploy.sh — publishes a local directory to a GitHub Pages branch. +# Part of the gh-pages-deploy composite action. +# +# All configuration is read from environment variables (injection-safe). +# Never interpolate ${{ }} expressions directly into this script. +# +# Environment variables: +# INPUT_GITHUB_TOKEN — GitHub token for authenticating git push +# INPUT_PUBLISH_BRANCH — target branch (default: gh-pages) +# INPUT_PUBLISH_DIR — local source directory (default: public) +# INPUT_DESTINATION_DIR — subdirectory within publish_branch (default: '') +# INPUT_KEEP_FILES — preserve existing files in destination (default: false) +# INPUT_EXCLUDE_ASSETS — newline/comma-separated paths to exclude via rsync +# INPUT_USER_NAME — git commit author name +# INPUT_USER_EMAIL — git commit author email +# INPUT_COMMIT_MESSAGE — commit message +# INPUT_DISABLE_NOJEKYLL— if 'true', skip creating .nojekyll + +set -euo pipefail + +PUBLISH_BRANCH="${INPUT_PUBLISH_BRANCH:-gh-pages}" +PUBLISH_DIR="${INPUT_PUBLISH_DIR:-public}" +DESTINATION_DIR="${INPUT_DESTINATION_DIR:-}" +KEEP_FILES="${INPUT_KEEP_FILES:-false}" +USER_NAME="${INPUT_USER_NAME:-github-actions[bot]}" +USER_EMAIL="${INPUT_USER_EMAIL:-github-actions[bot]@users.noreply.github.com}" +COMMIT_MESSAGE="${INPUT_COMMIT_MESSAGE:-chore: deploy to ${PUBLISH_BRANCH}}" +DISABLE_NOJEKYLL="${INPUT_DISABLE_NOJEKYLL:-false}" +EXCLUDE_ASSETS="${INPUT_EXCLUDE_ASSETS:-.github}" + +WORKTREE_DIR="$(mktemp -d)" + +git config user.name "${USER_NAME}" +git config user.email "${USER_EMAIL}" + +# If a GitHub token is provided, configure the remote URL to use it so that +# git push works even when actions/checkout was not called with persist-credentials. +if [[ -n "${INPUT_GITHUB_TOKEN:-}" ]]; then + REPO_URL=$(git remote get-url origin) + # Convert SSH to HTTPS if needed (git@github.com:owner/repo.git → https://…) + if [[ "${REPO_URL}" == git@github.com:* ]]; then + REPO_URL="${REPO_URL/#git@github.com:/https://github.com/}" + fi + # Inject the token as credentials in the HTTPS URL + AUTHED_URL="${REPO_URL/#https:\/\//https://x-access-token:${INPUT_GITHUB_TOKEN}@}" + git remote set-url origin "${AUTHED_URL}" +fi + +# Fetch the existing publish branch (silently skip if it doesn't exist yet) +git fetch origin "${PUBLISH_BRANCH}:refs/remotes/origin/${PUBLISH_BRANCH}" 2>/dev/null || true + +# Check out the publish branch into a separate worktree so we can update it +# without leaving the main checkout in a detached-HEAD state. +if git rev-parse --verify "refs/remotes/origin/${PUBLISH_BRANCH}" >/dev/null 2>&1; then + git branch --force "${PUBLISH_BRANCH}" "refs/remotes/origin/${PUBLISH_BRANCH}" + git worktree add "${WORKTREE_DIR}" "${PUBLISH_BRANCH}" +else + # First-ever deploy: create an orphan branch + git worktree add --orphan -b "${PUBLISH_BRANCH}" "${WORKTREE_DIR}" +fi + +# Determine the destination path within the worktree +if [[ -n "${DESTINATION_DIR}" ]]; then + DEST="${WORKTREE_DIR}/${DESTINATION_DIR}" +else + DEST="${WORKTREE_DIR}" +fi + +mkdir -p "${DEST}" + +# When keep_files is false and deploying to a subdirectory, clear stale content +# so deleted files don't linger. Root deployments always preserve other content +# (e.g. the previews/ directory) regardless of keep_files. +if [[ "${KEEP_FILES}" != "true" && -n "${DESTINATION_DIR}" ]]; then + rm -rf "${DEST:?}/"* 2>/dev/null || true +fi + +# Ensure Jekyll processing is disabled (unless explicitly opted out) +if [[ "${DISABLE_NOJEKYLL}" != "true" ]]; then + touch "${WORKTREE_DIR}/.nojekyll" +fi + +# Build rsync --exclude flags from EXCLUDE_ASSETS. +# Supports both newline-separated and comma-separated values. +RSYNC_EXCLUDES=() +while IFS= read -r item; do + # Trim leading and trailing whitespace + item="${item#"${item%%[![:space:]]*}"}" + item="${item%"${item##*[![:space:]]}"}" + [[ -n "${item}" ]] && RSYNC_EXCLUDES+=("--exclude=${item}") +done < <(printf '%s\n' "${EXCLUDE_ASSETS}" | tr ',' '\n') + +# Sync source into destination +rsync -a ${RSYNC_EXCLUDES[@]+"${RSYNC_EXCLUDES[@]}"} "${PUBLISH_DIR%/}/" "${DEST}/" + +cd "${WORKTREE_DIR}" +git add -A +if git diff --cached --quiet; then + echo "Nothing to deploy — ${PUBLISH_BRANCH}${DESTINATION_DIR:+/${DESTINATION_DIR}} is already up to date" +else + git commit -m "${COMMIT_MESSAGE}" + git push origin "${PUBLISH_BRANCH}" + echo "Deployed to ${PUBLISH_BRANCH}${DESTINATION_DIR:+/${DESTINATION_DIR}}" +fi + +# Unregister and remove the worktree to avoid conflicts when the action is +# called multiple times within the same job (e.g. a preview workflow that +# deploys the site and then uploads screenshots). +cd - +git worktree remove --force "${WORKTREE_DIR}" 2>/dev/null || rm -rf "${WORKTREE_DIR}" diff --git a/.github/actions/paths-filter/action.yml b/.github/actions/paths-filter/action.yml new file mode 100644 index 0000000..076ed0b --- /dev/null +++ b/.github/actions/paths-filter/action.yml @@ -0,0 +1,121 @@ +name: 'Paths Filter' +description: > + Determine which filter groups have matching changed files by comparing against + a base commit. Drop-in composite-action replacement for dorny/paths-filter, + maintaining the same core inputs and per-filter-group boolean outputs. + +# Usage example: +# +# - uses: actions/checkout@... +# with: +# fetch-depth: 0 +# +# - uses: ./.github/actions/paths-filter +# id: filter +# with: +# filters: | +# code: +# - '**/*.js' +# - '**/*.ts' +# - 'tests/**' +# +# - if: steps.filter.outputs.code == 'true' +# run: npm test + +inputs: + token: + description: > + GitHub token. Accepted for interface compatibility with dorny/paths-filter + but not used internally; authentication is handled by actions/checkout. + required: false + default: ${{ github.token }} + + ref: + description: > + Git reference to use as HEAD when computing the diff. Accepted for + interface compatibility; the action always diffs INPUT_SHA (github.sha) + against the computed or explicit base. + required: false + default: '' + + base: + description: > + Explicit base commit SHA to compare against. When provided, skips + auto-detection from the event payload. Pass this when the action is + called from inside a workflow_call reusable workflow where + github.event_name is "workflow_call" and auto-detection would be wrong. + required: false + default: '' + + filters: + description: > + YAML string defining one or more named filter groups, each containing a + list of glob patterns. Format mirrors dorny/paths-filter: + + code: + - '**/*.js' + - '**/*.ts' + - 'tests/**' + required: true + + list-files: + description: > + Format for the per-filter _files output. + One of: none | json | csv | shell | escape. + Defaults to none (no file list output). + required: false + default: 'none' + + initial-fetch-depth: + description: > + Accepted for interface compatibility with dorny/paths-filter. + This action assumes the repo was already checked out with sufficient + depth (fetch-depth: 0) by the calling workflow. + required: false + default: '10' + + event_name: + description: > + The real GitHub event name from the calling workflow. + Required when this action is used inside a workflow_call reusable + workflow because github.event_name is always "workflow_call" there, + not the original triggering event. + When empty, the action reads github.event_name directly, which is + correct for workflows triggered by push/pull_request/merge_group. + required: false + default: '' + +outputs: + code: + description: > + "true" if the "code" filter group has any matched files; "false" otherwise. + This output matches the primary filter used in this repository. + value: ${{ steps.run.outputs.code }} + + changes: + description: > + JSON object mapping every filter group name to true/false. + Use this to read results for filter groups other than "code". + value: ${{ steps.run.outputs.changes }} + +runs: + using: composite + steps: + - name: Run paths filter + id: run + # All GitHub context values are assigned to env vars here in the env: block. + # ${{ }} expressions are evaluated by the GitHub Actions runner before the + # shell starts, so they are never interpreted as shell code. This is the + # injection-safe pattern; contrast with interpolating ${{ }} directly + # inside a run: script where attacker-controlled values could escape quotes. + env: + INPUT_BASE: ${{ inputs.base }} + INPUT_EVENT_NAME: ${{ inputs.event_name || github.event_name }} + GH_PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + GH_PUSH_BEFORE: ${{ github.event.before }} + GH_MERGE_BASE_SHA: ${{ github.event.merge_group.base_sha }} + INPUT_SHA: ${{ github.sha }} + INPUT_FILTERS: ${{ inputs.filters }} + INPUT_LIST_FILES: ${{ inputs.list-files }} + run: node "$GITHUB_ACTION_PATH/filter.js" + shell: bash diff --git a/.github/actions/paths-filter/filter.js b/.github/actions/paths-filter/filter.js new file mode 100644 index 0000000..42b4d68 --- /dev/null +++ b/.github/actions/paths-filter/filter.js @@ -0,0 +1,306 @@ +#!/usr/bin/env node +// filter.js — paths-filter composite action worker. +// +// Parses a dorny/paths-filter-compatible YAML filters string, computes or +// receives a git base SHA, diffs against HEAD, and writes per-filter boolean +// outputs to $GITHUB_OUTPUT. +// +// All GitHub context is read from environment variables set via the env: block +// in action.yml — never interpolated directly from ${{ }} expressions — +// so the script is immune to GitHub Actions expression injection. +// +// Environment variables: +// INPUT_BASE — explicit base SHA (optional; overrides auto-detection) +// INPUT_EVENT_NAME — GitHub event name for base auto-detection +// GH_PR_BASE_SHA — github.event.pull_request.base.sha +// GH_PUSH_BEFORE — github.event.before +// GH_MERGE_BASE_SHA — github.event.merge_group.base_sha +// INPUT_SHA — HEAD sha (default: HEAD) +// INPUT_FILTERS — YAML filter definitions (required) +// INPUT_LIST_FILES — file list format: none | json | csv | shell (default: none) +// GITHUB_OUTPUT — path to the runner output file (set by runner) + +'use strict'; + +const { spawnSync } = require('child_process'); +const fs = require('fs'); + +// --------------------------------------------------------------------------- +// Filter YAML parser +// Supports the dorny/paths-filter format: +// filterName: +// - 'glob/pattern/**' +// anotherFilter: +// - '**/*.ts' +// --------------------------------------------------------------------------- + +/** + * Parse a dorny/paths-filter-compatible YAML filters string. + * + * @param {string} yaml + * @returns {Record} + */ +function parseFilters(yaml) { + const result = {}; + let current = null; + for (const rawLine of (yaml || '').split('\n')) { + const line = rawLine.replace(/\r$/, ''); + // Filter name: starts at column 0, no leading whitespace, ends with colon + const nameMatch = line.match(/^([\w][\w-]*):\s*$/); + // List item: leading whitespace + '- ' + value + const itemMatch = line.match(/^\s+-\s+(.+?)\s*$/); + if (nameMatch) { + current = nameMatch[1]; + result[current] = []; + } else if (itemMatch && current !== null) { + // Strip surrounding single or double quotes from the pattern value + const pattern = itemMatch[1] + .replace(/^'(.*)'$/, '$1') + .replace(/^"(.*)"$/, '$1'); + result[current].push(pattern); + } + } + return result; +} + +// --------------------------------------------------------------------------- +// Glob-to-regex converter +// --------------------------------------------------------------------------- + +/** + * Convert a glob pattern to a regular expression string (without anchors). + * Handles: + * ** — matches any path including slashes + * * — matches any characters within a single path segment + * ? — matches a single non-slash character + * rest — literals, with regex-special chars escaped + * + * @param {string} glob + * @returns {string} + */ +function globToRegex(glob) { + const regexSpecial = new Set(['.', '+', '^', '$', '{', '}', '(', ')', '|', '[', ']', '\\']); + let re = ''; + let i = 0; + while (i < glob.length) { + const ch = glob[i]; + if (ch === '*' && i + 1 < glob.length && glob[i + 1] === '*') { + re += '.*'; + i += 2; + // Skip the optional trailing slash after ** (e.g. "tests/**/") + if (i < glob.length && glob[i] === '/') i++; + } else if (ch === '*') { + re += '[^/]*'; + i++; + } else if (ch === '?') { + re += '[^/]'; + i++; + } else if (regexSpecial.has(ch)) { + re += '\\' + ch; + i++; + } else { + re += ch; + i++; + } + } + return re; +} + +// --------------------------------------------------------------------------- +// Base SHA computation +// Mirrors the same event-type logic as detect-changes.sh. +// --------------------------------------------------------------------------- + +/** + * Compute the git base SHA from event context. + * + * @param {string} eventName + * @param {string} prBaseSha + * @param {string} pushBefore + * @param {string} mergeBaseSha + * @returns {string} + */ +function computeBase(eventName, prBaseSha, pushBefore, mergeBaseSha) { + switch (eventName) { + case 'pull_request': + case 'pull_request_target': + return prBaseSha || ''; + case 'push': + return pushBefore || ''; + default: + return mergeBaseSha || ''; + } +} + +// --------------------------------------------------------------------------- +// File matching +// --------------------------------------------------------------------------- + +/** + * Return all files in changedFiles that match at least one glob pattern. + * + * @param {string[]} changedFiles + * @param {string[]} patterns + * @returns {string[]} + */ +function matchFiles(changedFiles, patterns) { + const regexes = patterns.map(p => new RegExp('^' + globToRegex(p) + '$')); + return changedFiles.filter(f => regexes.some(r => r.test(f))); +} + +// --------------------------------------------------------------------------- +// File list formatter +// --------------------------------------------------------------------------- + +/** + * Format a list of matched files for the _files output. + * + * @param {string[]} files + * @param {'none'|'json'|'csv'|'shell'|'escape'} format + * @returns {string} + */ +function formatFileList(files, format) { + switch (format) { + case 'json': + return JSON.stringify(files); + case 'csv': + return files.join(','); + case 'shell': + case 'escape': + // Single-quoted, shell-safe (each ' within a path is escaped as '\'') + return files.map(f => `'${f.replace(/'/g, "'\\''")}'`).join(' '); + default: + return ''; + } +} + +// --------------------------------------------------------------------------- +// Filter runner (pure — no I/O) +// --------------------------------------------------------------------------- + +/** + * Apply all filters to the changed file list and build GITHUB_OUTPUT lines. + * + * @param {string[]} changedFiles + * @param {Record} filtersMap + * @param {string} listFiles — output format for file lists + * @returns {{ lines: string[], changes: Record }} + */ +function runFilter(changedFiles, filtersMap, listFiles) { + const lines = []; + const changes = {}; + const lf = (listFiles || 'none').toLowerCase(); + + for (const [name, patterns] of Object.entries(filtersMap)) { + const matched = matchFiles(changedFiles, patterns); + const hasMatch = matched.length > 0; + changes[name] = hasMatch; + lines.push(`${name}=${hasMatch}`); + if (lf !== 'none' && hasMatch) { + lines.push(`${name}_files=${formatFileList(matched, lf)}`); + } + } + + lines.push(`changes=${JSON.stringify(changes)}`); + return { lines, changes }; +} + +// --------------------------------------------------------------------------- +// Git I/O +// --------------------------------------------------------------------------- + +/** + * Get the list of changed files between base and sha using git. + * Falls back to all tracked files when base is empty or all-zeros. + * + * @param {string} base — base commit SHA (or empty) + * @param {string} sha — head commit SHA + * @returns {{ files: string[], error: string|null }} + */ +function getChangedFiles(base, sha) { + const isEmpty = !base || base === '0000000000000000000000000000000000000000'; + if (isEmpty) { + const r = spawnSync('git', ['ls-files'], { encoding: 'utf8' }); + if (r.status !== 0) { + return { files: [], error: `git ls-files failed: ${r.stderr || '(no stderr)'}` }; + } + return { files: r.stdout.trim().split('\n').filter(Boolean), error: null }; + } + const r = spawnSync('git', ['diff', '--name-only', base, sha], { encoding: 'utf8' }); + if (r.status !== 0) { + return { + files: [], + error: `git diff failed for base '${base}': ${r.stderr || '(no stderr)'}`, + }; + } + return { files: r.stdout.trim().split('\n').filter(Boolean), error: null }; +} + +// --------------------------------------------------------------------------- +// Main entry point +// --------------------------------------------------------------------------- + +/** + * Read configuration from environment variables, run the filter, and write + * results to $GITHUB_OUTPUT. + * + * On any git failure the script falls back to treating all filter groups as + * changed (code=true equivalent) so CI is never silently skipped. + */ +function main() { + const explicitBase = process.env.INPUT_BASE || ''; + const eventName = process.env.INPUT_EVENT_NAME || ''; + const prBaseSha = process.env.GH_PR_BASE_SHA || ''; + const pushBefore = process.env.GH_PUSH_BEFORE || ''; + const mergeBaseSha = process.env.GH_MERGE_BASE_SHA || ''; + const sha = process.env.INPUT_SHA || 'HEAD'; + const filtersYaml = process.env.INPUT_FILTERS || ''; + const listFiles = process.env.INPUT_LIST_FILES || 'none'; + const outputFile = process.env.GITHUB_OUTPUT || ''; + + const base = explicitBase || computeBase(eventName, prBaseSha, pushBefore, mergeBaseSha); + const { files, error } = getChangedFiles(base, sha); + + const filtersMap = parseFilters(filtersYaml); + let outputLines; + + if (error) { + // Safe fallback: treat all filters as matched so CI is never silently skipped + process.stderr.write(`paths-filter: ${error} — treating all filters as changed\n`); + const safeChanges = Object.fromEntries(Object.keys(filtersMap).map(n => [n, true])); + outputLines = [ + ...Object.keys(filtersMap).map(n => `${n}=true`), + `changes=${JSON.stringify(safeChanges)}`, + ]; + } else { + outputLines = runFilter(files, filtersMap, listFiles).lines; + } + + const outputStr = outputLines.join('\n') + '\n'; + + if (outputFile) { + try { + fs.appendFileSync(outputFile, outputStr); + } catch (e) { + process.stderr.write(`paths-filter: failed to write to GITHUB_OUTPUT: ${e.message}\n`); + } + } + + process.stdout.write(outputStr); +} + +// Run main when executed directly; export everything for testing +if (require.main === module) { + main(); +} + +module.exports = { + parseFilters, + globToRegex, + computeBase, + matchFiles, + formatFileList, + runFilter, + getChangedFiles, + main, +}; diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 40fc3ab..d658fe1 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -27,51 +27,19 @@ jobs: run: npm run build - name: Deploy to GitHub Pages (gh-pages branch) - run: | - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - - # Fetch the existing gh-pages branch (silently skip if it doesn't exist yet) - git fetch origin gh-pages:refs/remotes/origin/gh-pages 2>/dev/null || true - - # Check out gh-pages into a worktree so we can update it in-place. - # This preserves any existing content (e.g. previews/) — equivalent to keep_files: true. - if git rev-parse --verify refs/remotes/origin/gh-pages >/dev/null 2>&1; then - git branch --force gh-pages refs/remotes/origin/gh-pages - git worktree add /tmp/gh-pages gh-pages - else - # First-ever deploy: create an orphan gh-pages branch - git worktree add --orphan -b gh-pages /tmp/gh-pages - fi - - # Ensure Jekyll processing is disabled - touch /tmp/gh-pages/.nojekyll - - # Sync the built site into the worktree root. - # Excluded paths keep non-site files out of the deployment. - # Keep this list in sync with the exclude list in preview.yml - # (screenshots is an additional preview-only exclusion). - # .gitignore is excluded so it never lands in gh-pages; if it did, - # `git add -A` would silently skip script.js, styles.css and the - # generated data files (all listed in .gitignore). - rsync -a \ - --exclude='.git' \ - --exclude='.github' \ - --exclude='.gitignore' \ - --exclude='node_modules' \ - --exclude='tests' \ - --exclude='scripts' \ - --exclude='package-lock.json' \ - --exclude='package.json' \ - --exclude='milestones.yaml' \ - --exclude='project-stats.yaml' \ - ./ /tmp/gh-pages/ - - cd /tmp/gh-pages - git add -A - if git diff --cached --quiet; then - echo "Nothing to deploy — gh-pages is already up to date" - else - git commit -m "chore: deploy $(git rev-parse --short HEAD) to gh-pages" - git push origin gh-pages - fi + uses: ./.github/actions/gh-pages-deploy + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + publish_dir: ./ + commit_message: "chore: deploy ${{ github.sha }} to gh-pages" + exclude_assets: | + .git + .github + .gitignore + node_modules + tests + scripts + package-lock.json + package.json + milestones.yaml + project-stats.yaml diff --git a/.github/workflows/detect-changes.yml b/.github/workflows/detect-changes.yml index a2c6158..ca7f8c1 100644 --- a/.github/workflows/detect-changes.yml +++ b/.github/workflows/detect-changes.yml @@ -19,8 +19,8 @@ name: Detect Relevant Changes # Why event_name is a required input: # Inside a reusable workflow triggered via workflow_call, github.event_name is # always "workflow_call", not the original triggering event (push/pull_request/…). -# The caller must pass the real event name so the script can resolve the correct -# base SHA from the inherited github.event payload. +# The caller must pass the real event name so the paths-filter action can resolve +# the correct base SHA from the inherited github.event payload. on: workflow_call: @@ -61,24 +61,31 @@ jobs: # Full history is required so the base commit is reachable for git diff. fetch-depth: 0 - - name: Detect relevant file changes + - name: Filter changed paths + if: ${{ !inputs.always_run }} + id: filter + uses: ./.github/actions/paths-filter + with: + event_name: ${{ inputs.event_name }} + filters: | + code: + - '**/*.js' + - '**/*.ts' + - '**/*.css' + - 'tests/**' + - 'package*.json' + + - name: Set result id: result - # GitHub context values are set as env vars here in the env: block. - # ${{ }} expressions in env: are evaluated by the GitHub Actions runner - # before the shell starts, so they are never interpreted as shell code. - # This is the correct pattern for passing context to shell scripts; by - # contrast, interpolating ${{ }} directly inside run: text would allow - # a malicious value to break out of quotes and execute arbitrary commands. + # Use env: to pass outputs safely — avoids any interpolation concern + # on the always_run boolean and the filter output string. env: ALWAYS_RUN: ${{ inputs.always_run }} - GH_EVENT_NAME: ${{ inputs.event_name }} - GH_PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} - GH_PUSH_BEFORE: ${{ github.event.before }} - GH_MERGE_BASE_SHA: ${{ github.event.merge_group.base_sha }} - GH_SHA: ${{ github.sha }} + FILTER_CODE: ${{ steps.filter.outputs.code }} run: | if [[ "${ALWAYS_RUN}" == "true" ]]; then echo "code=true" >> "$GITHUB_OUTPUT" else - bash .github/scripts/detect-changes.sh + echo "code=${FILTER_CODE}" >> "$GITHUB_OUTPUT" fi + diff --git a/.github/workflows/preview-cleanup.yml b/.github/workflows/preview-cleanup.yml index 14638d7..2eec43b 100644 --- a/.github/workflows/preview-cleanup.yml +++ b/.github/workflows/preview-cleanup.yml @@ -17,13 +17,15 @@ jobs: ref: gh-pages - name: Remove PR preview directory + env: + PR_NUMBER: ${{ github.event.number }} run: | - PR_DIR="previews/pr-${{ github.event.number }}" + PR_DIR="previews/pr-${PR_NUMBER}" git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" if [ -d "$PR_DIR" ]; then git rm -rf "$PR_DIR" - git commit -m "chore: remove preview for PR #${{ github.event.number }}" + git commit -m "chore: remove preview for PR #${PR_NUMBER}" git push echo "Removed $PR_DIR" else diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index f547c61..b86e093 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -30,54 +30,24 @@ jobs: run: npm run screenshots - name: Deploy PR preview to gh-pages branch - run: | - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - - # Fetch the existing gh-pages branch (silently skip if it doesn't exist yet) - git fetch origin gh-pages:refs/remotes/origin/gh-pages 2>/dev/null || true - - # Check out gh-pages into a worktree so we can update it in-place. - # This preserves production files and other previews — equivalent to keep_files: true. - if git rev-parse --verify refs/remotes/origin/gh-pages >/dev/null 2>&1; then - git branch --force gh-pages refs/remotes/origin/gh-pages - git worktree add /tmp/gh-pages gh-pages - else - git worktree add --orphan -b gh-pages /tmp/gh-pages - fi - - DEST="/tmp/gh-pages/previews/pr-${{ github.event.number }}" - mkdir -p "$DEST" - - # Sync built files into the preview subdirectory. - # Keep the exclude list in sync with deploy.yml (screenshots is an additional - # preview-only exclusion: they are uploaded via the Content API in the next step, - # so rsync must not overwrite them before that step runs). - # .gitignore is excluded so it never lands in gh-pages; if it did, - # `git add -A` would silently skip script.js, styles.css and the - # generated data files (all listed in .gitignore). - rsync -a \ - --exclude='.git' \ - --exclude='.github' \ - --exclude='.gitignore' \ - --exclude='node_modules' \ - --exclude='tests' \ - --exclude='scripts' \ - --exclude='package-lock.json' \ - --exclude='package.json' \ - --exclude='milestones.yaml' \ - --exclude='project-stats.yaml' \ - --exclude='screenshots' \ - ./ "$DEST/" - - cd /tmp/gh-pages - git add -A - if git diff --cached --quiet; then - echo "Nothing to deploy — preview is already up to date" - else - git commit -m "chore: deploy preview for PR #${{ github.event.number }}" - git push origin gh-pages - fi + uses: ./.github/actions/gh-pages-deploy + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + publish_dir: ./ + destination_dir: previews/pr-${{ github.event.number }} + commit_message: "chore: deploy preview for PR #${{ github.event.number }}" + exclude_assets: | + .git + .github + .gitignore + node_modules + tests + scripts + package-lock.json + package.json + milestones.yaml + project-stats.yaml + screenshots - name: Upload screenshots to GitHub id: upload-screenshots diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index d9b290c..03e2e5a 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -102,6 +102,7 @@ Every PR description (written by a human or agent) must follow this structure: | A2 | Enable TypeScript type-checking via `checkJs: true` in `tsconfig.json` with JSDoc annotations. This catches type errors in plain `.js` files without requiring a full TS migration. | #54 | | A3 | `death-clock-core.js` must never reference the DOM (`document`, `window`, `getElementById`, etc.). All DOM wiring belongs in `src/js/`. This boundary keeps the core unit-testable. | AGENTS.md | | A4 | The CommonJS + browser dual-export pattern (`module.exports` for Jest, `window.DeathClockCore` for the browser) must be maintained. Do not convert to ES modules without updating all consumers. | AGENTS.md | +| A5 | Composite action outputs must be declared statically in `action.yml`. For dynamic per-filter outputs, expose a `changes` JSON blob output so callers can read any filter name. `git worktree remove --force` must be called at the end of any script that opens a worktree — otherwise a second call in the same job fails with "already checked out". | #— | --- @@ -145,7 +146,16 @@ Entries are grouped by release. Add new entries at the top of the appropriate re ### v1.7.x -#### PR #— refactor: extract reusable detect-changes workflow + fix injection + shell tests +#### PR #— feat: composite actions for gh-pages-deploy and paths-filter with full test coverage + +- **Problem:** The `peaceiris/actions-gh-pages` and `dorny/paths-filter` replacements were implemented as inline shell scripts without a clean reusable-action interface, and `filter.js` was not tracked by codecov. +- **Approach:** Created two composite actions (`.github/actions/paths-filter/` and `.github/actions/gh-pages-deploy/`) with inputs matching the original third-party actions. `detect-changes.yml` now uses `paths-filter` action. `deploy.yml` and `preview.yml` now use `gh-pages-deploy` action. `filter.js` exports all pure functions (injection-safe via env vars) and is added to `collectCoverageFrom` with per-file thresholds. 67 Jest tests cover `filter.js` at 99%/90% stmt/branch. 19 bash tests cover `deploy.sh`. Fixed injection in `preview-cleanup.yml` (moved `${{ github.event.number }}` to `env:`). +- **Learning:** Composite actions must declare outputs statically; use `changes` (JSON) as a catch-all output for dynamic filter names. `git worktree remove --force` must be called at the end of any deploy script that creates a worktree, otherwise subsequent calls in the same job fail with "already checked out" errors. rsync's quick-check uses mtime+size — tests that write the same file twice in < 1 second must use different-length content to force detection. (→ A1, S5) +- **Key files:** `.github/actions/paths-filter/action.yml`, `.github/actions/paths-filter/filter.js`, `.github/actions/gh-pages-deploy/action.yml`, `.github/actions/gh-pages-deploy/deploy.sh`, `tests/paths-filter.test.js`, `tests/gh-pages-deploy.test.sh`, `package.json` + +--- + + - **Problem:** The `git diff` path-filter logic was duplicated verbatim in both `unit-tests.yml` and `e2e-tests.yml`, and each copy interpolated GitHub context values (`${{ github.event_name }}`, `${{ github.sha }}`, etc.) directly into `run:` scripts — an expression-injection anti-pattern. - **Approach:** Extracted the logic into `.github/scripts/detect-changes.sh` (reads all GitHub context from env vars, never from `${{ }}` interpolation). Wrapped it in a reusable `workflow_call` workflow (`.github/workflows/detect-changes.yml`) with `event_name` and `always_run` inputs and a `code` output. Both callers now use `uses: ./.github/workflows/detect-changes.yml`. Added 19 bash unit tests in `tests/detect-changes.test.sh` and an `npm run test:shell` script. The `test` job in `unit-tests.yml` runs shell tests in CI. The git failure fallback now outputs `code=true` (safe: run CI) rather than `code=false` (unsafe: silently skip CI). diff --git a/package.json b/package.json index ec5e167..73cfa06 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "test": "jest --coverage", "pretest:ci": "npm run build:milestones && npm run build:js", "test:ci": "jest --ci --coverage", - "test:shell": "bash tests/detect-changes.test.sh", + "test:shell": "bash tests/detect-changes.test.sh && bash tests/gh-pages-deploy.test.sh", "test:e2e": "playwright test", "test:e2e:ui": "playwright test --ui", "build:milestones": "node scripts/build-milestones.js", @@ -38,7 +38,8 @@ ], "collectCoverageFrom": [ "death-clock-core.js", - "script.js" + "script.js", + ".github/actions/paths-filter/filter.js" ], "coverageThreshold": { "global": { @@ -57,6 +58,12 @@ "functions": 60, "branches": 50, "lines": 90 + }, + "./.github/actions/paths-filter/filter.js": { + "statements": 85, + "functions": 85, + "branches": 75, + "lines": 85 } } } diff --git a/tests/gh-pages-deploy.test.sh b/tests/gh-pages-deploy.test.sh new file mode 100644 index 0000000..52ae1ca --- /dev/null +++ b/tests/gh-pages-deploy.test.sh @@ -0,0 +1,369 @@ +#!/usr/bin/env bash +# tests/gh-pages-deploy.test.sh +# +# Unit tests for .github/actions/gh-pages-deploy/deploy.sh +# +# Tests create isolated temporary git repositories in /tmp so they never +# depend on the real repo's history and leave no side effects. +# +# Run with: bash tests/gh-pages-deploy.test.sh + +set -uo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SCRIPT="${REPO_ROOT}/.github/actions/gh-pages-deploy/deploy.sh" + +PASS=0 +FAIL=0 + +# Temporary directories created during a test — cleaned up by EXIT trap +TMPREMOTE="" +TMPWORK="" +TMPSRC="" + +cleanup() { + rm -rf "${TMPREMOTE:-}" "${TMPWORK:-}" "${TMPSRC:-}" +} +trap cleanup EXIT + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +assert() { + local desc="$1" expected="$2" actual="$3" + if [[ "${actual}" == "${expected}" ]]; then + echo " ✅ ${desc}" + PASS=$((PASS + 1)) + else + echo " ❌ ${desc}" + echo " expected : '${expected}'" + echo " got : '${actual}'" + FAIL=$((FAIL + 1)) + fi +} + +assert_contains() { + local desc="$1" needle="$2" haystack="$3" + if [[ "${haystack}" == *"${needle}"* ]]; then + echo " ✅ ${desc}" + PASS=$((PASS + 1)) + else + echo " ❌ ${desc}" + echo " expected to contain : '${needle}'" + echo " in : '${haystack}'" + FAIL=$((FAIL + 1)) + fi +} + +assert_file_exists() { + local desc="$1" path="$2" + if [[ -f "${path}" ]]; then + echo " ✅ ${desc}" + PASS=$((PASS + 1)) + else + echo " ❌ ${desc}: file not found: ${path}" + FAIL=$((FAIL + 1)) + fi +} + +assert_file_missing() { + local desc="$1" path="$2" + if [[ ! -f "${path}" ]]; then + echo " ✅ ${desc}" + PASS=$((PASS + 1)) + else + echo " ❌ ${desc}: file unexpectedly exists: ${path}" + FAIL=$((FAIL + 1)) + fi +} + +# setup_repos — creates TMPREMOTE (bare), TMPWORK (working copy), TMPSRC (source dir). +# Adds a commit on main so the remote has a HEAD. +setup_repos() { + TMPREMOTE=$(mktemp -d) + TMPWORK=$(mktemp -d) + TMPSRC=$(mktemp -d) + + git -C "${TMPREMOTE}" init --bare -q + + git -C "${TMPWORK}" init -q + git -C "${TMPWORK}" remote add origin "${TMPREMOTE}" + git -C "${TMPWORK}" config user.email "test@example.com" + git -C "${TMPWORK}" config user.name "Test Bot" + + # Initial commit on main + echo "source" > "${TMPWORK}/README.md" + git -C "${TMPWORK}" add -A + git -C "${TMPWORK}" commit -q -m "init" + git -C "${TMPWORK}" push -q origin HEAD:main +} + +# run_deploy [KEY=VAL ...] — runs deploy.sh inside TMPWORK with given env vars +run_deploy() { + ( + cd "${TMPWORK}" + env \ + INPUT_GITHUB_TOKEN="" \ + INPUT_PUBLISH_BRANCH="gh-pages" \ + INPUT_PUBLISH_DIR="${TMPSRC}" \ + INPUT_DESTINATION_DIR="" \ + INPUT_KEEP_FILES="false" \ + INPUT_EXCLUDE_ASSETS=".github" \ + INPUT_USER_NAME="test-bot" \ + INPUT_USER_EMAIL="bot@test.com" \ + INPUT_COMMIT_MESSAGE="chore: deploy" \ + INPUT_DISABLE_NOJEKYLL="false" \ + "$@" \ + bash "${SCRIPT}" + ) 2>/dev/null +} + +# file_in_branch — reads a file from the given branch via git show +file_in_branch() { + local branch="$1" path="$2" + git -C "${TMPWORK}" fetch -q origin "${branch}:${branch}" 2>/dev/null || true + git -C "${TMPWORK}" show "${branch}:${path}" 2>/dev/null || echo "__MISSING__" +} + +# list_branch_root — lists files at the root of the given branch +list_branch_root() { + local branch="$1" + git -C "${TMPWORK}" fetch -q origin "${branch}:${branch}" 2>/dev/null || true + git -C "${TMPWORK}" ls-tree --name-only "${branch}" 2>/dev/null || true +} + +# --------------------------------------------------------------------------- +# Test: creates gh-pages branch and deploys files +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — new branch creation ===" +echo "" + +setup_repos +echo "built index" > "${TMPSRC}/index.html" +echo "app code" > "${TMPSRC}/app.js" + +run_deploy + +# Check that index.html landed in gh-pages +actual=$(file_in_branch "gh-pages" "index.html") +assert "index.html content is correct" "built index" "${actual}" + +actual=$(file_in_branch "gh-pages" "app.js") +assert "app.js content is correct" "app code" "${actual}" + +# --------------------------------------------------------------------------- +# Test: .nojekyll is created by default +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — .nojekyll ===" +echo "" + +actual=$(file_in_branch "gh-pages" ".nojekyll") +assert "nojekyll: .nojekyll exists by default" "" "${actual}" + +cleanup; setup_repos +echo "page" > "${TMPSRC}/index.html" +run_deploy INPUT_DISABLE_NOJEKYLL="true" + +actual=$(list_branch_root "gh-pages") +if echo "${actual}" | grep -q "\.nojekyll"; then + echo " ❌ nojekyll: .nojekyll should not exist when disable_nojekyll=true" + FAIL=$((FAIL + 1)) +else + echo " ✅ nojekyll: .nojekyll absent when disable_nojekyll=true" + PASS=$((PASS + 1)) +fi + +# --------------------------------------------------------------------------- +# Test: destination_dir deploys to subdirectory +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — destination_dir ===" +echo "" + +cleanup; setup_repos + +# First deploy to root so gh-pages branch exists +echo "root page" > "${TMPSRC}/root.html" +run_deploy + +# Now deploy to a subdirectory +echo "preview page" > "${TMPSRC}/index.html" +run_deploy INPUT_DESTINATION_DIR="previews/pr-42" + +actual=$(file_in_branch "gh-pages" "previews/pr-42/index.html") +assert "destination_dir: file in subdirectory" "preview page" "${actual}" + +# Root file should still be present (worktree preserves other content) +actual=$(file_in_branch "gh-pages" "root.html") +assert "destination_dir: root files preserved" "root page" "${actual}" + +# --------------------------------------------------------------------------- +# Test: keep_files=false with destination_dir clears stale files +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — keep_files=false clears destination_dir ===" +echo "" + +# The previous test already deployed index.html to previews/pr-42. +# Now deploy ONLY new.html (no index.html) with keep_files=false. +cleanup; setup_repos + +# Initial deploy to create gh-pages +echo "old file" > "${TMPSRC}/stale.html" +echo "keep file" > "${TMPSRC}/keep.html" +run_deploy INPUT_DESTINATION_DIR="previews/pr-1" + +# Second deploy with different source (stale.html gone, new.html added) +rm "${TMPSRC}/stale.html" +echo "new content" > "${TMPSRC}/new.html" +run_deploy INPUT_DESTINATION_DIR="previews/pr-1" INPUT_KEEP_FILES="false" + +actual=$(file_in_branch "gh-pages" "previews/pr-1/new.html") +assert "keep_files=false: new file deployed" "new content" "${actual}" + +actual=$(file_in_branch "gh-pages" "previews/pr-1/stale.html") +assert "keep_files=false: stale file removed" "__MISSING__" "${actual}" + +# --------------------------------------------------------------------------- +# Test: keep_files=true with destination_dir preserves stale files +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — keep_files=true preserves destination_dir ===" +echo "" + +cleanup; setup_repos + +echo "existing file" > "${TMPSRC}/existing.html" +run_deploy INPUT_DESTINATION_DIR="previews/pr-2" + +# Deploy new file without removing existing.html from source +echo "second file" > "${TMPSRC}/second.html" +run_deploy INPUT_DESTINATION_DIR="previews/pr-2" INPUT_KEEP_FILES="true" + +actual=$(file_in_branch "gh-pages" "previews/pr-2/existing.html") +assert "keep_files=true: existing file preserved" "existing file" "${actual}" + +actual=$(file_in_branch "gh-pages" "previews/pr-2/second.html") +assert "keep_files=true: new file added" "second file" "${actual}" + +# --------------------------------------------------------------------------- +# Test: exclude_assets are not deployed +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — exclude_assets ===" +echo "" + +cleanup; setup_repos + +echo "site content" > "${TMPSRC}/index.html" +mkdir -p "${TMPSRC}/node_modules" +echo "dep" > "${TMPSRC}/node_modules/dep.js" +echo "secret" > "${TMPSRC}/.env" + +run_deploy INPUT_EXCLUDE_ASSETS="node_modules,.env" + +actual=$(file_in_branch "gh-pages" "index.html") +assert "exclude_assets: index.html is deployed" "site content" "${actual}" + +actual=$(file_in_branch "gh-pages" "node_modules/dep.js") +assert "exclude_assets: node_modules excluded" "__MISSING__" "${actual}" + +actual=$(file_in_branch "gh-pages" ".env") +assert "exclude_assets: .env excluded" "__MISSING__" "${actual}" + +# --------------------------------------------------------------------------- +# Test: newline-separated exclude_assets +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — exclude_assets (newline-separated) ===" +echo "" + +cleanup; setup_repos + +echo "page" > "${TMPSRC}/page.html" +echo "secret" > "${TMPSRC}/secret.txt" + +run_deploy INPUT_EXCLUDE_ASSETS="$(printf 'secret.txt\n.github')" + +actual=$(file_in_branch "gh-pages" "page.html") +assert "newline excludes: page.html deployed" "page" "${actual}" + +actual=$(file_in_branch "gh-pages" "secret.txt") +assert "newline excludes: secret.txt excluded" "__MISSING__" "${actual}" + +# --------------------------------------------------------------------------- +# Test: no-op deploy when nothing changed +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — no-op when nothing changed ===" +echo "" + +cleanup; setup_repos + +echo "stable" > "${TMPSRC}/index.html" +run_deploy + +# Get the commit count before second deploy +commit_count_before=$(git -C "${TMPWORK}" fetch -q origin gh-pages:gh-pages 2>/dev/null; \ + git -C "${TMPWORK}" rev-list --count gh-pages 2>/dev/null || echo "0") + +# Deploy again with identical content +output=$(run_deploy 2>&1 || true) +commit_count_after=$(git -C "${TMPWORK}" fetch -q origin gh-pages:gh-pages 2>/dev/null; \ + git -C "${TMPWORK}" rev-list --count gh-pages 2>/dev/null || echo "0") + +assert "no-op: commit count unchanged" "${commit_count_before}" "${commit_count_after}" +assert_contains "no-op: reports nothing to deploy" "Nothing to deploy" "${output}" + +# --------------------------------------------------------------------------- +# Test: custom commit_message is used +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — custom commit_message ===" +echo "" + +cleanup; setup_repos + +echo "content" > "${TMPSRC}/index.html" +run_deploy INPUT_COMMIT_MESSAGE="feat: custom deploy message" + +git -C "${TMPWORK}" fetch -q origin gh-pages:gh-pages 2>/dev/null +actual_msg=$(git -C "${TMPWORK}" log -1 --format="%s" gh-pages 2>/dev/null || echo "") +assert "commit_message: custom message used" "feat: custom deploy message" "${actual_msg}" + +# --------------------------------------------------------------------------- +# Test: second deploy to existing branch (update path) +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — update existing branch ===" +echo "" + +cleanup; setup_repos + +echo "version one content" > "${TMPSRC}/index.html" +run_deploy + +# Use content with a different size so rsync's quick-check (mtime+size) always +# detects the change, even when both writes happen within the same second. +echo "version two content — updated" > "${TMPSRC}/index.html" +run_deploy + +actual=$(file_in_branch "gh-pages" "index.html") +assert "update: second deploy updates file" "version two content — updated" "${actual}" + +# --------------------------------------------------------------------------- +# Summary +# --------------------------------------------------------------------------- + +echo "" +echo "=======================================" +echo "Results: ${PASS} passed, ${FAIL} failed" +echo "=======================================" +echo "" + +if [[ ${FAIL} -gt 0 ]]; then + exit 1 +fi diff --git a/tests/paths-filter.test.js b/tests/paths-filter.test.js new file mode 100644 index 0000000..0bd2a93 --- /dev/null +++ b/tests/paths-filter.test.js @@ -0,0 +1,576 @@ +// tests/paths-filter.test.js +// +// Unit tests for .github/actions/paths-filter/filter.js +// +// Pure-function tests run without any mocking. +// I/O tests (getChangedFiles, main) mock child_process and fs so they +// exercise all code paths without requiring a real git repository. + +'use strict'; + +// --------------------------------------------------------------------------- +// Mock child_process and fs BEFORE requiring the module under test +// --------------------------------------------------------------------------- +jest.mock('child_process', () => ({ spawnSync: jest.fn() })); +jest.mock('fs', () => ({ appendFileSync: jest.fn() })); + +const { spawnSync } = require('child_process'); +const fs = require('fs'); + +const { + parseFilters, + globToRegex, + computeBase, + matchFiles, + formatFileList, + runFilter, + getChangedFiles, + main, +} = require('../.github/actions/paths-filter/filter'); + +// --------------------------------------------------------------------------- +// Helper: reset env vars before each test +// --------------------------------------------------------------------------- +const ENV_KEYS = [ + 'INPUT_BASE', 'INPUT_EVENT_NAME', 'GH_PR_BASE_SHA', 'GH_PUSH_BEFORE', + 'GH_MERGE_BASE_SHA', 'INPUT_SHA', 'INPUT_FILTERS', 'INPUT_LIST_FILES', + 'GITHUB_OUTPUT', +]; + +beforeEach(() => { + ENV_KEYS.forEach(k => delete process.env[k]); + jest.clearAllMocks(); +}); + +// ============================================================================ +// parseFilters +// ============================================================================ + +describe('parseFilters', () => { + test('returns empty object for empty string', () => { + expect(parseFilters('')).toEqual({}); + }); + + test('returns empty object for null/undefined', () => { + expect(parseFilters(null)).toEqual({}); + expect(parseFilters(undefined)).toEqual({}); + }); + + test('parses a single filter with one pattern', () => { + const yaml = `code:\n - '**/*.js'`; + expect(parseFilters(yaml)).toEqual({ code: ['**/*.js'] }); + }); + + test('parses a single filter with multiple patterns', () => { + const yaml = [ + 'code:', + " - '**/*.js'", + " - '**/*.ts'", + " - '**/*.css'", + " - 'tests/**'", + " - 'package*.json'", + ].join('\n'); + expect(parseFilters(yaml)).toEqual({ + code: ['**/*.js', '**/*.ts', '**/*.css', 'tests/**', 'package*.json'], + }); + }); + + test('parses multiple filter groups', () => { + const yaml = [ + 'frontend:', + " - 'src/**/*.js'", + 'backend:', + " - 'api/**/*.py'", + ].join('\n'); + expect(parseFilters(yaml)).toEqual({ + frontend: ['src/**/*.js'], + backend: ['api/**/*.py'], + }); + }); + + test('strips single quotes from pattern values', () => { + expect(parseFilters("code:\n - '**/*.js'")).toEqual({ code: ['**/*.js'] }); + }); + + test('strips double quotes from pattern values', () => { + expect(parseFilters('code:\n - "**/*.js"')).toEqual({ code: ['**/*.js'] }); + }); + + test('handles patterns without quotes', () => { + expect(parseFilters('code:\n - **/*.js')).toEqual({ code: ['**/*.js'] }); + }); + + test('ignores indented content before first filter name', () => { + const yaml = ' - orphan pattern\ncode:\n - src/**'; + expect(parseFilters(yaml)).toEqual({ code: ['src/**'] }); + }); + + test('handles filter names with hyphens', () => { + const yaml = 'my-filter:\n - src/**'; + expect(parseFilters(yaml)).toEqual({ 'my-filter': ['src/**'] }); + }); + + test('handles Windows-style CRLF line endings', () => { + const yaml = 'code:\r\n - src/**\r\n'; + expect(parseFilters(yaml)).toEqual({ code: ['src/**'] }); + }); + + test('handles trailing whitespace on items', () => { + const yaml = "code:\n - '**/*.js' "; + expect(parseFilters(yaml)).toEqual({ code: ['**/*.js'] }); + }); +}); + +// ============================================================================ +// globToRegex +// ============================================================================ + +describe('globToRegex', () => { + function match(pattern, filePath) { + return new RegExp('^' + globToRegex(pattern) + '$').test(filePath); + } + + // ** patterns + test('**/*.js matches any .js file at any depth', () => { + expect(match('**/*.js', 'app.js')).toBe(true); + expect(match('**/*.js', 'src/app.js')).toBe(true); + expect(match('**/*.js', 'src/lib/util.js')).toBe(true); + expect(match('**/*.js', 'app.ts')).toBe(false); + }); + + test('tests/** matches any file under tests/', () => { + expect(match('tests/**', 'tests/foo.test.js')).toBe(true); + expect(match('tests/**', 'tests/e2e/spec.js')).toBe(true); + expect(match('tests/**', 'src/tests/foo.js')).toBe(false); + }); + + test('**/tests/** matches tests/ anywhere in path', () => { + expect(match('**/tests/**', 'tests/foo.js')).toBe(true); + expect(match('**/tests/**', 'src/tests/foo.js')).toBe(true); + }); + + // * patterns (single segment) + test('*.js matches a top-level .js file only', () => { + expect(match('*.js', 'app.js')).toBe(true); + expect(match('*.js', 'src/app.js')).toBe(false); + }); + + test('package*.json matches package.json and package-lock.json', () => { + expect(match('package*.json', 'package.json')).toBe(true); + expect(match('package*.json', 'package-lock.json')).toBe(true); + expect(match('package*.json', 'my-package.json')).toBe(false); + }); + + // ? patterns + test('? matches exactly one non-slash character', () => { + expect(match('src/?.js', 'src/a.js')).toBe(true); + expect(match('src/?.js', 'src/ab.js')).toBe(false); + expect(match('src/?.js', 'src/a/b.js')).toBe(false); + }); + + // Regex-special characters in literal parts + test('dots in literal patterns are treated as literal dots', () => { + expect(match('README.md', 'README.md')).toBe(true); + expect(match('README.md', 'READMEXmd')).toBe(false); + }); + + test('brackets and parens are escaped', () => { + expect(match('a[b].js', 'a[b].js')).toBe(true); + expect(match('a(b).js', 'a(b).js')).toBe(true); + }); + + test('backslash is escaped', () => { + const regex = globToRegex('a\\b'); + expect(regex).toContain('\\\\'); + }); + + // Edge cases + test('empty pattern produces empty regex (matches empty string only)', () => { + expect(match('', '')).toBe(true); + expect(match('', 'a')).toBe(false); + }); + + test('** alone matches any path', () => { + expect(match('**', 'anything/deep/path.js')).toBe(true); + }); +}); + +// ============================================================================ +// computeBase +// ============================================================================ + +describe('computeBase', () => { + test('pull_request uses prBaseSha', () => { + expect(computeBase('pull_request', 'abc123', 'before456', 'merge789')).toBe('abc123'); + }); + + test('pull_request_target uses prBaseSha', () => { + expect(computeBase('pull_request_target', 'abc123', 'before456', '')).toBe('abc123'); + }); + + test('push uses pushBefore', () => { + expect(computeBase('push', 'prBase', 'before456', 'mergeBase')).toBe('before456'); + }); + + test('merge_group uses mergeBaseSha', () => { + expect(computeBase('merge_group', '', '', 'merge789')).toBe('merge789'); + }); + + test('unknown event falls back to mergeBaseSha', () => { + expect(computeBase('workflow_dispatch', 'pr', 'push', 'fallback')).toBe('fallback'); + }); + + test('returns empty string when sha is not available', () => { + expect(computeBase('push', '', '', '')).toBe(''); + }); + + test('returns empty string for pull_request with empty sha', () => { + expect(computeBase('pull_request', '', 'before', 'merge')).toBe(''); + }); +}); + +// ============================================================================ +// matchFiles +// ============================================================================ + +describe('matchFiles', () => { + const files = [ + 'src/app.js', + 'src/lib.ts', + 'styles/main.css', + 'tests/foo.test.js', + 'package.json', + 'package-lock.json', + 'README.md', + 'config.yaml', + ]; + + test('matches .js files with **/*.js', () => { + const result = matchFiles(files, ['**/*.js']); + expect(result).toContain('src/app.js'); + expect(result).toContain('tests/foo.test.js'); + expect(result).not.toContain('src/lib.ts'); + }); + + test('matches tests/ directory with tests/**', () => { + expect(matchFiles(files, ['tests/**'])).toEqual(['tests/foo.test.js']); + }); + + test('matches package.json files with package*.json', () => { + const result = matchFiles(files, ['package*.json']); + expect(result).toContain('package.json'); + expect(result).toContain('package-lock.json'); + }); + + test('multiple patterns: file matched by any one pattern is included', () => { + const result = matchFiles(files, ['**/*.ts', '**/*.css']); + expect(result).toContain('src/lib.ts'); + expect(result).toContain('styles/main.css'); + expect(result).not.toContain('src/app.js'); + }); + + test('returns empty array when no files match', () => { + expect(matchFiles(files, ['**/*.go'])).toEqual([]); + }); + + test('returns empty array for empty patterns list', () => { + expect(matchFiles(files, [])).toEqual([]); + }); + + test('returns empty array for empty file list', () => { + expect(matchFiles([], ['**/*.js'])).toEqual([]); + }); +}); + +// ============================================================================ +// formatFileList +// ============================================================================ + +describe('formatFileList', () => { + const files = ['src/app.js', 'src/lib.ts']; + + test('none returns empty string', () => { + expect(formatFileList(files, 'none')).toBe(''); + }); + + test('unknown format returns empty string', () => { + expect(formatFileList(files, 'unknown')).toBe(''); + }); + + test('json returns a JSON array string', () => { + expect(formatFileList(files, 'json')).toBe('["src/app.js","src/lib.ts"]'); + }); + + test('csv returns comma-joined list', () => { + expect(formatFileList(files, 'csv')).toBe('src/app.js,src/lib.ts'); + }); + + test('shell returns single-quoted space-separated list', () => { + expect(formatFileList(files, 'shell')).toBe("'src/app.js' 'src/lib.ts'"); + }); + + test('escape is an alias for shell', () => { + expect(formatFileList(files, 'escape')).toBe("'src/app.js' 'src/lib.ts'"); + }); + + test('shell escapes embedded single quotes', () => { + const result = formatFileList(["it's a file.js"], 'shell'); + expect(result).toBe("'it'\\''s a file.js'"); + }); + + test('handles empty file list', () => { + expect(formatFileList([], 'json')).toBe('[]'); + expect(formatFileList([], 'csv')).toBe(''); + expect(formatFileList([], 'shell')).toBe(''); + }); +}); + +// ============================================================================ +// runFilter +// ============================================================================ + +describe('runFilter', () => { + const files = ['src/app.js', 'src/lib.ts', 'README.md']; + const filters = { + code: ['**/*.js', '**/*.ts'], + docs: ['README.md', '**/*.md'], + }; + + test('outputs =true for each matched filter', () => { + const { lines } = runFilter(files, filters, 'none'); + expect(lines).toContain('code=true'); + expect(lines).toContain('docs=true'); + }); + + test('outputs =false when no files match', () => { + const { lines } = runFilter(['README.md'], { code: ['**/*.js'] }, 'none'); + expect(lines).toContain('code=false'); + }); + + test('includes changes JSON as last line', () => { + const { lines, changes } = runFilter(files, filters, 'none'); + expect(lines[lines.length - 1]).toBe(`changes=${JSON.stringify(changes)}`); + expect(changes).toEqual({ code: true, docs: true }); + }); + + test('adds _files when list-files=json and filter matched', () => { + const { lines } = runFilter(['src/app.js'], { code: ['**/*.js'] }, 'json'); + expect(lines).toContain('code_files=["src/app.js"]'); + }); + + test('adds _files when list-files=csv', () => { + const { lines } = runFilter( + ['src/app.js', 'src/lib.ts'], + { code: ['**/*.js', '**/*.ts'] }, + 'csv', + ); + expect(lines).toContain('code_files=src/app.js,src/lib.ts'); + }); + + test('adds _files when list-files=shell', () => { + const { lines } = runFilter(['src/app.js'], { code: ['**/*.js'] }, 'shell'); + expect(lines).toContain("code_files='src/app.js'"); + }); + + test('does not add _files when filter did not match', () => { + const { lines } = runFilter([], { code: ['**/*.js'] }, 'json'); + expect(lines.some(l => l.startsWith('code_files='))).toBe(false); + }); + + test('handles empty changedFiles (no matches)', () => { + const { changes } = runFilter([], filters, 'none'); + expect(changes).toEqual({ code: false, docs: false }); + }); + + test('handles empty filtersMap', () => { + const { lines, changes } = runFilter(files, {}, 'none'); + expect(changes).toEqual({}); + expect(lines).toEqual(['changes={}']); + }); +}); + +// ============================================================================ +// getChangedFiles +// ============================================================================ + +describe('getChangedFiles', () => { + test('uses git ls-files when base is empty', () => { + spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\nb.js\n', stderr: '' }); + const { files, error } = getChangedFiles('', 'HEAD'); + expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files'], { encoding: 'utf8' }); + expect(files).toEqual(['a.js', 'b.js']); + expect(error).toBeNull(); + }); + + test('uses git ls-files when base is all-zeros', () => { + spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\n', stderr: '' }); + getChangedFiles('0000000000000000000000000000000000000000', 'HEAD'); + expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files'], { encoding: 'utf8' }); + }); + + test('uses git diff when base is a real SHA', () => { + spawnSync.mockReturnValue({ status: 0, stdout: 'changed.js\n', stderr: '' }); + const { files, error } = getChangedFiles('abc123', 'def456'); + expect(spawnSync).toHaveBeenCalledWith( + 'git', ['diff', '--name-only', 'abc123', 'def456'], { encoding: 'utf8' }, + ); + expect(files).toEqual(['changed.js']); + expect(error).toBeNull(); + }); + + test('returns error when git ls-files fails', () => { + spawnSync.mockReturnValue({ status: 1, stdout: '', stderr: 'fatal error' }); + const { files, error } = getChangedFiles('', 'HEAD'); + expect(files).toEqual([]); + expect(error).toContain('git ls-files failed'); + }); + + test('returns error when git diff fails (bad base SHA)', () => { + spawnSync.mockReturnValue({ status: 128, stdout: '', stderr: 'unknown revision' }); + const { files, error } = getChangedFiles('badref', 'HEAD'); + expect(files).toEqual([]); + expect(error).toContain("git diff failed for base 'badref'"); + }); + + test('filters out empty lines from output', () => { + spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\n\nb.js\n', stderr: '' }); + const { files } = getChangedFiles('abc', 'def'); + expect(files).toEqual(['a.js', 'b.js']); + }); + + test('returns empty files array when git output is empty', () => { + spawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); + const { files } = getChangedFiles('abc', 'def'); + expect(files).toEqual([]); + }); +}); + +// ============================================================================ +// main +// ============================================================================ + +describe('main', () => { + function setEnv(vars) { + Object.entries(vars).forEach(([k, v]) => { process.env[k] = v; }); + } + + const FILTERS_YAML = 'code:\n - "**/*.js"\n - "**/*.ts"'; + + test('writes code=true when matching files found (pull_request)', () => { + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + INPUT_EVENT_NAME: 'pull_request', + GH_PR_BASE_SHA: 'base-sha', + GH_PUSH_BEFORE: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'head-sha', + GITHUB_OUTPUT: '/tmp/test-output', + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'src/app.js\n', stderr: '' }); + + main(); + + const written = process.stdout.write.mock + ? process.stdout.write.mock.calls.map(c => c[0]).join('') + : ''; + // Verify GITHUB_OUTPUT was written + expect(fs.appendFileSync).toHaveBeenCalledWith( + '/tmp/test-output', + expect.stringContaining('code=true'), + ); + }); + + test('writes code=false when no matching files found', () => { + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + INPUT_EVENT_NAME: 'push', + GH_PUSH_BEFORE: 'before-sha', + GH_PR_BASE_SHA: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'head-sha', + GITHUB_OUTPUT: '/tmp/test-output', + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'README.md\n', stderr: '' }); + + main(); + + expect(fs.appendFileSync).toHaveBeenCalledWith( + '/tmp/test-output', + expect.stringContaining('code=false'), + ); + }); + + test('uses explicit INPUT_BASE when provided (overrides event auto-detect)', () => { + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + INPUT_BASE: 'explicit-base', + INPUT_EVENT_NAME: 'push', + GH_PUSH_BEFORE: 'wrong-before-sha', + GH_PR_BASE_SHA: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'head-sha', + GITHUB_OUTPUT: '/tmp/test-output', + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.ts\n', stderr: '' }); + + main(); + + expect(spawnSync).toHaveBeenCalledWith( + 'git', ['diff', '--name-only', 'explicit-base', 'head-sha'], { encoding: 'utf8' }, + ); + }); + + test('falls back to all-changed on git diff failure', () => { + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + INPUT_EVENT_NAME: 'push', + GH_PUSH_BEFORE: 'bad-sha', + GH_PR_BASE_SHA: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'head-sha', + GITHUB_OUTPUT: '/tmp/test-output', + }); + spawnSync.mockReturnValue({ status: 128, stdout: '', stderr: 'fatal: bad SHA' }); + + main(); + + // On failure, all filters should be forced true + expect(fs.appendFileSync).toHaveBeenCalledWith( + '/tmp/test-output', + expect.stringContaining('code=true'), + ); + }); + + test('skips GITHUB_OUTPUT write when env var is not set', () => { + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + INPUT_EVENT_NAME: 'push', + GH_PUSH_BEFORE: '', + GH_PR_BASE_SHA: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'HEAD', + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + + main(); + + expect(fs.appendFileSync).not.toHaveBeenCalled(); + }); + + test('handles fs.appendFileSync error gracefully', () => { + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + INPUT_EVENT_NAME: 'push', + GH_PUSH_BEFORE: '', + GH_PR_BASE_SHA: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'HEAD', + GITHUB_OUTPUT: '/tmp/test-output', + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + fs.appendFileSync.mockImplementation(() => { throw new Error('disk full'); }); + + // Should not throw + expect(() => main()).not.toThrow(); + }); +}); From 88eaee159897d29be4f3906f7bf4d0b6c4413540 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:52:42 +0000 Subject: [PATCH 04/10] fix: address code review feedback on paths-filter and gh-pages-deploy actions Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/0047cdb7-30cf-42d6-b536-50e08c386248 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- .github/actions/gh-pages-deploy/deploy.sh | 2 +- .github/actions/paths-filter/filter.js | 4 ++-- docs/LEARNINGS.md | 3 ++- tests/paths-filter.test.js | 3 --- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.github/actions/gh-pages-deploy/deploy.sh b/.github/actions/gh-pages-deploy/deploy.sh index dd2653a..d0e3a3f 100644 --- a/.github/actions/gh-pages-deploy/deploy.sh +++ b/.github/actions/gh-pages-deploy/deploy.sh @@ -73,7 +73,7 @@ mkdir -p "${DEST}" # so deleted files don't linger. Root deployments always preserve other content # (e.g. the previews/ directory) regardless of keep_files. if [[ "${KEEP_FILES}" != "true" && -n "${DESTINATION_DIR}" ]]; then - rm -rf "${DEST:?}/"* 2>/dev/null || true + find "${DEST}" -mindepth 1 -delete 2>/dev/null || true fi # Ensure Jekyll processing is disabled (unless explicitly opted out) diff --git a/.github/actions/paths-filter/filter.js b/.github/actions/paths-filter/filter.js index 42b4d68..881a26b 100644 --- a/.github/actions/paths-filter/filter.js +++ b/.github/actions/paths-filter/filter.js @@ -45,8 +45,8 @@ function parseFilters(yaml) { let current = null; for (const rawLine of (yaml || '').split('\n')) { const line = rawLine.replace(/\r$/, ''); - // Filter name: starts at column 0, no leading whitespace, ends with colon - const nameMatch = line.match(/^([\w][\w-]*):\s*$/); + // Filter name: starts at column 0 with a letter, no leading whitespace, ends with colon + const nameMatch = line.match(/^([a-zA-Z][\w-]*):\s*$/); // List item: leading whitespace + '- ' + value const itemMatch = line.match(/^\s+-\s+(.+?)\s*$/); if (nameMatch) { diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 03e2e5a..6473c77 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -102,7 +102,8 @@ Every PR description (written by a human or agent) must follow this structure: | A2 | Enable TypeScript type-checking via `checkJs: true` in `tsconfig.json` with JSDoc annotations. This catches type errors in plain `.js` files without requiring a full TS migration. | #54 | | A3 | `death-clock-core.js` must never reference the DOM (`document`, `window`, `getElementById`, etc.). All DOM wiring belongs in `src/js/`. This boundary keeps the core unit-testable. | AGENTS.md | | A4 | The CommonJS + browser dual-export pattern (`module.exports` for Jest, `window.DeathClockCore` for the browser) must be maintained. Do not convert to ES modules without updating all consumers. | AGENTS.md | -| A5 | Composite action outputs must be declared statically in `action.yml`. For dynamic per-filter outputs, expose a `changes` JSON blob output so callers can read any filter name. `git worktree remove --force` must be called at the end of any script that opens a worktree — otherwise a second call in the same job fails with "already checked out". | #— | +| A5 | Composite action outputs must be declared statically in `action.yml`. For dynamic per-filter outputs (filter names unknown at authoring time), expose a `changes` JSON blob output so callers can read any filter name without needing static declarations for each. | #— | +| A6 | `git worktree remove --force` must be called at the end of any deploy script that opens a worktree. Without cleanup, a second call in the same job fails with "already checked out at path". In tests, use different-length file content when writing to the same path twice within a second — rsync's quick-check uses mtime+size and will skip a same-size same-mtime file. | #— | --- diff --git a/tests/paths-filter.test.js b/tests/paths-filter.test.js index 0bd2a93..cc8674b 100644 --- a/tests/paths-filter.test.js +++ b/tests/paths-filter.test.js @@ -470,9 +470,6 @@ describe('main', () => { main(); - const written = process.stdout.write.mock - ? process.stdout.write.mock.calls.map(c => c[0]).join('') - : ''; // Verify GITHUB_OUTPUT was written expect(fs.appendFileSync).toHaveBeenCalledWith( '/tmp/test-output', From 8eb3f1a479c89da950734544e1c5a6ebe1adfb63 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 12:32:21 +0000 Subject: [PATCH 05/10] fix: restore 100% coverage, add trailing-slash patterns, set keep_files in deploy.yml Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/6069e328-f7f9-49c0-8432-e49f35b9902a Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- .github/actions/paths-filter/filter.js | 17 +++- .github/workflows/deploy.yml | 1 + docs/LEARNINGS.md | 11 ++- package.json | 2 +- tests/paths-filter.test.js | 126 +++++++++++++++++++++++++ 5 files changed, 151 insertions(+), 6 deletions(-) diff --git a/.github/actions/paths-filter/filter.js b/.github/actions/paths-filter/filter.js index 881a26b..00c54be 100644 --- a/.github/actions/paths-filter/filter.js +++ b/.github/actions/paths-filter/filter.js @@ -70,25 +70,33 @@ function parseFilters(yaml) { /** * Convert a glob pattern to a regular expression string (without anchors). * Handles: + * foo/ — trailing slash means "all files inside directory foo/" (equiv. to foo/**) * ** — matches any path including slashes * * — matches any characters within a single path segment * ? — matches a single non-slash character * rest — literals, with regex-special chars escaped * + * The trailing-slash convention lets filter lists unambiguously refer to + * directories: 'src/' matches every file inside src/, while 'src' matches + * only a file literally named 'src' (since git diff --name-only never + * outputs bare directory names). + * * @param {string} glob * @returns {string} */ function globToRegex(glob) { + // A trailing '/' means "all files inside this directory" — treat as dir/** + const pattern = (glob || '').endsWith('/') ? glob + '**' : (glob || ''); const regexSpecial = new Set(['.', '+', '^', '$', '{', '}', '(', ')', '|', '[', ']', '\\']); let re = ''; let i = 0; - while (i < glob.length) { - const ch = glob[i]; - if (ch === '*' && i + 1 < glob.length && glob[i + 1] === '*') { + while (i < pattern.length) { + const ch = pattern[i]; + if (ch === '*' && i + 1 < pattern.length && pattern[i + 1] === '*') { re += '.*'; i += 2; // Skip the optional trailing slash after ** (e.g. "tests/**/") - if (i < glob.length && glob[i] === '/') i++; + if (i < pattern.length && pattern[i] === '/') i++; } else if (ch === '*') { re += '[^/]*'; i++; @@ -290,6 +298,7 @@ function main() { } // Run main when executed directly; export everything for testing +/* istanbul ignore next */ if (require.main === module) { main(); } diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index d658fe1..0d424b9 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -31,6 +31,7 @@ jobs: with: github_token: ${{ secrets.GITHUB_TOKEN }} publish_dir: ./ + keep_files: true commit_message: "chore: deploy ${{ github.sha }} to gh-pages" exclude_assets: | .git diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 6473c77..bc4e180 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -147,7 +147,16 @@ Entries are grouped by release. Add new entries at the top of the appropriate re ### v1.7.x -#### PR #— feat: composite actions for gh-pages-deploy and paths-filter with full test coverage +#### PR #— fix: codecov line coverage restored, trailing-slash directory patterns, keep_files and glob coverage + +- **Problem:** Adding `filter.js` to Jest coverage dropped project-wide line coverage below 100% (line 294 — the `if (require.main === module)` guard — was uncovered), triggering codecov's zero-tolerance threshold. `deploy.yml` didn't explicitly set `keep_files: true`. Filter patterns had no way to unambiguously target a directory vs a file with the same name. +- **Approach:** (1) Added `/* istanbul ignore next */` to the main-module guard (standard Node.js pattern; impossible to cover without spawning a subprocess); (2) added 12 targeted tests to cover all remaining uncovered branches — `computeBase` default case with empty SHA, `runFilter` with `undefined` listFiles, `getChangedFiles` error paths with empty stderr, and three `main()` env-var fallback paths; (3) implemented trailing-slash directory pattern support directly in `globToRegex` (`src/` → internally treated as `src/**`), with disambiguation vs a same-name file guaranteed because git diff only returns file paths, never bare directory names; (4) widened `collectCoverageFrom` from a specific path to `.github/actions/**/*.js` so any future action scripts are auto-tracked; (5) added `keep_files: true` to `deploy.yml`. +- **Learning:** Adding a new file to `collectCoverageFrom` that has even one uncovered line can drop project-wide LINE coverage, triggering codecov's `threshold: 100%` zero-tolerance check. Use `/* istanbul ignore next */` on `if (require.main === module)` in every Node.js action script — it's a standard guard that can only run outside a test context. `||` operators create Istanbul "binary-expr" branches; both sides must be exercised. Use a glob (`.github/actions/**/*.js`) in `collectCoverageFrom` so new action scripts are automatically tracked. Trailing `/` on a filter pattern is unambiguous in git-diff context: `src/` matches files inside `src/`; `src` matches only a file literally named `src` at that path. (→ T5, A5) +- **Key files:** `.github/actions/paths-filter/filter.js`, `package.json`, `.github/workflows/deploy.yml`, `tests/paths-filter.test.js`, `docs/LEARNINGS.md` + +--- + + - **Problem:** The `peaceiris/actions-gh-pages` and `dorny/paths-filter` replacements were implemented as inline shell scripts without a clean reusable-action interface, and `filter.js` was not tracked by codecov. - **Approach:** Created two composite actions (`.github/actions/paths-filter/` and `.github/actions/gh-pages-deploy/`) with inputs matching the original third-party actions. `detect-changes.yml` now uses `paths-filter` action. `deploy.yml` and `preview.yml` now use `gh-pages-deploy` action. `filter.js` exports all pure functions (injection-safe via env vars) and is added to `collectCoverageFrom` with per-file thresholds. 67 Jest tests cover `filter.js` at 99%/90% stmt/branch. 19 bash tests cover `deploy.sh`. Fixed injection in `preview-cleanup.yml` (moved `${{ github.event.number }}` to `env:`). diff --git a/package.json b/package.json index 73cfa06..83f2c07 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "collectCoverageFrom": [ "death-clock-core.js", "script.js", - ".github/actions/paths-filter/filter.js" + ".github/actions/**/*.js" ], "coverageThreshold": { "global": { diff --git a/tests/paths-filter.test.js b/tests/paths-filter.test.js index cc8674b..04cf183 100644 --- a/tests/paths-filter.test.js +++ b/tests/paths-filter.test.js @@ -193,6 +193,31 @@ describe('globToRegex', () => { test('** alone matches any path', () => { expect(match('**', 'anything/deep/path.js')).toBe(true); }); + + // Trailing slash — directory patterns + test('trailing slash matches all files inside a directory', () => { + expect(match('src/', 'src/app.js')).toBe(true); + expect(match('src/', 'src/deep/util.ts')).toBe(true); + // Does NOT match a file literally named 'src' (no slash in path) + expect(match('src/', 'src')).toBe(false); + // Does NOT match an unrelated sibling directory + expect(match('src/', 'other/src/app.js')).toBe(false); + }); + + test('trailing slash distinguishes directory from same-name file', () => { + // 'src' matches only a file literally named 'src' + expect(match('src', 'src')).toBe(true); + expect(match('src', 'src/app.js')).toBe(false); + // 'src/' matches files inside the src/ directory, not the bare file 'src' + expect(match('src/', 'src')).toBe(false); + expect(match('src/', 'src/app.js')).toBe(true); + }); + + test('nested directory trailing slash works', () => { + expect(match('tests/e2e/', 'tests/e2e/spec.js')).toBe(true); + expect(match('tests/e2e/', 'tests/e2e/deep/more.js')).toBe(true); + expect(match('tests/e2e/', 'tests/unit/spec.js')).toBe(false); + }); }); // ============================================================================ @@ -227,6 +252,11 @@ describe('computeBase', () => { test('returns empty string for pull_request with empty sha', () => { expect(computeBase('pull_request', '', 'before', 'merge')).toBe(''); }); + + test('default case returns empty string when mergeBaseSha is empty', () => { + // Covers the falsy branch of `mergeBaseSha || ''` in the default case + expect(computeBase('workflow_dispatch', '', '', '')).toBe(''); + }); }); // ============================================================================ @@ -280,6 +310,19 @@ describe('matchFiles', () => { test('returns empty array for empty file list', () => { expect(matchFiles([], ['**/*.js'])).toEqual([]); }); + + test('trailing slash pattern matches files inside the directory', () => { + const result = matchFiles(files, ['src/']); + expect(result).toContain('src/app.js'); + expect(result).toContain('src/lib.ts'); + expect(result).not.toContain('README.md'); + expect(result).not.toContain('tests/foo.test.js'); + }); + + test('trailing slash does not match a file with the same name as a directory', () => { + // 'config.yaml' is a file; 'config/' should not match it + expect(matchFiles(['config.yaml', 'config/settings.yaml'], ['config/'])).toEqual(['config/settings.yaml']); + }); }); // ============================================================================ @@ -387,6 +430,13 @@ describe('runFilter', () => { expect(changes).toEqual({}); expect(lines).toEqual(['changes={}']); }); + + test('undefined listFiles defaults to none (no file list output)', () => { + // Covers the falsy branch of `listFiles || 'none'` + const { lines } = runFilter(['src/app.js'], { code: ['**/*.js'] }, undefined); + expect(lines).toContain('code=true'); + expect(lines.some(l => l.startsWith('code_files='))).toBe(false); + }); }); // ============================================================================ @@ -425,6 +475,13 @@ describe('getChangedFiles', () => { expect(error).toContain('git ls-files failed'); }); + test('returns error message with fallback text when ls-files stderr is empty', () => { + // Covers the falsy branch of `r.stderr || '(no stderr)'` + spawnSync.mockReturnValue({ status: 1, stdout: '', stderr: '' }); + const { error } = getChangedFiles('', 'HEAD'); + expect(error).toContain('(no stderr)'); + }); + test('returns error when git diff fails (bad base SHA)', () => { spawnSync.mockReturnValue({ status: 128, stdout: '', stderr: 'unknown revision' }); const { files, error } = getChangedFiles('badref', 'HEAD'); @@ -432,6 +489,13 @@ describe('getChangedFiles', () => { expect(error).toContain("git diff failed for base 'badref'"); }); + test('returns error message with fallback text when diff stderr is empty', () => { + // Covers the falsy branch of `r.stderr || '(no stderr)'` for the diff path + spawnSync.mockReturnValue({ status: 128, stdout: '', stderr: '' }); + const { error } = getChangedFiles('badref', 'HEAD'); + expect(error).toContain('(no stderr)'); + }); + test('filters out empty lines from output', () => { spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\n\nb.js\n', stderr: '' }); const { files } = getChangedFiles('abc', 'def'); @@ -570,4 +634,66 @@ describe('main', () => { // Should not throw expect(() => main()).not.toThrow(); }); + + test('INPUT_SHA defaults to HEAD when not set', () => { + // Covers the falsy branch of `process.env.INPUT_SHA || 'HEAD'` + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + INPUT_EVENT_NAME: 'push', + GH_PUSH_BEFORE: 'before-sha', + GH_PR_BASE_SHA: '', + GH_MERGE_BASE_SHA: '', + GITHUB_OUTPUT: '/tmp/test-output', + // INPUT_SHA intentionally omitted → defaults to 'HEAD' + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.ts\n', stderr: '' }); + + main(); + + expect(spawnSync).toHaveBeenCalledWith( + 'git', ['diff', '--name-only', 'before-sha', 'HEAD'], { encoding: 'utf8' }, + ); + }); + + test('INPUT_EVENT_NAME defaults to empty string when not set', () => { + // Covers the falsy branch of `process.env.INPUT_EVENT_NAME || ''` + setEnv({ + INPUT_FILTERS: FILTERS_YAML, + // INPUT_EVENT_NAME intentionally omitted → defaults to '' + GH_PR_BASE_SHA: '', + GH_PUSH_BEFORE: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'HEAD', + GITHUB_OUTPUT: '/tmp/test-output', + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + + main(); + + // With no event name, computeBase defaults to mergeBaseSha (empty) → + // getChangedFiles uses git ls-files (empty base) + expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files'], { encoding: 'utf8' }); + }); + + test('INPUT_FILTERS defaults to empty string when not set, producing empty output', () => { + // Covers the falsy branch of `process.env.INPUT_FILTERS || ''` + setEnv({ + // INPUT_FILTERS intentionally omitted → defaults to '' + INPUT_EVENT_NAME: 'push', + GH_PUSH_BEFORE: '', + GH_PR_BASE_SHA: '', + GH_MERGE_BASE_SHA: '', + INPUT_SHA: 'HEAD', + GITHUB_OUTPUT: '/tmp/test-output', + }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + + main(); + + // Empty filters → output is just the changes JSON with no filter groups + expect(fs.appendFileSync).toHaveBeenCalledWith( + '/tmp/test-output', + expect.stringContaining('changes={}'), + ); + }); }); From 669de03a7fd28d2207d610af63696cf45af5af62 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 12:35:38 +0000 Subject: [PATCH 06/10] fix: address code review feedback on validation, comments, and documentation Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/6069e328-f7f9-49c0-8432-e49f35b9902a Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- .github/actions/gh-pages-deploy/deploy.sh | 6 ++++++ .github/actions/paths-filter/filter.js | 2 ++ .github/workflows/detect-changes.yml | 9 ++++++++- .github/workflows/preview-cleanup.yml | 6 ++++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/.github/actions/gh-pages-deploy/deploy.sh b/.github/actions/gh-pages-deploy/deploy.sh index d0e3a3f..a0ad639 100644 --- a/.github/actions/gh-pages-deploy/deploy.sh +++ b/.github/actions/gh-pages-deploy/deploy.sh @@ -45,6 +45,9 @@ if [[ -n "${INPUT_GITHUB_TOKEN:-}" ]]; then # Inject the token as credentials in the HTTPS URL AUTHED_URL="${REPO_URL/#https:\/\//https://x-access-token:${INPUT_GITHUB_TOKEN}@}" git remote set-url origin "${AUTHED_URL}" + # Note: the SSH-to-HTTPS conversion above assumes the standard github.com SSH + # URL format (git@github.com:owner/repo.git). GitHub Enterprise instances with + # custom hostnames will need to set an HTTPS origin URL before calling this action. fi # Fetch the existing publish branch (silently skip if it doesn't exist yet) @@ -73,6 +76,9 @@ mkdir -p "${DEST}" # so deleted files don't linger. Root deployments always preserve other content # (e.g. the previews/ directory) regardless of keep_files. if [[ "${KEEP_FILES}" != "true" && -n "${DESTINATION_DIR}" ]]; then + # Suppress errors (|| true): a non-zero exit here means DEST is already empty + # or a file is already gone — both are harmless. Genuine failures (e.g. + # permission errors on the runner) will surface at the rsync or git-add step. find "${DEST}" -mindepth 1 -delete 2>/dev/null || true fi diff --git a/.github/actions/paths-filter/filter.js b/.github/actions/paths-filter/filter.js index 00c54be..406c5dd 100644 --- a/.github/actions/paths-filter/filter.js +++ b/.github/actions/paths-filter/filter.js @@ -298,6 +298,8 @@ function main() { } // Run main when executed directly; export everything for testing +// istanbul ignore next — standard Node.js direct-run guard; cannot be +// reached from inside Jest (require.main is the jest runner, not this file) /* istanbul ignore next */ if (require.main === module) { main(); diff --git a/.github/workflows/detect-changes.yml b/.github/workflows/detect-changes.yml index ca7f8c1..161bcd2 100644 --- a/.github/workflows/detect-changes.yml +++ b/.github/workflows/detect-changes.yml @@ -86,6 +86,13 @@ jobs: if [[ "${ALWAYS_RUN}" == "true" ]]; then echo "code=true" >> "$GITHUB_OUTPUT" else - echo "code=${FILTER_CODE}" >> "$GITHUB_OUTPUT" + # Validate that FILTER_CODE is exactly 'true' or 'false' so an + # unexpected value from the filter step never silently propagates. + if [[ "${FILTER_CODE}" != "true" && "${FILTER_CODE}" != "false" ]]; then + echo "Unexpected FILTER_CODE value: '${FILTER_CODE}' — defaulting to true" >&2 + echo "code=true" >> "$GITHUB_OUTPUT" + else + echo "code=${FILTER_CODE}" >> "$GITHUB_OUTPUT" + fi fi diff --git a/.github/workflows/preview-cleanup.yml b/.github/workflows/preview-cleanup.yml index 2eec43b..b8c43a4 100644 --- a/.github/workflows/preview-cleanup.yml +++ b/.github/workflows/preview-cleanup.yml @@ -20,6 +20,12 @@ jobs: env: PR_NUMBER: ${{ github.event.number }} run: | + # Validate PR_NUMBER is a positive integer to guard against malformed + # context values that could produce an unexpected directory path. + if [[ ! "${PR_NUMBER}" =~ ^[0-9]+$ ]]; then + echo "Unexpected PR_NUMBER value: '${PR_NUMBER}' — aborting cleanup" >&2 + exit 1 + fi PR_DIR="previews/pr-${PR_NUMBER}" git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" From 82cbcab276851f13801a282d1fc129de3a7455f3 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 27 Apr 2026 12:51:41 +0000 Subject: [PATCH 07/10] fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit --- docs/LEARNINGS.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index bc4e180..68aa575 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -102,8 +102,8 @@ Every PR description (written by a human or agent) must follow this structure: | A2 | Enable TypeScript type-checking via `checkJs: true` in `tsconfig.json` with JSDoc annotations. This catches type errors in plain `.js` files without requiring a full TS migration. | #54 | | A3 | `death-clock-core.js` must never reference the DOM (`document`, `window`, `getElementById`, etc.). All DOM wiring belongs in `src/js/`. This boundary keeps the core unit-testable. | AGENTS.md | | A4 | The CommonJS + browser dual-export pattern (`module.exports` for Jest, `window.DeathClockCore` for the browser) must be maintained. Do not convert to ES modules without updating all consumers. | AGENTS.md | -| A5 | Composite action outputs must be declared statically in `action.yml`. For dynamic per-filter outputs (filter names unknown at authoring time), expose a `changes` JSON blob output so callers can read any filter name without needing static declarations for each. | #— | -| A6 | `git worktree remove --force` must be called at the end of any deploy script that opens a worktree. Without cleanup, a second call in the same job fails with "already checked out at path". In tests, use different-length file content when writing to the same path twice within a second — rsync's quick-check uses mtime+size and will skip a same-size same-mtime file. | #— | +| A5 | Composite action outputs must be declared statically in `action.yml`. For dynamic per-filter outputs (filter names unknown at authoring time), expose a `changes` JSON blob output so callers can read any filter name without needing static declarations for each. | #109 | +| A6 | `git worktree remove --force` must be called at the end of any deploy script that opens a worktree. Without cleanup, a second call in the same job fails with "already checked out at path". In tests, use different-length file content when writing to the same path twice within a second — rsync's quick-check uses mtime+size and will skip a same-size same-mtime file. | #109 | --- @@ -147,7 +147,7 @@ Entries are grouped by release. Add new entries at the top of the appropriate re ### v1.7.x -#### PR #— fix: codecov line coverage restored, trailing-slash directory patterns, keep_files and glob coverage +#### PR `#109` fix: codecov line coverage restored, trailing-slash directory patterns, keep_files and glob coverage - **Problem:** Adding `filter.js` to Jest coverage dropped project-wide line coverage below 100% (line 294 — the `if (require.main === module)` guard — was uncovered), triggering codecov's zero-tolerance threshold. `deploy.yml` didn't explicitly set `keep_files: true`. Filter patterns had no way to unambiguously target a directory vs a file with the same name. - **Approach:** (1) Added `/* istanbul ignore next */` to the main-module guard (standard Node.js pattern; impossible to cover without spawning a subprocess); (2) added 12 targeted tests to cover all remaining uncovered branches — `computeBase` default case with empty SHA, `runFilter` with `undefined` listFiles, `getChangedFiles` error paths with empty stderr, and three `main()` env-var fallback paths; (3) implemented trailing-slash directory pattern support directly in `globToRegex` (`src/` → internally treated as `src/**`), with disambiguation vs a same-name file guaranteed because git diff only returns file paths, never bare directory names; (4) widened `collectCoverageFrom` from a specific path to `.github/actions/**/*.js` so any future action scripts are auto-tracked; (5) added `keep_files: true` to `deploy.yml`. @@ -158,15 +158,19 @@ Entries are grouped by release. Add new entries at the top of the appropriate re +#### PR `#109` feat: convert shell scripts to composite actions with full test coverage + - **Problem:** The `peaceiris/actions-gh-pages` and `dorny/paths-filter` replacements were implemented as inline shell scripts without a clean reusable-action interface, and `filter.js` was not tracked by codecov. - **Approach:** Created two composite actions (`.github/actions/paths-filter/` and `.github/actions/gh-pages-deploy/`) with inputs matching the original third-party actions. `detect-changes.yml` now uses `paths-filter` action. `deploy.yml` and `preview.yml` now use `gh-pages-deploy` action. `filter.js` exports all pure functions (injection-safe via env vars) and is added to `collectCoverageFrom` with per-file thresholds. 67 Jest tests cover `filter.js` at 99%/90% stmt/branch. 19 bash tests cover `deploy.sh`. Fixed injection in `preview-cleanup.yml` (moved `${{ github.event.number }}` to `env:`). -- **Learning:** Composite actions must declare outputs statically; use `changes` (JSON) as a catch-all output for dynamic filter names. `git worktree remove --force` must be called at the end of any deploy script that creates a worktree, otherwise subsequent calls in the same job fail with "already checked out" errors. rsync's quick-check uses mtime+size — tests that write the same file twice in < 1 second must use different-length content to force detection. (→ A1, S5) +- **Learning:** Composite actions must declare outputs statically; use `changes` (JSON) as a catch-all output for dynamic filter names. `git worktree remove --force` must be called at the end of any deploy script that creates a worktree, otherwise subsequent calls in the same job fail with "already checked out" errors. rsync's quick-check uses mtime+size — tests that write the same file twice in < 1 second must use different-length content to force detection. (→ A5, A6) - **Key files:** `.github/actions/paths-filter/action.yml`, `.github/actions/paths-filter/filter.js`, `.github/actions/gh-pages-deploy/action.yml`, `.github/actions/gh-pages-deploy/deploy.sh`, `tests/paths-filter.test.js`, `tests/gh-pages-deploy.test.sh`, `package.json` --- +#### PR `#109` refactor: deduplicate git diff logic with reusable workflow and shell tests + - **Problem:** The `git diff` path-filter logic was duplicated verbatim in both `unit-tests.yml` and `e2e-tests.yml`, and each copy interpolated GitHub context values (`${{ github.event_name }}`, `${{ github.sha }}`, etc.) directly into `run:` scripts — an expression-injection anti-pattern. - **Approach:** Extracted the logic into `.github/scripts/detect-changes.sh` (reads all GitHub context from env vars, never from `${{ }}` interpolation). Wrapped it in a reusable `workflow_call` workflow (`.github/workflows/detect-changes.yml`) with `event_name` and `always_run` inputs and a `code` output. Both callers now use `uses: ./.github/workflows/detect-changes.yml`. Added 19 bash unit tests in `tests/detect-changes.test.sh` and an `npm run test:shell` script. The `test` job in `unit-tests.yml` runs shell tests in CI. The git failure fallback now outputs `code=true` (safe: run CI) rather than `code=false` (unsafe: silently skip CI). - **Learning:** Pass `event_name` as an explicit `string` input to reusable workflows because `github.event_name` inside a `workflow_call` callee is always `"workflow_call"`, not the original triggering event. The `github.event` payload and `github.sha` are inherited correctly. Always default to the "run CI" safe side when git diff fails on an unresolvable ref. (→ S4, S5) @@ -176,6 +180,8 @@ Entries are grouped by release. Add new entries at the top of the appropriate re +#### PR `#109` refactor: replace third-party actions with native git worktree and diff + - **Problem:** Two third-party GitHub Actions (`peaceiris/actions-gh-pages` and `dorny/paths-filter`) added supply-chain risk from less-known organisations when native equivalents exist. - **Approach:** Replaced `peaceiris/actions-gh-pages` in `deploy.yml` and `preview.yml` with native `git worktree` + `rsync` shell steps that replicate `keep_files: true`, `destination_dir`, and `exclude_assets`. Replaced `dorny/paths-filter` in `unit-tests.yml` and `e2e-tests.yml` with a native `git diff --name-only` shell step and `fetch-depth: 0` checkout. Also added a per-PR `concurrency` group to `preview.yml` to serialize gh-pages pushes. - **Learning:** `git worktree add` is the cleanest way to check out a second branch (e.g. `gh-pages`) alongside the current checkout without a separate clone. Use `git branch --force refs/remotes/origin/` first so the worktree has a local tracking branch to push from. Always use `touch .nojekyll` in the gh-pages worktree to prevent Jekyll processing — peaceiris did this automatically. (→ S4) From 44882ef981edbd9cb90ef32362665cddece4b513 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:04:42 +0000 Subject: [PATCH 08/10] fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit --- docs/LEARNINGS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index ae6bfdc..f2ee0c2 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -115,8 +115,8 @@ Every PR description (written by a human or agent) must follow this structure: | S1 | All dynamic strings inserted via `innerHTML` must be escaped with `escHtml()` in `src/js/05-security.js`. Never assign untrusted data directly to `innerHTML`. | AGENTS.md | | S2 | GitHub Actions `uses:` references must be pinned to a full commit SHA with the semver tag as an inline comment (`@abc1234 # v3.1.0`). Mutable tags (`@v3`) can be silently redirected, creating a supply-chain risk. | AGENTS.md | | S3 | Dependabot is configured to open weekly PRs for GitHub Actions SHA bumps. Do not skip or dismiss those PRs. | AGENTS.md | -| S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | #— | -| S5 | Always pass GitHub context values to shell scripts via `env:` vars (e.g. `GH_SHA: ${{ github.sha }}`), never by interpolating `${{ }}` directly inside `run:`. Inline interpolation allows expression injection if an attacker controls the context value. | #— | +| S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | #109 | +| S5 | Always pass GitHub context values to shell scripts via `env:` vars (e.g. `GH_SHA: ${{ github.sha }}`), never by interpolating `${{ }}` directly inside `run:`. Inline interpolation allows expression injection if an attacker controls the context value. | #109 | --- From 51b7ae0fa88e06a76aacaf1de92a0893e07b77c1 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:07:48 +0000 Subject: [PATCH 09/10] fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit --- docs/LEARNINGS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 88fc08f..b4798d5 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -148,7 +148,7 @@ Entries are grouped by release. Add new entries at the top of the appropriate re ### v1.7.x -#### PR `#109` refactor: replace third-party actions with native git worktree and diff +#### PR #109 — chore: replace peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell - **Problem:** Two third-party GitHub Actions (`peaceiris/actions-gh-pages` and `dorny/paths-filter`) added supply-chain risk from less-known organisations when native equivalents exist. - **Approach:** Replaced `peaceiris/actions-gh-pages` in `deploy.yml` and `preview.yml` with native `git worktree` + `rsync` shell steps that replicate `keep_files: true`, `destination_dir`, and `exclude_assets`. Replaced `dorny/paths-filter` in `unit-tests.yml` and `e2e-tests.yml` with a native `git diff --name-only` shell step and `fetch-depth: 0` checkout. Also added a per-PR `concurrency` group to `preview.yml` to serialize gh-pages pushes. From 406f30569841b08bf0e092afe13837aaf5e74e70 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 2 May 2026 11:32:37 +0000 Subject: [PATCH 10/10] fix: address all remaining CodeRabbit PR #109 review comments Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/5186c4fa-4576-41cf-a87e-2a817e718f97 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- .github/actions/gh-pages-deploy/deploy.sh | 45 +++++++++++------ .github/actions/paths-filter/filter.js | 15 ++++-- .github/workflows/detect-changes.yml | 6 +++ .github/workflows/preview.yml | 9 ++-- .github/workflows/unit-tests.yml | 2 +- docs/LEARNINGS.md | 9 ++++ tests/gh-pages-deploy.test.sh | 61 ++++++++++++++++++++++- tests/paths-filter.test.js | 53 ++++++++++---------- 8 files changed, 151 insertions(+), 49 deletions(-) diff --git a/.github/actions/gh-pages-deploy/deploy.sh b/.github/actions/gh-pages-deploy/deploy.sh index a0ad639..e6e1940 100644 --- a/.github/actions/gh-pages-deploy/deploy.sh +++ b/.github/actions/gh-pages-deploy/deploy.sh @@ -31,6 +31,12 @@ EXCLUDE_ASSETS="${INPUT_EXCLUDE_ASSETS:-.github}" WORKTREE_DIR="$(mktemp -d)" +# Ensure the worktree is always removed on exit, even if the script fails midway. +cleanup() { + git worktree remove --force "${WORKTREE_DIR}" 2>/dev/null || rm -rf "${WORKTREE_DIR}" +} +trap cleanup EXIT + git config user.name "${USER_NAME}" git config user.email "${USER_EMAIL}" @@ -72,9 +78,8 @@ fi mkdir -p "${DEST}" -# When keep_files is false and deploying to a subdirectory, clear stale content -# so deleted files don't linger. Root deployments always preserve other content -# (e.g. the previews/ directory) regardless of keep_files. +# When keep_files is false and deploying to a subdirectory, pre-clear the +# destination so deleted files don't linger before rsync runs. if [[ "${KEEP_FILES}" != "true" && -n "${DESTINATION_DIR}" ]]; then # Suppress errors (|| true): a non-zero exit here means DEST is already empty # or a file is already gone — both are harmless. Genuine failures (e.g. @@ -82,14 +87,18 @@ if [[ "${KEEP_FILES}" != "true" && -n "${DESTINATION_DIR}" ]]; then find "${DEST}" -mindepth 1 -delete 2>/dev/null || true fi -# Ensure Jekyll processing is disabled (unless explicitly opted out) -if [[ "${DISABLE_NOJEKYLL}" != "true" ]]; then - touch "${WORKTREE_DIR}/.nojekyll" -fi - # Build rsync --exclude flags from EXCLUDE_ASSETS. # Supports both newline-separated and comma-separated values. RSYNC_EXCLUDES=() +# Always protect the .git worktree reference file so rsync --delete never +# removes it (it only exists in DEST, not in PUBLISH_DIR, so without this +# exclude the --delete flag would wipe it and break subsequent git commands). +RSYNC_EXCLUDES+=("--exclude=.git") +# For root deploys with keep_files=false, protect previews/ so rsync --delete +# doesn't remove preview directories that live alongside the production content. +if [[ -z "${DESTINATION_DIR}" && "${KEEP_FILES}" != "true" ]]; then + RSYNC_EXCLUDES+=("--exclude=previews/") +fi while IFS= read -r item; do # Trim leading and trailing whitespace item="${item#"${item%%[![:space:]]*}"}" @@ -97,8 +106,18 @@ while IFS= read -r item; do [[ -n "${item}" ]] && RSYNC_EXCLUDES+=("--exclude=${item}") done < <(printf '%s\n' "${EXCLUDE_ASSETS}" | tr ',' '\n') -# Sync source into destination -rsync -a ${RSYNC_EXCLUDES[@]+"${RSYNC_EXCLUDES[@]}"} "${PUBLISH_DIR%/}/" "${DEST}/" +# Sync source into destination. +# Pass --delete when keep_files is false so stale files are removed — for root +# deploys this is the only mechanism (the find-based clear above is skipped). +DELETE_FLAG=() +[[ "${KEEP_FILES}" != "true" ]] && DELETE_FLAG=("--delete") +rsync -a "${DELETE_FLAG[@]}" ${RSYNC_EXCLUDES[@]+"${RSYNC_EXCLUDES[@]}"} "${PUBLISH_DIR%/}/" "${DEST}/" + +# Ensure Jekyll processing is disabled after rsync (so rsync --delete cannot +# remove it — we create it after the sync, not before). +if [[ "${DISABLE_NOJEKYLL}" != "true" ]]; then + touch "${WORKTREE_DIR}/.nojekyll" +fi cd "${WORKTREE_DIR}" git add -A @@ -110,8 +129,6 @@ else echo "Deployed to ${PUBLISH_BRANCH}${DESTINATION_DIR:+/${DESTINATION_DIR}}" fi -# Unregister and remove the worktree to avoid conflicts when the action is -# called multiple times within the same job (e.g. a preview workflow that -# deploys the site and then uploads screenshots). +# Return to the main working directory so the EXIT trap's `git worktree remove` +# does not fail with "is the current directory" when the script exits. cd - -git worktree remove --force "${WORKTREE_DIR}" 2>/dev/null || rm -rf "${WORKTREE_DIR}" diff --git a/.github/actions/paths-filter/filter.js b/.github/actions/paths-filter/filter.js index 406c5dd..4241692 100644 --- a/.github/actions/paths-filter/filter.js +++ b/.github/actions/paths-filter/filter.js @@ -228,20 +228,20 @@ function runFilter(changedFiles, filtersMap, listFiles) { function getChangedFiles(base, sha) { const isEmpty = !base || base === '0000000000000000000000000000000000000000'; if (isEmpty) { - const r = spawnSync('git', ['ls-files'], { encoding: 'utf8' }); + const r = spawnSync('git', ['ls-files', '-z'], { encoding: 'utf8' }); if (r.status !== 0) { return { files: [], error: `git ls-files failed: ${r.stderr || '(no stderr)'}` }; } - return { files: r.stdout.trim().split('\n').filter(Boolean), error: null }; + return { files: r.stdout.split('\0').filter(Boolean), error: null }; } - const r = spawnSync('git', ['diff', '--name-only', base, sha], { encoding: 'utf8' }); + const r = spawnSync('git', ['diff', '--name-only', '-z', base, sha], { encoding: 'utf8' }); if (r.status !== 0) { return { files: [], error: `git diff failed for base '${base}': ${r.stderr || '(no stderr)'}`, }; } - return { files: r.stdout.trim().split('\n').filter(Boolean), error: null }; + return { files: r.stdout.split('\0').filter(Boolean), error: null }; } // --------------------------------------------------------------------------- @@ -270,6 +270,13 @@ function main() { const { files, error } = getChangedFiles(base, sha); const filtersMap = parseFilters(filtersYaml); + + if (Object.keys(filtersMap).length === 0) { + process.stderr.write('paths-filter: no valid filters parsed from INPUT_FILTERS — check your filters: input\n'); + process.exitCode = 1; + return; + } + let outputLines; if (error) { diff --git a/.github/workflows/detect-changes.yml b/.github/workflows/detect-changes.yml index 161bcd2..cb645ce 100644 --- a/.github/workflows/detect-changes.yml +++ b/.github/workflows/detect-changes.yml @@ -72,6 +72,12 @@ jobs: - '**/*.js' - '**/*.ts' - '**/*.css' + - '.github/actions/**/*.sh' + - '.github/actions/**/*.yml' + - '.github/actions/**/*.yaml' + - '.github/scripts/**/*.sh' + - '.github/workflows/**/*.yml' + - '.github/workflows/**/*.yaml' - 'tests/**' - 'package*.json' diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index b86e093..85302b7 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -4,10 +4,13 @@ on: pull_request: types: [opened, synchronize, reopened] -# Serialize gh-pages pushes per PR; cancel in-progress runs when a new push arrives. +# Serialize ALL gh-pages pushes under one shared lock so preview and production +# deploys never push to the branch simultaneously (which causes non-fast-forward +# failures). Using cancel-in-progress: false queues preview builds safely behind +# any in-flight production deploy instead of cancelling it. concurrency: - group: "pages-preview-${{ github.event.number }}" - cancel-in-progress: true + group: "pages" + cancel-in-progress: false permissions: contents: write diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 525678b..deb98ab 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -56,7 +56,7 @@ jobs: steps: - name: Report status run: | - if [[ "${{ needs.test.result }}" == "failure" ]]; then + if [[ "${{ needs.test.result }}" == "failure" || "${{ needs.changes.result }}" == "failure" ]]; then echo "Unit tests failed" exit 1 fi diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index b4798d5..43990b6 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -148,6 +148,15 @@ Entries are grouped by release. Add new entries at the top of the appropriate re ### v1.7.x +#### PR #109 (follow-up) — fix: address all remaining CodeRabbit review comments + +- **Problem:** Six unresolved CodeRabbit issues remained after the initial PR #109 merge: (1) root deploys never deleted stale files — renamed/removed pages lived on gh-pages forever; (2) `detect-changes.yml` only tracked JS/TS/CSS/tests, so shell/YAML CI file changes skipped CI entirely; (3) `preview.yml` used a different concurrency group than `deploy.yml`, allowing simultaneous gh-pages pushes; (4) `unit-tests.yml` status step didn't catch `changes` job failures; (5) `tests/gh-pages-deploy.test.sh` had `cd "${TMPWORK}"` without `|| exit 1`; (6) `filter.js` used newline-split git output (fragile for filenames with spaces/special chars) and had no fail-fast guard for empty filters. +- **Approach:** (1) Added `rsync --delete` + `--exclude=.git` (protects the worktree ref file) + `--exclude=previews/` (protects sibling preview dirs) for root deploys; moved `.nojekyll` creation to AFTER rsync to prevent `--delete` from removing it; added an EXIT trap in deploy.sh for guaranteed worktree cleanup; (2) Added 6 CI glob patterns to detect-changes.yml filter; (3) Changed preview.yml concurrency group to `"pages"` (shared with deploy.yml) + `cancel-in-progress: false`; (4) Added `needs.changes.result` check in unit-tests.yml status step; (5) Added `|| exit 1` to subshell cd; (6) Switched to NUL-delimited `-z` git output in filter.js + fail-fast guard for empty filtersMap; updated all affected tests. +- **Learning:** `rsync --delete` removes the `.git` worktree reference file from `DEST` because it only exists in destination, not source — always add `--exclude=.git` first. Create `.nojekyll` AFTER rsync (not before) to prevent `--delete` from removing it. Use `rsync --delete` + `--exclude=previews/` for root gh-pages deploys so removed pages don't stay live permanently. Use `git ls-files -z` / `git diff --name-only -z` and split on `\0` for robustness with any filenames. A shared concurrency group (`"pages"`) is required across ALL workflows that push the same branch. (→ A5, A6) +- **Key files:** `.github/actions/gh-pages-deploy/deploy.sh`, `.github/workflows/detect-changes.yml`, `.github/workflows/preview.yml`, `.github/workflows/unit-tests.yml`, `tests/gh-pages-deploy.test.sh`, `.github/actions/paths-filter/filter.js`, `tests/paths-filter.test.js` + +--- + #### PR #109 — chore: replace peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell - **Problem:** Two third-party GitHub Actions (`peaceiris/actions-gh-pages` and `dorny/paths-filter`) added supply-chain risk from less-known organisations when native equivalents exist. diff --git a/tests/gh-pages-deploy.test.sh b/tests/gh-pages-deploy.test.sh index 52ae1ca..12f2019 100644 --- a/tests/gh-pages-deploy.test.sh +++ b/tests/gh-pages-deploy.test.sh @@ -102,7 +102,7 @@ setup_repos() { # run_deploy [KEY=VAL ...] — runs deploy.sh inside TMPWORK with given env vars run_deploy() { ( - cd "${TMPWORK}" + cd "${TMPWORK}" || exit 1 env \ INPUT_GITHUB_TOKEN="" \ INPUT_PUBLISH_BRANCH="gh-pages" \ @@ -354,6 +354,65 @@ run_deploy actual=$(file_in_branch "gh-pages" "index.html") assert "update: second deploy updates file" "version two content — updated" "${actual}" +# --------------------------------------------------------------------------- +# Test: root deploy with keep_files=false removes stale files +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — root deploy removes stale files ===" +echo "" + +cleanup; setup_repos + +# Initial root deploy with a file that will become stale +echo "stale content" > "${TMPSRC}/stale.html" +echo "keep content" > "${TMPSRC}/keep.html" +run_deploy + +# Second deploy without stale.html (keep_files defaults to false) +rm "${TMPSRC}/stale.html" +echo "new content" > "${TMPSRC}/new.html" +run_deploy + +actual=$(file_in_branch "gh-pages" "new.html") +assert "root deploy keep_files=false: new file deployed" "new content" "${actual}" + +actual=$(file_in_branch "gh-pages" "keep.html") +assert "root deploy keep_files=false: unchanged file kept" "keep content" "${actual}" + +actual=$(file_in_branch "gh-pages" "stale.html") +assert "root deploy keep_files=false: stale file removed" "__MISSING__" "${actual}" + +# --------------------------------------------------------------------------- +# Test: root deploy with keep_files=false preserves previews/ directory +# --------------------------------------------------------------------------- +echo "" +echo "=== gh-pages-deploy — root deploy preserves previews/ ===" +echo "" + +# previews/ was created in the previous test setup via a subdirectory deploy +cleanup; setup_repos + +# Step 1: root deploy to create the branch +echo "site version one" > "${TMPSRC}/index.html" +run_deploy + +# Step 2: subdirectory deploy to simulate a preview +echo "preview content" > "${TMPSRC}/preview.html" +run_deploy INPUT_DESTINATION_DIR="previews/pr-99" + +# Step 3: new root deploy — previews/ must NOT be wiped +rm "${TMPSRC}/preview.html" +# Use a different-length string so rsync's mtime+size quick-check detects the change +# even when both writes happen within the same second. +echo "site version two — updated" > "${TMPSRC}/index.html" +run_deploy + +actual=$(file_in_branch "gh-pages" "index.html") +assert "root deploy preserves previews/: root file updated" "site version two — updated" "${actual}" + +actual=$(file_in_branch "gh-pages" "previews/pr-99/preview.html") +assert "root deploy preserves previews/: previews/ directory preserved" "preview content" "${actual}" + # --------------------------------------------------------------------------- # Summary # --------------------------------------------------------------------------- diff --git a/tests/paths-filter.test.js b/tests/paths-filter.test.js index 04cf183..4665a48 100644 --- a/tests/paths-filter.test.js +++ b/tests/paths-filter.test.js @@ -39,6 +39,7 @@ const ENV_KEYS = [ beforeEach(() => { ENV_KEYS.forEach(k => delete process.env[k]); + process.exitCode = 0; jest.clearAllMocks(); }); @@ -445,24 +446,24 @@ describe('runFilter', () => { describe('getChangedFiles', () => { test('uses git ls-files when base is empty', () => { - spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\nb.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\0b.js\0', stderr: '' }); const { files, error } = getChangedFiles('', 'HEAD'); - expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files'], { encoding: 'utf8' }); + expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files', '-z'], { encoding: 'utf8' }); expect(files).toEqual(['a.js', 'b.js']); expect(error).toBeNull(); }); test('uses git ls-files when base is all-zeros', () => { - spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\0', stderr: '' }); getChangedFiles('0000000000000000000000000000000000000000', 'HEAD'); - expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files'], { encoding: 'utf8' }); + expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files', '-z'], { encoding: 'utf8' }); }); test('uses git diff when base is a real SHA', () => { - spawnSync.mockReturnValue({ status: 0, stdout: 'changed.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'changed.js\0', stderr: '' }); const { files, error } = getChangedFiles('abc123', 'def456'); expect(spawnSync).toHaveBeenCalledWith( - 'git', ['diff', '--name-only', 'abc123', 'def456'], { encoding: 'utf8' }, + 'git', ['diff', '--name-only', '-z', 'abc123', 'def456'], { encoding: 'utf8' }, ); expect(files).toEqual(['changed.js']); expect(error).toBeNull(); @@ -496,8 +497,10 @@ describe('getChangedFiles', () => { expect(error).toContain('(no stderr)'); }); - test('filters out empty lines from output', () => { - spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\n\nb.js\n', stderr: '' }); + test('filters out empty entries from NUL-delimited output', () => { + // A trailing NUL (as git -z always appends) produces an empty split entry + // that filter(Boolean) must discard. + spawnSync.mockReturnValue({ status: 0, stdout: 'a.js\0b.js\0', stderr: '' }); const { files } = getChangedFiles('abc', 'def'); expect(files).toEqual(['a.js', 'b.js']); }); @@ -530,7 +533,7 @@ describe('main', () => { INPUT_SHA: 'head-sha', GITHUB_OUTPUT: '/tmp/test-output', }); - spawnSync.mockReturnValue({ status: 0, stdout: 'src/app.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'src/app.js\0', stderr: '' }); main(); @@ -551,7 +554,7 @@ describe('main', () => { INPUT_SHA: 'head-sha', GITHUB_OUTPUT: '/tmp/test-output', }); - spawnSync.mockReturnValue({ status: 0, stdout: 'README.md\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'README.md\0', stderr: '' }); main(); @@ -572,12 +575,12 @@ describe('main', () => { INPUT_SHA: 'head-sha', GITHUB_OUTPUT: '/tmp/test-output', }); - spawnSync.mockReturnValue({ status: 0, stdout: 'app.ts\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.ts\0', stderr: '' }); main(); expect(spawnSync).toHaveBeenCalledWith( - 'git', ['diff', '--name-only', 'explicit-base', 'head-sha'], { encoding: 'utf8' }, + 'git', ['diff', '--name-only', '-z', 'explicit-base', 'head-sha'], { encoding: 'utf8' }, ); }); @@ -611,7 +614,7 @@ describe('main', () => { GH_MERGE_BASE_SHA: '', INPUT_SHA: 'HEAD', }); - spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\0', stderr: '' }); main(); @@ -628,7 +631,7 @@ describe('main', () => { INPUT_SHA: 'HEAD', GITHUB_OUTPUT: '/tmp/test-output', }); - spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\0', stderr: '' }); fs.appendFileSync.mockImplementation(() => { throw new Error('disk full'); }); // Should not throw @@ -646,12 +649,12 @@ describe('main', () => { GITHUB_OUTPUT: '/tmp/test-output', // INPUT_SHA intentionally omitted → defaults to 'HEAD' }); - spawnSync.mockReturnValue({ status: 0, stdout: 'app.ts\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.ts\0', stderr: '' }); main(); expect(spawnSync).toHaveBeenCalledWith( - 'git', ['diff', '--name-only', 'before-sha', 'HEAD'], { encoding: 'utf8' }, + 'git', ['diff', '--name-only', '-z', 'before-sha', 'HEAD'], { encoding: 'utf8' }, ); }); @@ -666,17 +669,17 @@ describe('main', () => { INPUT_SHA: 'HEAD', GITHUB_OUTPUT: '/tmp/test-output', }); - spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\0', stderr: '' }); main(); // With no event name, computeBase defaults to mergeBaseSha (empty) → // getChangedFiles uses git ls-files (empty base) - expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files'], { encoding: 'utf8' }); + expect(spawnSync).toHaveBeenCalledWith('git', ['ls-files', '-z'], { encoding: 'utf8' }); }); - test('INPUT_FILTERS defaults to empty string when not set, producing empty output', () => { - // Covers the falsy branch of `process.env.INPUT_FILTERS || ''` + test('exits with code 1 when INPUT_FILTERS is not set (fail-fast guard)', () => { + // Covers the fail-fast guard: empty filtersMap → process.exitCode = 1 + early return setEnv({ // INPUT_FILTERS intentionally omitted → defaults to '' INPUT_EVENT_NAME: 'push', @@ -686,14 +689,12 @@ describe('main', () => { INPUT_SHA: 'HEAD', GITHUB_OUTPUT: '/tmp/test-output', }); - spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\n', stderr: '' }); + spawnSync.mockReturnValue({ status: 0, stdout: 'app.js\0', stderr: '' }); main(); - // Empty filters → output is just the changes JSON with no filter groups - expect(fs.appendFileSync).toHaveBeenCalledWith( - '/tmp/test-output', - expect.stringContaining('changes={}'), - ); + // Empty filters → fail-fast guard sets exit code 1 and returns before writing output + expect(process.exitCode).toBe(1); + expect(fs.appendFileSync).not.toHaveBeenCalled(); }); });