From cafaacdc0c34775ac029dbe30aeea287307ac4da Mon Sep 17 00:00:00 2001 From: Test User Date: Sat, 10 Jan 2026 10:48:19 -0700 Subject: [PATCH] fix(session): address code review feedback for session management - Fix SC2155: separate declare from assign in get_session_id(), log_session_transition(), init_session_tracking() - Use jq for safe JSON generation in reset_session() and init_session_tracking() instead of heredocs (prevents special character issues) - Add corruption tolerance to log_session_transition() with JSON validation and fallback to empty array on parse failures - Add generate_session_id() to create unique session IDs (ralph--) - Add update_session_last_used() helper called on each loop iteration - init_session_tracking() now generates unique session_id and sets last_used - Add session files to .gitignore (.ralph_session, .ralph_session_history, .claude_session_id) All 265 tests pass. --- .gitignore | 3 + ralph_loop.sh | 174 +++++++++++++++++++++++++++++++++++++------------- 2 files changed, 133 insertions(+), 44 deletions(-) diff --git a/.gitignore b/.gitignore index faf2e5c..cb3ffae 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,9 @@ .last_reset .exit_signals status.json +.ralph_session +.ralph_session_history +.claude_session_id # Logs logs/* diff --git a/ralph_loop.sh b/ralph_loop.sh index df7632a..fdedb66 100755 --- a/ralph_loop.sh +++ b/ralph_loop.sh @@ -494,9 +494,13 @@ get_session_id() { return 0 fi - # Extract session_id from JSON file - local session_id=$(jq -r '.session_id // ""' "$RALPH_SESSION_FILE" 2>/dev/null) - if [[ "$session_id" == "null" ]]; then + # Extract session_id from JSON file (SC2155: separate declare from assign) + local session_id + session_id=$(jq -r '.session_id // ""' "$RALPH_SESSION_FILE" 2>/dev/null) + local jq_status=$? + + # Handle jq failure or null/empty results + if [[ $jq_status -ne 0 || -z "$session_id" || "$session_id" == "null" ]]; then session_id="" fi echo "$session_id" @@ -508,22 +512,30 @@ get_session_id() { reset_session() { local reason=${1:-"manual_reset"} - # Always create/overwrite the session file to ensure consistent state - cat > "$RALPH_SESSION_FILE" << EOF -{ - "session_id": "", - "created_at": "", - "last_used": "", - "reset_at": "$(get_iso_timestamp)", - "reset_reason": "$reason" -} -EOF + # Get current timestamp + local reset_timestamp + reset_timestamp=$(get_iso_timestamp) + + # Always create/overwrite the session file using jq for safe JSON escaping + jq -n \ + --arg session_id "" \ + --arg created_at "" \ + --arg last_used "" \ + --arg reset_at "$reset_timestamp" \ + --arg reset_reason "$reason" \ + '{ + session_id: $session_id, + created_at: $created_at, + last_used: $last_used, + reset_at: $reset_at, + reset_reason: $reset_reason + }' > "$RALPH_SESSION_FILE" # Also clear the Claude session file for consistency rm -f "$CLAUDE_SESSION_FILE" 2>/dev/null - # Log the session transition - log_session_transition "active" "reset" "$reason" "${loop_count:-0}" + # Log the session transition (non-fatal to prevent script exit under set -e) + log_session_transition "active" "reset" "$reason" "${loop_count:-0}" || true log_status "INFO" "Session reset: $reason" } @@ -536,15 +548,14 @@ log_session_transition() { local reason=$3 local loop_number=${4:-0} - # Initialize history file if needed - if [[ ! -f "$RALPH_SESSION_HISTORY_FILE" ]]; then - echo '[]' > "$RALPH_SESSION_HISTORY_FILE" - fi + # Get timestamp once (SC2155: separate declare from assign) + local ts + ts=$(get_iso_timestamp) - # Create transition entry + # Create transition entry using jq for safe JSON (SC2155: separate declare from assign) local transition - transition=$(jq -n \ - --arg timestamp "$(get_iso_timestamp)" \ + transition=$(jq -n -c \ + --arg timestamp "$ts" \ --arg from_state "$from_state" \ --arg to_state "$to_state" \ --arg reason "$reason" \ @@ -557,40 +568,107 @@ log_session_transition() { loop_number: $loop_number }') - # Append to history and keep only last 50 entries - local history=$(cat "$RALPH_SESSION_HISTORY_FILE" 2>/dev/null || echo '[]') - history=$(echo "$history" | jq ". += [$transition] | .[-50:]") - echo "$history" > "$RALPH_SESSION_HISTORY_FILE" + # Read history file defensively - fallback to empty array on any failure + local history + if [[ -f "$RALPH_SESSION_HISTORY_FILE" ]]; then + history=$(cat "$RALPH_SESSION_HISTORY_FILE" 2>/dev/null) + # Validate JSON, fallback to empty array if corrupted + if ! echo "$history" | jq empty 2>/dev/null; then + history='[]' + fi + else + history='[]' + fi + + # Append transition and keep only last 50 entries + local updated_history + updated_history=$(echo "$history" | jq ". += [$transition] | .[-50:]" 2>/dev/null) + local jq_status=$? + + # Only write if jq succeeded + if [[ $jq_status -eq 0 && -n "$updated_history" ]]; then + echo "$updated_history" > "$RALPH_SESSION_HISTORY_FILE" + else + # Fallback: start fresh with just this transition + echo "[$transition]" > "$RALPH_SESSION_HISTORY_FILE" + fi +} + +# Generate a unique session ID using timestamp and random component +generate_session_id() { + local ts + ts=$(date +%s) + local rand + rand=$RANDOM + echo "ralph-${ts}-${rand}" } # Initialize session tracking (called at loop start) init_session_tracking() { + local ts + ts=$(get_iso_timestamp) + # Create session file if it doesn't exist if [[ ! -f "$RALPH_SESSION_FILE" ]]; then - cat > "$RALPH_SESSION_FILE" << EOF -{ - "session_id": "", - "created_at": "$(get_iso_timestamp)", - "last_used": "", - "reset_at": "", - "reset_reason": "" -} -EOF - log_status "INFO" "Initialized session tracking" + local new_session_id + new_session_id=$(generate_session_id) + + jq -n \ + --arg session_id "$new_session_id" \ + --arg created_at "$ts" \ + --arg last_used "$ts" \ + --arg reset_at "" \ + --arg reset_reason "" \ + '{ + session_id: $session_id, + created_at: $created_at, + last_used: $last_used, + reset_at: $reset_at, + reset_reason: $reset_reason + }' > "$RALPH_SESSION_FILE" + + log_status "INFO" "Initialized session tracking (session: $new_session_id)" + return 0 fi # Validate existing session file if ! jq empty "$RALPH_SESSION_FILE" 2>/dev/null; then log_status "WARN" "Corrupted session file detected, recreating..." - cat > "$RALPH_SESSION_FILE" << EOF -{ - "session_id": "", - "created_at": "$(get_iso_timestamp)", - "last_used": "", - "reset_at": "", - "reset_reason": "corrupted_file_recovery" + local new_session_id + new_session_id=$(generate_session_id) + + jq -n \ + --arg session_id "$new_session_id" \ + --arg created_at "$ts" \ + --arg last_used "$ts" \ + --arg reset_at "$ts" \ + --arg reset_reason "corrupted_file_recovery" \ + '{ + session_id: $session_id, + created_at: $created_at, + last_used: $last_used, + reset_at: $reset_at, + reset_reason: $reset_reason + }' > "$RALPH_SESSION_FILE" + fi } -EOF + +# Update last_used timestamp in session file (called on each loop iteration) +update_session_last_used() { + if [[ ! -f "$RALPH_SESSION_FILE" ]]; then + return 0 + fi + + local ts + ts=$(get_iso_timestamp) + + # Update last_used in existing session file + local updated + updated=$(jq --arg last_used "$ts" '.last_used = $last_used' "$RALPH_SESSION_FILE" 2>/dev/null) + local jq_status=$? + + if [[ $jq_status -eq 0 && -n "$updated" ]]; then + echo "$updated" > "$RALPH_SESSION_FILE" fi } @@ -899,6 +977,10 @@ main() { while true; do loop_count=$((loop_count + 1)) log_status "INFO" "DEBUG: Successfully incremented loop_count to $loop_count" + + # Update session last_used timestamp + update_session_last_used + log_status "INFO" "Loop #$loop_count - calling init_call_tracking..." init_call_tracking @@ -1031,6 +1113,10 @@ Files created: - $LOG_DIR/: All execution logs - $DOCS_DIR/: Generated documentation - $STATUS_FILE: Current status (JSON) + - .ralph_session: Session lifecycle tracking + - .ralph_session_history: Session transition history (last 50) + - .call_count: API call counter for rate limiting + - .last_reset: Timestamp of last rate limit reset Example workflow: ralph-setup my-project # Create project