Copilot/reference 28832299172#793
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Bryan-Roe, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
@Bryan-Roe Thanks for sending me some feedback. Unfortunately, I hit an error while trying to use the custom Copilot setup steps configured for this repository. The error I am seeing is: Once you or someone with the necessary access fixes the problem, please let me know in a comment and I'll try again. Thanks! |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e0ed2024-c620-442d-85ac-4e04d2a0a7e5) |
![bryan-roe-bot[bot]](https://avatars.githubusercontent.com/in/1073263?s=60&v=4){kind=small}
| run: | | ||
| set -euo pipefail | ||
|
|
||
| FILE_LIST="${{ steps.targets.outputs.file_list }}" | ||
|
|
||
| if [ -z "$FILE_LIST" ]; then | ||
| echo "No Markdown files changed for this run, or all changed Markdown files were excluded from linting." | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [ ! -f "$FILE_LIST" ] || [ ! -s "$FILE_LIST" ]; then | ||
| echo "No Markdown files changed for this run, or all changed Markdown files were excluded from linting." | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Read the NUL-delimited target list written by write_targets_from_diff. | ||
| mapfile -d '' -t markdown_files < "$FILE_LIST" | ||
|
|
||
| printf 'Linting markdown targets:\n' | ||
| printf ' - %s\n' "${markdown_files[@]}" | ||
|
|
||
| args=() | ||
| if [ "$EVENT_NAME" = "workflow_dispatch" ] && [ "${{ inputs.fix }}" = "true" ]; then | ||
| args+=(--fix) | ||
| fi | ||
|
|
||
| npx --yes markdownlint-cli2 "${args[@]}" "${markdown_files[@]}" | ||
|
|
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR".
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by run-shell-injection.
You can view more details about this finding in the Semgrep AppSec Platform.
There was a problem hiding this comment.
Pull request overview
This PR changes the Markdown Quality CI workflow so it no longer lints every *.md file in the repository on each run. Instead it computes the set of added/changed/renamed Markdown files from git history (PR base↔head, push before↔sha, or merge-base against the default branch on manual dispatch), applies the existing path exclusions, and lints only that explicit file list with npx markdownlint-cli2 (exiting successfully when nothing qualifies). A new unit test locks in the fetch depth, diff-based selection, skip-when-empty messaging, and that the lint step no longer uses the blanket-glob action.
Changes:
- Switched checkout to
fetch-depth: 0and added a "Select markdown targets" step that builds a NUL-delimited list of changed.mdfiles with the same exclusions as before. - Replaced the
markdownlint-cli2-action(repo-wide glob) withnpx --yes markdownlint-cli2over the explicit changed-file list, preserving the--fixbehavior on dispatch. - Added
tests/test_markdown_quality_workflow.pyasserting the new workflow structure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/markdown-quality.yml |
Full-history checkout + diff-based target selection; lint step rewritten to run npx markdownlint-cli2 on only changed Markdown files. |
tests/test_markdown_quality_workflow.py |
New unit test validating fetch depth, diff selection, skip-when-empty messaging, and absence of the blanket-glob action. |
Key points raised in comments:
- The lint invocation
npx --yes markdownlint-cli2is unpinned, making CI non-reproducible and dependent on whatever version npm resolves; the sibling workflow pins its equivalent (copilot-setup-steps.yml:199). - On
workflow_dispatchfrom the default branch the merge-base equals HEAD, yielding an empty diff so nothing is linted or auto-fixed — this degrades thefixdispatch flow that previously linted all Markdown. The sibling workflow avoids this with aninclude_all/ default-targets fallback.
| args+=(--fix) | ||
| fi | ||
|
|
||
| npx --yes markdownlint-cli2 "${args[@]}" "${markdown_files[@]}" |
| elif [ "$EVENT_NAME" = "workflow_dispatch" ]; then | ||
| git fetch --no-tags origin "$DEFAULT_BRANCH" | ||
| BASE_SHA="$(git merge-base "origin/$DEFAULT_BRANCH" "$HEAD_SHA")" | ||
| write_targets_from_diff "$BASE_SHA" "$HEAD_SHA" || true | ||
| fi |
Summary
Changes
Testing
Checklist
.github/copilot-instructions.md, instruction files)python scripts/test_runner.py --unit)Note
Low Risk
CI-only change to markdown lint scope; no runtime app or security paths affected, with behavior to skip when no targets.
Overview
Markdown Quality CI no longer lints every
*.mdin the repo. It now builds a target list from git history and runs markdownlint-cli2 only on those paths (or exits successfully when nothing qualifies).Checkout uses
fetch-depth: 0so PR/push/workflow_dispatch runs can diff the right commits. A new Select markdown targets step diffs base vs head (PR base/head, pushbefore/sha, or merge-base on manual dispatch), keeps added/changed/renamed.mdfiles that still exist, and applies the same exclusions as before (e.g.node_modules,CHANGELOG.md,data_out). The lint step was switched from the markdownlint-cli2 GitHub Action tonpx markdownlint-cli2over that explicit file list;--fixon dispatch is unchanged.A unit test loads the workflow YAML and locks in full fetch, diff-based selection, skip-when-empty messaging, and that lint does not use a blanket repo glob action.
Reviewed by Cursor Bugbot for commit 38ca13a. Bugbot is set up for automated code reviews on this repo. Configure here.