Quote workflow shell repository expansions to clear SC2086 findings - #48798
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| - name: Audit README via GitHub repository page | ||
| run: | | ||
| geo audit --url https://github.com/${{ github.repository }} --format json \ | ||
| geo audit --url "https://github.com/${{ github.repository }}" --format json \ |
| assert.Contains(t, text, "pr-data-head-sha.txt", "Shared PR prefetch should persist head SHA marker") | ||
| assert.Contains(t, text, "--json number,title,body,headRefName,headRefOid,additions,deletions,changedFiles,files", "Shared PR prefetch should capture head SHA in metadata") | ||
| assert.Contains(t, text, "Cache hit: using pre-fetched PR data for head", "Shared PR prefetch should verify cache by current head SHA") | ||
| assert.Contains(t, text, `gh pr diff "$PR_NUMBER" --repo "$EXPR_GITHUB_REPOSITORY"`, "Shared PR prefetch should quote repository on gh pr diff") |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #48798 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Pull request overview
Quotes repository-related shell expansions to resolve SC2086 findings in generated workflows.
Changes:
- Quotes shared PR-prefetch repository arguments.
- Introduces a quoted GEO repository URL environment variable.
- Regenerates affected workflow lock files.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/shared/pr-diff-data-fetch.md |
Quotes gh pr repository arguments. |
.github/workflows/pr-code-quality-reviewer.lock.yml |
Regenerates shared prefetch logic. |
.github/workflows/mattpocock-skills-reviewer.lock.yml |
Regenerates shared prefetch logic. |
.github/workflows/impeccable-skills-reviewer.lock.yml |
Regenerates shared prefetch logic. |
.github/workflows/daily-geo-optimizer.md |
Safely supplies the GEO audit repository URL. |
.github/workflows/daily-geo-optimizer.lock.yml |
Regenerates the GEO optimizer workflow. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Medium
| - name: Audit README via GitHub repository page | ||
| run: | | ||
| geo audit --url https://github.com/${{ github.repository }} --format json \ | ||
| geo audit --url "$REPOSITORY_URL" --format json \ |
| echo "Cache hit: using pre-fetched PR data for head ${CURRENT_HEAD_SHA} (${LINES} diff lines, ${COMMENT_COUNT} review comments)" | ||
| else | ||
| { gh pr diff "$PR_NUMBER" --repo $EXPR_GITHUB_REPOSITORY \ | ||
| { gh pr diff "$PR_NUMBER" --repo "$EXPR_GITHUB_REPOSITORY" \ |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. |
There was a problem hiding this comment.
Review: Quoting fixes for SC2086
Clean, mechanical fix. Verified the shared pr-diff-data-fetch.md quoting changes are correctly propagated into all three dependent lock files, and the geo-optimizer job-level REPOSITORY_URL env var change is consistent with the new os.environ["REPOSITORY_URL"]/os.environ["GITHUB_REPOSITORY"] usage in the Python heredoc.
Notes
GITHUB_REPOSITORYis a GitHub Actions default env var, always present at runtime, so bare dict access is safe.REPOSITORY_URLis declared at job level, correctly threading through to the later Python step.- No correctness, security, or performance regressions found in the changed lines. No blocking issues.
🔎 Code quality review by PR Code Quality Reviewer · sonnet50 · 20.4 AIC · ⌖ 7.04 AIC · ⊞ 7.5K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review
Applied /codebase-design - changes are focused, consistent, and correct.
Root cause fixed in the shared source (pr-diff-data-fetch.md) so all three consumer lock files update correctly. The daily-geo-optimizer.md correctly uses an env block and os.environ access - the right pattern for passing GitHub context values. Lock files regenerated in sync. No blocking issues found.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 25.1 AIC · ⌖ 4.75 AIC · ⊞ 6.8K
Comment /matt to run again
There was a problem hiding this comment.
The SC2086 fixes look correct and complete.
- The
REPOSITORY_URLenv-var pattern indaily-geo-optimizer.mdis the right approach: Actions expressions (${{ }}) cannot be double-quoted directly in shell, so hoisting the value into an env var and quoting"$REPOSITORY_URL"is idiomatic and safe. - Quoting
"$EXPR_GITHUB_REPOSITORY"in the shared PR-prefetch fragment fixes the remaining ShellCheck findings without any behavioural change. - Lock files are correctly regenerated from the updated shared fragment.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 22.3 AIC · ⌖ 5.85 AIC · ⊞ 5.1K
|
🎉 This pull request is included in a new release. Release: |
Static analysis reported
actionlint/ShellCheckSC2086warnings in generated workflowrun:blocks. The remaining hits came from unquoted repository-related shell expansions in one shared PR prefetch fragment and one workflow-local GEO audit command.Shared PR prefetch quoting
.github/workflows/shared/pr-diff-data-fetch.mdto quoteEXPR_GITHUB_REPOSITORYingh pr diffandgh pr view.impeccable-skills-reviewer.lock.ymlmattpocock-skills-reviewer.lock.ymlpr-code-quality-reviewer.lock.ymlGeo optimizer URL quoting
.github/workflows/daily-geo-optimizer.mdto quote the repository URL passed togeo audit.daily-geo-optimizer.lock.yml.Regenerated compiled workflows
.lock.ymlfiles so the source and generated workflows stay in sync with the quoting changes.Focused regression coverage
pkg/cli/pr_prefetch_cache_contract_test.gowith assertions that pin the quoted shared prefetch commands and the quoted GEO audit URL.Example of the change: