Skip to content

fix: update ralph_import.sh#148

Merged
frankbria merged 2 commits into
frankbria:mainfrom
dawenrenhub:bugfix/ralph-import-fix
Feb 1, 2026
Merged

fix: update ralph_import.sh#148
frankbria merged 2 commits into
frankbria:mainfrom
dawenrenhub:bugfix/ralph-import-fix

Conversation

@dawenrenhub
Copy link
Copy Markdown
Contributor

@dawenrenhub dawenrenhub commented Feb 1, 2026

Used outdated npx command to verify Claude code CLI while other places all used CLAUDE_CODE_CMD="claude"

Summary by CodeRabbit

  • Chores
    • Improved dependency checks in the build script to better detect required tooling and show clearer, actionable warnings when tools are missing; no change to runtime behavior or workflow.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 1, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Replaces a direct npx version check for the Claude Code CLI in ralph_import.sh with two presence checks: one for jq (warns if missing) and one for a CLAUDE_CODE_CMD command variable (warns it will be downloaded when first used). Control flow and no-early-exit behavior remain unchanged.

Changes

Cohort / File(s) Summary
Dependency check updates
ralph_import.sh
Added a jq presence warning; replaced direct npx @anthropic/claude-code`` version check with a CLAUDE_CODE_CMD-based availability check; updated warning text to reference the `CLAUDE_CODE_CMD` variable and clarified download behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Poem

🐇 I nibble at scripts, tidy each line,
jq gives me crunch, CLAUDE_CODE_CMD is fine,
Warnings that whisper where tools will appear,
A rabbit-approved change — light and clear!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix: update ralph_import.sh' is vague and generic, using non-descriptive terms that don't convey the specific nature of the change. Use a more specific title that describes the actual change, such as 'fix: replace Claude Code CLI dependency check with command availability check in ralph_import.sh' or 'fix: align ralph_import.sh to use CLAUDE_CODE_CMD variable for CLI verification'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Feb 1, 2026

Fix ralph_import.sh by changing check_dependencies to detect Claude Code via command -v "$CLAUDE_CODE_CMD" and warn when jq is missing

Update check_dependencies to use command -v for "$CLAUDE_CODE_CMD" detection and add a jq presence check with an installation hint; adjust the Claude Code warning to include the resolved command name.

📍Where to Start

Start with the check_dependencies function in ralph_import.sh.


Macroscope summarized 38fa2b2.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ralph_import.sh (1)

270-279: ⚠️ Potential issue | 🟠 Major

Add a jq dependency check with installation guidance in check_dependencies().

The guideline requires checking for jq availability and providing installation instructions upfront. While parse_conversion_response() handles missing jq gracefully (lines 127–130), proactive checking aligns with the pattern used in install.sh, ralph_loop.sh, and ralph_enable_ci.sh. Add the check as WARN since JSON parsing has a text fallback.

Suggested fix
 check_dependencies() {
     if ! command -v ralph-setup &> /dev/null; then
         log "ERROR" "Ralph not installed. Run ./install.sh first"
         exit 1
     fi
+
+    if ! command -v jq &> /dev/null; then
+        log "WARN" "jq not found. Install it (brew install jq | sudo apt-get install jq | choco install jq) for faster JSON parsing."
+    fi
     
     if ! command -v $CLAUDE_CODE_CMD &> /dev/null 2>&1; then
         log "WARN" "Claude Code CLI ($CLAUDE_CODE_CMD) not found. It will be downloaded when first used."
     fi
 }
🤖 Fix all issues with AI agents
In `@ralph_import.sh`:
- Around line 277-278: The shell check uses unquoted expansion in the if
condition: change the command invocation in the conditional that references
CLAUDE_CODE_CMD to use a quoted expansion (i.e., "${CLAUDE_CODE_CMD}" or
"$CLAUDE_CODE_CMD") so that command -v receives the variable as a single token
and won’t break on spaces or glob characters; update the if statement that
currently reads if ! command -v $CLAUDE_CODE_CMD ... to use the quoted variable
and keep the redirection (&> /dev/null 2>&1) and log call unchanged.

Comment thread ralph_import.sh Outdated
Apply CodeRabbit review suggestions:
- Quote CLAUDE_CODE_CMD variable to prevent issues with spaces/glob chars
- Add jq dependency check with installation guidance (WARN level since there's text fallback)
@frankbria frankbria merged commit 66947e7 into frankbria:main Feb 1, 2026
2 of 3 checks passed
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.

3 participants