fix: doc test changed files detection

[midnightercz]

doc test incorrectly detects changes file for PR in merge queue. This fix utilize tj-actions/changed-files to correctly detect changes files in various states of PR

Assisted-By: claude

[qodo-code-review]

Review Summary by Qodo

(Agentic_describe updated until commit 04d0290)

Fix docstring test changed files detection using tj-actions/changed-files

🐞 Bug fix

Walkthroughs

Description

• Replace manual PR file detection with tj-actions/changed-files action
• Fix docstring test failing in merge queue due to incorrect file detection
• Simplify docstring test step by removing complex shell scripting logic
• Add conditional execution of docstring test only when Python files changed

Diagram

flowchart LR
  A["PR Trigger"] --> B["Get changed Python files\n(tj-actions/changed-files)"]
  B -- "any_changed == true" --> C["Run ruff docstring check\non changed files"]
  B -- "no changes" --> D["Skip docstring test"]
  C --> E["Upload coverage to Codecov"]
  D --> E

Loading

File Changes

1. .github/workflows/python.yaml 🐞 Bug fix +10/-17

Replace manual file detection with tj-actions/changed-files action

• Added tj-actions/changed-files step to detect changed Python files using glob pattern **/*.py
• Replaced manual gh pr view shell script with the output of changed-files action
• Added conditional if check so docstring test only runs when Python files are changed
• Simplified docstring test step by removing complex bash loop and mime-type checking logic

.github/workflows/python.yaml

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Context used

✅ Compliance rules (platform): 25 rules

1. Shell-injection via filenames 🐞 Bug ⛨ Security

Description

The "docstring test" step expands steps.changed-files.outputs.all_changed_files unquoted in a bash
run command, so filenames containing shell metacharacters (e.g., ;, &, $()) can terminate
the ruff command and execute additional commands. This also breaks correctness for filenames with
spaces/newlines due to word-splitting and glob expansion.

Code

.github/workflows/python.yaml[R49-55]

      - name: docstring test
+        if: steps.changed-files.outputs.any_changed == 'true'
        run: |
-          # Get list of changed python files
          pip install ruff
-          FILES=$(gh pr view ${{ github.event.pull_request.number }} --json files --jq '.files.[].path')
-          PYFILES=""
-          for _file in ${FILES}; do
-            echo "Checking file : $_file"
-            if [[ $(file --mime-type "$_file" | cut -d" " -f 2) == "text/x-script.python" ]]; then
-              PYFILES="$PYFILES $_file "
-            else
-              echo "Skipping non-python file: $_file"
-            fi
-          done
-          if [ -z "$PYFILES" ]; then
-            echo "No Python files changed."
-          else
-            echo "Running ruff on: $PYFILES"
-            ruff check --select D ${PYFILES}
-          fi
+          echo "Running ruff on changed Python files:"
+          echo "${{ steps.changed-files.outputs.all_changed_files }}"
+          ruff check --select D ${{ steps.changed-files.outputs.all_changed_files }}

Evidence

The workflow is triggered for PR-related events, and the ruff command interpolates the
changed-files output directly into a bash command without quoting, which allows bash
tokenization/operator parsing of filename contents.

.github/workflows/python.yaml[3-13]
.github/workflows/python.yaml[49-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The workflow runs `ruff check` using an unquoted interpolation of the changed-files output, which allows shell metacharacters in filenames to be interpreted by bash (command separators/substitution) and also breaks on whitespace/globs.

## Issue Context
The workflow runs on `pull_request` and `merge_group` events; changed filenames come from the PR branch and are expanded directly in a shell command.

## Fix Focus Areas
- .github/workflows/python.yaml[49-55]

### Suggested implementation
1. Configure `tj-actions/changed-files` to output a newline-separated list (set `separator: "\n"`).
2. In the `run` step, read the list into a bash array and invoke ruff with proper quoting, e.g.:

```bash
pip install ruff
mapfile -t files <<< "${{ steps.changed-files.outputs.all_changed_files }}"
ruff check --select D -- "${files[@]}"
```

This avoids word-splitting, globbing, and shell operator interpretation from filenames.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-app-for-konflux-ci]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0)

Context used

✅ Compliance rules (platform): 23 rules

1. Unquoted file list 🐞 Bug ⛨ Security

Description

The docstring test runs ruff with an unquoted expansion of `${{
steps.changed-files.outputs.all_changed_files }}`, so filenames with spaces or shell metacharacters
will be split/interpreted by bash, checking the wrong paths or enabling command injection. This can
lead to false CI results or arbitrary command execution in the runner context if a PR introduces a
crafted .py filename.

Code

.github/workflows/python.yaml[R53-55]

+          echo "Running ruff on changed Python files:"
+          echo "${{ steps.changed-files.outputs.all_changed_files }}"
+          ruff check --select D ${{ steps.changed-files.outputs.all_changed_files }}

Relevance

⭐⭐ Medium

Similar unsafe unquoted file-list expansion flagged in PR #764; no evidence team acted on/accepted
fix.

PR-#764

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The workflow passes the changed-files output directly into a bash command (`ruff check ... ${{ ...
}}`) without any quoting or safe parsing, which will trigger word-splitting and shell interpretation
of special characters.

.github/workflows/python.yaml[43-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The workflow interpolates `steps.changed-files.outputs.all_changed_files` directly into a shell command without quoting, which is unsafe for filenames containing spaces/newlines or shell metacharacters.

## Issue Context
Git paths can legally contain spaces and characters like `;`, `&`, `$()`, etc. When expanded unquoted in bash, these can be split or interpreted, causing incorrect ruff arguments or shell injection.

## Fix Focus Areas
- .github/workflows/python.yaml[49-55]

## Suggested fix
Update the `tj-actions/changed-files` step to output a delimiter you can parse safely (commonly newline), then build a bash array and invoke ruff with a quoted array.

Example pattern:
```yaml
- name: Get changed Python files
 id: changed-files
 uses: tj-actions/changed-files@...
 with:
   files: |
     **/*.py
   separator: "\n"

- name: docstring test
 if: steps.changed-files.outputs.any_changed == 'true'
 run: |
   pip install ruff
   mapfile -t pyfiles <<< "${{ steps.changed-files.outputs.all_changed_files }}"
   ruff check --select D -- "${pyfiles[@]}"
```
If `separator` is not available/desired, write the list to a file and use a safe `while IFS= read -r` loop to construct the array before calling ruff.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[Copilot]

Pull request overview

This PR fixes the GitHub Actions “docstring test” step so it correctly detects changed Python files for pull requests, including when checks run via the merge queue (merge_group), by switching from a gh pr view-based approach to tj-actions/changed-files.

Changes:

• Add a tj-actions/changed-files step to compute the set of changed **/*.py files.
• Gate the docstring check so it only runs when Python files have changed, and run ruff against the action-provided file list.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.18%. Comparing base (36f91e4) to head (04d0290).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #814   +/-   ##
=======================================
  Coverage   94.18%   94.18%           
=======================================
  Files          38       38           
  Lines        3009     3012    +3     
=======================================
+ Hits         2834     2837    +3     
  Misses        175      175           

Flag	Coverage Δ
unit-tests	94.18% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.