Skip to content

fix: add ai-review.sh to bash -n syntax check list#3005

Merged
la14-1 merged 3 commits intomainfrom
qa/code-quality
Mar 26, 2026
Merged

fix: add ai-review.sh to bash -n syntax check list#3005
la14-1 merged 3 commits intomainfrom
qa/code-quality

Conversation

@la14-1
Copy link
Member

@la14-1 la14-1 commented Mar 26, 2026

Summary

  • ai-review.sh is sourced by e2e.sh but was missing from the bash -n syntax check loop in sh/test/e2e-lib.sh
  • This meant any syntax errors introduced into ai-review.sh would not be caught by the test harness
  • Added ai-review.sh to the list alongside the other E2E lib scripts

Code Quality Scan Results

Conducted a full scan of the codebase for:

  • Dead code — No unused functions found in sh/shared/*.sh or packages/cli/src/. All exports are actively used.
  • Stale references — No references to deleted files found. All cross-references between shell scripts and TypeScript are valid.
  • Python usage — None found. All inline scripting uses bun -e or jq as required.
  • Duplicate utilities — No identical helpers duplicated across cloud modules. All use the shared module pattern correctly.
  • Stale comments — No comments referencing removed infrastructure, old test files, or deleted functions found.
  • Finding: ai-review.sh missing from syntax check list in e2e-lib.sh (fixed above)

-- qa/code-quality

ai-review.sh is sourced by e2e.sh but was missing from the bash -n
syntax check loop in sh/test/e2e-lib.sh. This means syntax errors in
ai-review.sh would not be caught by the test harness.
Copy link
Member

@louisgv louisgv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: 6320f3c

Findings

No security issues found.

Change Analysis

  • Adds sh/e2e/lib/ai-review.sh to the bash syntax check list in sh/test/e2e-lib.sh
  • The added file exists and passes bash -n syntax validation
  • The ai-review.sh script itself is secure:
    • Uses proper env var handling with safe fallbacks
    • Passes data to bun via environment variables (not interpolation) preventing injection
    • Uses mktemp for secure temp file creation
    • Properly cleans up temp files
    • Has proper timeouts on curl calls
    • Advisory-only (returns 0, won't fail tests)

Tests

  • bash -n: PASS (both e2e-lib.sh and ai-review.sh)
  • bun test: N/A (no TypeScript changes)
  • curl|bash: OK (not applicable for test suite scripts)
  • macOS compat: OK (no bash 4.x features, no echo -e)

-- security/pr-reviewer

@la14-1 la14-1 merged commit 463b839 into main Mar 26, 2026
5 checks passed
@la14-1 la14-1 deleted the qa/code-quality branch March 26, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants