fix(skills): replace stream-based lookahead with array indexing in list-changed-files.sh

[katriendg]

Fixed a bug in the pr-reference skill's list-changed-files.sh where nested read calls inside a while read loop consumed lines from the shared input stream. When consecutive diff --git headers appeared without intervening type-indicator lines, the inner read would swallow the next header, causing the outer loop to skip files or misclassify change types. The fix replaced the stream-based approach with mapfile array indexing for correct lookahead behavior.

Description

The extract_files() function in list-changed-files.sh parsed diff output to classify each changed file. A nested read for lookahead broke iteration when diff headers appeared back-to-back.

• Replaced stream-based iteration with mapfile -t lines to load all grep output into an array before the loop, switching to while (( i < count )) arithmetic iteration with explicit index-based lookahead via lines[i + 1]
• Added || true guards to all (( i++ )) arithmetic expressions to prevent set -e failures when the pre-increment value evaluates as falsy in Bash
• Restructured rename detection into distinct conditional branches: ^rename\ from matching, path-mismatch checks, and a fallback for the last entry in the array where no next line exists

Related Issue(s)

Fixes #1375

Type of Change

Select all that apply:

Code & Documentation:

• Bug fix (non-breaking change fixing an issue)
• New feature (non-breaking change adding functionality)
• Breaking change (fix or feature causing existing functionality to change)
• Documentation update

Infrastructure & Configuration:

• GitHub Actions workflow
• Linting configuration (markdown, PowerShell, etc.)
• Security configuration
• DevContainer configuration
• Dependency update

AI Artifacts:

• Reviewed contribution with prompt-builder agent and addressed all feedback
• Copilot instructions (.github/instructions/*.instructions.md)
• Copilot prompt (.github/prompts/*.prompt.md)
• Copilot agent (.github/agents/*.agent.md)
• Copilot skill (.github/skills/*/SKILL.md)

Other:

• Script/automation (.ps1, .sh, .py)
• Other (please describe):

Sample Prompts (for AI Artifact Contributions)

Testing

• Diff-based assessment: Verified the extract_files() function's final state matches expected array-indexed iteration pattern with correct || true guards and rename detection branches.
• Automated validation: Results from required automated checks are recorded below.
• Manual testing was not performed.

Checklist

Required Checks

• Documentation is updated (if applicable) (N/A — no documentation changes required for this bug fix)
• Files follow existing naming conventions
• Changes are backwards compatible (if applicable)
• Tests added for new functionality (if applicable) (N/A — bug fix to existing functionality)

AI Artifact Contributions

• Used /prompt-analyze to review contribution
• Addressed all feedback from prompt-builder review
• Verified contribution follows common standards and type-specific requirements

Required Automated Checks

The following validation commands must pass before merging:

• Markdown linting: npm run lint:md
• Spell checking: npm run spell-check
• Frontmatter validation: npm run lint:frontmatter
• Skill structure validation: npm run validate:skills
• Link validation: npm run lint:md-links
• PowerShell analysis: npm run lint:ps
• Plugin freshness: npm run plugin:generate
• Docusaurus tests: npm run docs:test

Security Considerations

• This PR does not contain any sensitive or NDA information
• Any new dependencies have been reviewed for security issues (N/A — no dependency changes)
• Security-related scripts follow the principle of least privilege (N/A — no security script modifications)

Additional Notes

• The mapfile approach trades bounded memory for iteration correctness, which is appropriate since grep output is bounded by the number of files in a diff.
• The 2>/dev/null || true on the grep invocation was preserved to handle empty input files gracefully.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.65%. Comparing base (4adb608) to head (edc55e7).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1376      +/-   ##
==========================================
- Coverage   87.66%   87.65%   -0.02%     
==========================================
  Files          61       61              
  Lines        9328     9328              
==========================================
- Hits         8177     8176       -1     
- Misses       1151     1152       +1     

Flag	Coverage Δ
pester	85.22% <ø> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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

[github-actions]

Advisory review, this PR is from a maintainer. Findings are informational only.

Review Summary

✅ This PR meets initial quality standards. The fix is correct, well-scoped, and addresses the root cause of the stream-consumption bug.

---

Issue Alignment

✅ Fixes #1375. The PR directly targets the identified root cause: a nested read call inside a while read loop that consumed lines from the shared process-substitution stream. When two consecutive diff --git headers appeared without an intervening type-indicator line, the inner read would swallow the next header, silently dropping files from the output. The mapfile-plus-index-based approach is the canonical Bash fix for this class of problem.

---

PR Template Compliance

✅ All required sections are filled in. Type-of-change checkboxes (Bug fix, Script/automation) correctly reflect the change. The Tests added for new functionality item is appropriately annotated as N/A with rationale. Security and checklist items are correctly scoped.

---

Coding Standards

✅ The script follows the repository's Bash conventions:

• #!/usr/bin/env bash shebang is present.
• Copyright and SPDX headers are intact.
• set -euo pipefail strict mode is in place.
• [[ ... ]] and (( ... )) are used correctly.
• local scoping is applied to all function-local variables.

---

Code Quality

✅ The implementation is correct across all cases:

• Back-to-back diff --git (no type indicator): next_line doesn't match any type pattern, no spurious i++ is issued, and the outer (( i++ )) || true advances normally.
• New/deleted file: The type-indicator line is consumed by the inner (( i++ )) || true, and the outer increment advances to the next diff --git. Correct.
• Rename (rename from in filtered output): The inner increment consumes the rename from line; paths come from the diff --git header. Correct.
• Last line in array (no lookahead possible): Falls through to the elif [[ "$old_path" != "$new_path" ]] branch for path-mismatch renames. Correct.
• (( i++ )) || true guards: Necessary and correct — Bash exits with status 1 when an arithmetic expression evaluates to 0 (post-increment of i=0 returns 0, which is falsy under set -e). The || true guard is the right idiom.
• Empty input: mapfile produces an empty array, count=0, the while loop never executes. Correct.

---

Action Items

None — the PR is ready to merge.

Note

🔒 Integrity filter blocked 1 item

The following item were blocked because they don't meet the GitHub integrity level.

• #1375 issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by PR Review for issue #1376 · ● 844.1K

[WilliamBerryiii]

@katriendg ... do you know if the powershell version suffers from this as well?