👣 [just] gh-process v4.4 fixes bugs and adds install script for prereqs

[chicks-net]

Done

• 👣 [just] gh-process v4.4 fixes bugs and adds install script for prereqs

Meta

(Automated in .just/gh-process.just.)

[Copilot]

Pull request overview

This PR updates the gh-process justfile workflow to version 4.4, fixing bugs and adding infrastructure for managing prerequisites. Key improvements include removing debug tracing from the PR workflow, adding pull request validation, and introducing comprehensive tooling for shellcheck linting and prerequisite installation.

• Removes debug tracing (set -x) from PR creation workflow for cleaner output
• Adds new recipes for PR body management, release age tracking, and automated workflow updates
• Introduces shellcheck linting for bash scripts in just recipes and prerequisite installation script

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
justfile	Imports new shellcheck.just module for bash script linting
.just/shellcheck.just	New module that extracts and validates bash scripts from just recipes using shellcheck
.just/install-prerequisites.sh	New installation script that detects OS and helps install required tools (just, gh, shellcheck, markdownlint-cli2, jq)
.just/gh-process.just	Updates PR workflow to v4.4 with bug fixes, new validation checks, and additional recipes for PR management and release tracking

---

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

[Copilot]

The backslash before the exclamation mark is unnecessary and unconventional in bash. Use if ! gh pr view &>/dev/null; then instead for standard negation syntax.

Suggested change

	if \! gh pr view &>/dev/null; then
	if ! gh pr view &>/dev/null; then

[Copilot]

The dependency chain using && creates an implicit requirement that claude_review always runs after _on_a_pull_request. The comment on line 190 '# chains to claude_review' suggests this is intentional, but the recipe body still contains logic. Consider whether the chaining should happen after the recipe's own logic completes, or clarify the execution flow in the comment.

[claude]

Code Review: PR #21 - gh-process v4.4

Summary

This PR adds significant enhancements to the just-based workflow automation, including bug fixes, new prerequisite installation tooling, and shellcheck integration. Overall, the changes are well-structured and add valuable functionality.

Positive Aspects ✅

1. Good bash practices: Consistent use of set -euo pipefail for strict error handling
2. Cross-platform support: The install script handles macOS, Linux (apt-get, dnf, pacman), and provides fallback guidance
3. Clear user feedback: Excellent use of color-coded output and informative messages
4. Smart architecture: The shellcheck.just AWK-based script extraction is clever and well-commented
5. Version bump: Updated from v3.9 to v4.4 with meaningful improvements

Issues & Concerns 🔴

High Priority

1. Security: Temp file race condition (.just/gh-process.just)

   • Lines 37, 210, 214, 223, 237, 257, 261, 270, 303: Using mktemp /tmp/justfile.XXXXXX
   • Issue: On multi-user systems, predictable temp file patterns can be exploited
   • Fix: Use mktemp without a pattern (e.g., mktemp) or use mktemp -t justfile.XXXXXX for safer temp file creation
   • Affects: pr, pr_update, pr_verify recipes

2. Quoting issue in AWK system() call (.just/pr_verify:283, 296, 312)

   • Lines use: system("cat '"$stdin_content"'")
   • Issue: If $stdin_content contains spaces or special characters, this could fail or cause unexpected behavior
   • Fix: AWK's system() with shell variable interpolation in double quotes - should use proper escaping or pass via AWK variable

3. Incomplete error handling (.just/install-prerequisites.sh:135)

   • npm install -g markdownlint-cli2 may require sudo on some systems
   • Fix: Add guidance or error handling for permission issues

Medium Priority

4. Removed debugging output (.just/gh-process.just:20)

   • Changed from set -euxo pipefail with set +x to just set -euo pipefail
   • Removed the -x (xtrace) flag
   • Consideration: This is intentional per the PR description ("fixes bugs"), but may make debugging harder. Consider adding a debug mode toggle.

5. Potential AWK pattern over-matching (.just/shellcheck.just:47)

   • Pattern /^[a-zA-Z_]/ && /:/ could match variable assignments like foo:="bar"
   • Note: The comment on line 44 acknowledges this concern and explains the shebang check mitigates it, but it's still worth monitoring

6. Missing temp file cleanup on error (.just/gh-process.just)

   • If script fails between temp file creation and cleanup, files remain
   • Fix: Add trap handlers for cleanup, like shellcheck.just:18 does

7. Git operations without verification (.just/gh-process.just:399)

   • again recipe does git push without checking if there are changes to push
   • Fix: Add git diff --quiet check or similar

Low Priority

8. Hard-coded sleep values

   • Line 146: sleep 1 after release creation
   • Line 401: sleep 2 before pr_checks
   • Issue: Arbitrary sleeps can be unreliable; API operations might need more/less time
   • Suggestion: Consider exponential backoff or polling for readiness

9. Missing file existence check (.just/shellcheck.just:12)

   • JUSTFILES=(justfile .just/*.just) - if no .just/*.just files exist, bash will include the literal pattern
   • Note: Handled by line 28's file check, but could use shopt -s nullglob for cleaner approach

10. Inconsistent error exit codes

    • Various exit codes (100, 101, 102, 103) are used but not documented
    • Suggestion: Add a comment explaining the exit code scheme

Suggestions 💡

1. Test coverage: Consider adding tests for the AWK script extraction logic in shellcheck.just
2. Documentation: The install-prerequisites.sh script is well-documented, but the new recipes (pr_update, pr_verify, release_age, again) could use brief descriptions in comments
3. Color variable consistency: install-prerequisites.sh defines color codes but gh-process.just uses just's templating (e.g., {{BLUE}}). This is fine but worth noting
4. Dependency on claude user: The claude_review recipe filters for author.login == "claude" - ensure this is future-proof if the automation account name changes

Security Considerations 🔒

1. Command injection risk: AWK system() calls with shell variable interpolation (mentioned in issue Feature suggestion: add support for mpt(x)sas via sas(x)ircu utilities #2)
2. Temp file security: Predictable temp file names (mentioned in issue WWN from MegaCli not always match with /dev/disk/by-id #1)
3. npm global install: Requires careful permission handling

Testing Recommendations 🧪

1. Test install-prerequisites.sh on fresh Linux VMs with different package managers
2. Test AWK script extraction with edge cases: nested colons, multiline strings in recipes
3. Test temp file handling with concurrent executions of pr_update/pr_verify
4. Verify pr_verify with stdin containing special characters, quotes, backticks

Performance Considerations ⚡

No significant performance concerns. The AWK-based script extraction is efficient, and temp file usage is reasonable for the workflow.

Conclusion

This is a solid PR that adds useful functionality. The main concerns are around temp file security and quoting in AWK system() calls. Once those are addressed, this will be a great addition to the workflow automation.

Recommendation: Address high-priority security issues before merging. Medium and low priority items can be follow-up work.

---

Review generated by Claude Code