fix(import): address additional code review feedback
- Convert CLAUDE_ALLOWED_TOOLS to bash array for proper quoting
- Use array expansion "${CLAUDE_ALLOWED_TOOLS[@]}" in CLI invocation
- Default empty version components to 0 (handles "2.1" style versions)
- Add stderr_file cleanup in JSON error path
- Add type validation for PARSED_FILES_CREATED before array iteration
- Check for empty file names in JSON array iteration
- Fix stale test counts in README.md (165 → 276, 8 → 11 test files)
This commit is contained in:
parent
bcc3db9474
commit
1c9059b902
2 changed files with 33 additions and 18 deletions
|
|
@ -32,7 +32,7 @@ Ralph is an implementation of the Geoffrey Huntley's technique for Claude Code t
|
||||||
- tmux integration for live monitoring
|
- tmux integration for live monitoring
|
||||||
- PRD import functionality
|
- PRD import functionality
|
||||||
- **CI/CD pipeline with GitHub Actions**
|
- **CI/CD pipeline with GitHub Actions**
|
||||||
- 165 passing tests across 8 test files
|
- 276 passing tests across 11 test files
|
||||||
|
|
||||||
### Recent Improvements
|
### Recent Improvements
|
||||||
|
|
||||||
|
|
@ -372,7 +372,7 @@ If you want to run the test suite:
|
||||||
# Install BATS testing framework
|
# Install BATS testing framework
|
||||||
npm install -g bats bats-support bats-assert
|
npm install -g bats bats-support bats-assert
|
||||||
|
|
||||||
# Run all tests (165 tests)
|
# Run all tests (276 tests)
|
||||||
npm test
|
npm test
|
||||||
|
|
||||||
# Run specific test suites
|
# Run specific test suites
|
||||||
|
|
@ -389,8 +389,8 @@ bats tests/integration/test_loop_execution.bats
|
||||||
```
|
```
|
||||||
|
|
||||||
Current test status:
|
Current test status:
|
||||||
- **165 tests** across 8 test files
|
- **276 tests** across 11 test files
|
||||||
- **100% pass rate** (165/165 passing)
|
- **100% pass rate** (276/276 passing)
|
||||||
- Comprehensive unit and integration tests
|
- Comprehensive unit and integration tests
|
||||||
- Specialized tests for JSON parsing, CLI flags, circuit breaker, and installation workflows
|
- Specialized tests for JSON parsing, CLI flags, circuit breaker, and installation workflows
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,8 @@ CLAUDE_CODE_CMD="claude"
|
||||||
# Modern CLI Configuration (Phase 1.1)
|
# Modern CLI Configuration (Phase 1.1)
|
||||||
# These flags enable structured JSON output and controlled file operations
|
# These flags enable structured JSON output and controlled file operations
|
||||||
CLAUDE_OUTPUT_FORMAT="json"
|
CLAUDE_OUTPUT_FORMAT="json"
|
||||||
CLAUDE_ALLOWED_TOOLS='"Read" "Write" "Bash(mkdir:*)" "Bash(cp:*)"'
|
# Use bash array for proper quoting of each tool argument
|
||||||
|
declare -a CLAUDE_ALLOWED_TOOLS=('Read' 'Write' 'Bash(mkdir:*)' 'Bash(cp:*)')
|
||||||
CLAUDE_MIN_VERSION="2.0.76" # Minimum version for modern CLI features
|
CLAUDE_MIN_VERSION="2.0.76" # Minimum version for modern CLI features
|
||||||
|
|
||||||
# Temporary file names
|
# Temporary file names
|
||||||
|
|
@ -144,6 +145,14 @@ check_claude_version() {
|
||||||
IFS='.' read -r ver_major ver_minor ver_patch <<< "$version"
|
IFS='.' read -r ver_major ver_minor ver_patch <<< "$version"
|
||||||
IFS='.' read -r min_major min_minor min_patch <<< "$CLAUDE_MIN_VERSION"
|
IFS='.' read -r min_major min_minor min_patch <<< "$CLAUDE_MIN_VERSION"
|
||||||
|
|
||||||
|
# Default empty components to 0 (handles versions like "2.1" without patch)
|
||||||
|
ver_major=${ver_major:-0}
|
||||||
|
ver_minor=${ver_minor:-0}
|
||||||
|
ver_patch=${ver_patch:-0}
|
||||||
|
min_major=${min_major:-0}
|
||||||
|
min_minor=${min_minor:-0}
|
||||||
|
min_patch=${min_patch:-0}
|
||||||
|
|
||||||
# Compare major version
|
# Compare major version
|
||||||
if [[ $ver_major -lt $min_major ]]; then
|
if [[ $ver_major -lt $min_major ]]; then
|
||||||
log "WARN" "Claude Code CLI version $version is below recommended $CLAUDE_MIN_VERSION"
|
log "WARN" "Claude Code CLI version $version is below recommended $CLAUDE_MIN_VERSION"
|
||||||
|
|
@ -362,7 +371,8 @@ PROMPTEOF
|
||||||
if [[ "$use_modern_cli" == "true" ]]; then
|
if [[ "$use_modern_cli" == "true" ]]; then
|
||||||
# Modern CLI invocation with JSON output and controlled tool permissions
|
# Modern CLI invocation with JSON output and controlled tool permissions
|
||||||
# --allowedTools permits file operations without user prompts
|
# --allowedTools permits file operations without user prompts
|
||||||
if $CLAUDE_CODE_CMD --output-format "$CLAUDE_OUTPUT_FORMAT" --allowedTools $CLAUDE_ALLOWED_TOOLS < "$CONVERSION_PROMPT_FILE" > "$CONVERSION_OUTPUT_FILE" 2> "$stderr_file"; then
|
# Array expansion preserves quoting for each tool argument
|
||||||
|
if $CLAUDE_CODE_CMD --output-format "$CLAUDE_OUTPUT_FORMAT" --allowedTools "${CLAUDE_ALLOWED_TOOLS[@]}" < "$CONVERSION_PROMPT_FILE" > "$CONVERSION_OUTPUT_FILE" 2> "$stderr_file"; then
|
||||||
cli_exit_code=0
|
cli_exit_code=0
|
||||||
else
|
else
|
||||||
cli_exit_code=$?
|
cli_exit_code=$?
|
||||||
|
|
@ -402,7 +412,7 @@ PROMPTEOF
|
||||||
if [[ -n "$PARSED_ERROR_CODE" ]]; then
|
if [[ -n "$PARSED_ERROR_CODE" ]]; then
|
||||||
log "ERROR" "Error code: $PARSED_ERROR_CODE"
|
log "ERROR" "Error code: $PARSED_ERROR_CODE"
|
||||||
fi
|
fi
|
||||||
rm -f "$CONVERSION_PROMPT_FILE" "$CONVERSION_OUTPUT_FILE"
|
rm -f "$CONVERSION_PROMPT_FILE" "$CONVERSION_OUTPUT_FILE" "$stderr_file"
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|
@ -444,17 +454,22 @@ PROMPTEOF
|
||||||
|
|
||||||
# If JSON provided files_created, use that to inform verification
|
# If JSON provided files_created, use that to inform verification
|
||||||
if [[ "$json_parsed" == "true" && -n "$PARSED_FILES_CREATED" && "$PARSED_FILES_CREATED" != "[]" ]]; then
|
if [[ "$json_parsed" == "true" && -n "$PARSED_FILES_CREATED" && "$PARSED_FILES_CREATED" != "[]" ]]; then
|
||||||
# Parse JSON array and verify each file exists
|
# Validate that PARSED_FILES_CREATED is a valid JSON array before iteration
|
||||||
local json_files
|
local is_array
|
||||||
json_files=$(echo "$PARSED_FILES_CREATED" | jq -r '.[]' 2>/dev/null)
|
is_array=$(echo "$PARSED_FILES_CREATED" | jq -e 'type == "array"' 2>/dev/null)
|
||||||
if [[ -n "$json_files" ]]; then
|
if [[ "$is_array" == "true" ]]; then
|
||||||
while IFS= read -r file; do
|
# Parse JSON array and verify each file exists
|
||||||
if [[ -f "$file" ]]; then
|
local json_files
|
||||||
created_files+=("$file")
|
json_files=$(echo "$PARSED_FILES_CREATED" | jq -r '.[]' 2>/dev/null)
|
||||||
else
|
if [[ -n "$json_files" ]]; then
|
||||||
missing_files+=("$file")
|
while IFS= read -r file; do
|
||||||
fi
|
if [[ -n "$file" && -f "$file" ]]; then
|
||||||
done <<< "$json_files"
|
created_files+=("$file")
|
||||||
|
elif [[ -n "$file" ]]; then
|
||||||
|
missing_files+=("$file")
|
||||||
|
fi
|
||||||
|
done <<< "$json_files"
|
||||||
|
fi
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue