# Phase 6 Logic Review - Issues Found & Fixes Required

**Date**: February 26, 2026
**Status**: Issues Identified - Action Required
**Severity**: 1 CRITICAL, 3 HIGH, 4 MEDIUM

---

## CRITICAL ISSUES

### 1. P6.14 (Laravel Vendor Size) - Unit Loss Bug
**File**: extended-analysis-functions.sh, Line 1239
**Severity**: 🔴 CRITICAL

**Problem**:
```bash
local vendor_size=$(du -sh "$docroot/vendor" 2>/dev/null | cut -f1 | grep -o "[0-9]*")
```

**Issue**:
- `du -sh` returns "1.2G" or "500M"
- `cut -f1` extracts "1.2G" or "500M"
- `grep -o "[0-9]*"` extracts ONLY digits, losing unit: "12" or "500"
- Comparison `if [ "$vendor_size" -gt 500 ]` fails:
  - "1.2G" → "12" → 12 is NOT > 500 (FALSE NEGATIVE)
  - "500M" → "500" → 500 is NOT > 500 (FALSE NEGATIVE)
  - "100M" → "100" → 100 is NOT > 500 (FALSE NEGATIVE)

**Fix**:
```bash
# Option 1: Extract only the number part correctly
local vendor_size=$(du -sh "$docroot/vendor" 2>/dev/null | awk '{print $1}')
# Then convert to MB or use direct string comparison
if [[ "$vendor_size" =~ ([0-9.]+)([KMG]) ]]; then
    local size_num="${BASH_REMATCH[1]}"
    local size_unit="${BASH_REMATCH[2]}"
    local size_mb=$(case "$size_unit" in
        K) echo "scale=0; $size_num / 1024" | bc ;;
        M) echo "$size_num" | cut -d. -f1 ;;
        G) echo "scale=0; $size_num * 1024" | bc ;;
    esac)
    if [ "$size_mb" -gt 500 ]; then
        # Alert
    fi
fi

# Option 2: Simpler - check if contains G (guaranteed > 500MB)
if du -sh "$docroot/vendor" 2>/dev/null | grep -q "G"; then
    # Alert for > 500MB (any G value is > 500M)
fi
```

**Impact**: Currently NEVER triggers alert for vendor size > 500MB

---

### 2. P6.22 (System Load) - Integer Comparison Bug
**File**: extended-analysis-functions.sh, Line 1348
**Severity**: 🔴 CRITICAL

**Problem**:
```bash
local load_ratio=$(echo "scale=2; $loadavg / $cpu_count" | bc)
if [ "${load_ratio%.*}" -gt 2 ]; then
```

**Issue**:
- `${load_ratio%.*}` strips decimal part: "2.5" → "2", "1.8" → "1", "3.0" → "3"
- Integer comparison: `[ "2" -gt 2 ]` = FALSE (wrong!)
- Should trigger on 2.5x ratio but doesn't
- Only triggers when ratio >= 3.0

**Fix**:
```bash
# Option 1: Use bc for floating point comparison
if (( $(echo "$load_ratio > 2.0" | bc -l) )); then
    # Alert
fi

# Option 2: Compare as integers after multiplying by 10
local load_ratio_int=$(echo "scale=0; $loadavg * 10 / $cpu_count" | bc)
if [ "$load_ratio_int" -gt 20 ]; then
    # Alert (ratio > 2.0)
fi

# Option 3: Simpler - compare directly with bc
if bc <<< "$load_ratio > 2" | grep -q "1"; then
    # Alert
fi
```

**Impact**: Fails to alert when load ratio is between 2.0-3.0 (should alert)

---

### 3. P6.18 (Process Limits) - Off-by-One Error
**File**: extended-analysis-functions.sh, Line 1295
**Severity**: 🔴 CRITICAL

**Problem**:
```bash
local used_processes=$(ps aux | wc -l)
```

**Issue**:
- `ps aux` output includes HEADER line
- Actual count = displayed processes + 1
- If 500 processes running, `ps aux | wc -l` = 501
- Comparison logic is off by 1
- May trigger false alerts

**Fix**:
```bash
# Option 1: Skip header line
local used_processes=$(ps aux | tail -n +2 | wc -l)

# Option 2: Use ps with specific format
local used_processes=$(ps -e | tail -n +2 | wc -l)

# Option 3: Subtract 1 from count
local used_processes=$(($(ps aux | wc -l) - 1))
```

**Impact**: Process limit alerts are off by 1, may miss or falsely trigger

---

## HIGH SEVERITY ISSUES

### 4. P6.17 (I/O Scheduler) - Hardcoded Device
**File**: extended-analysis-functions.sh, Line 1283
**Severity**: 🟠 HIGH

**Problem**:
```bash
local scheduler=$(cat /sys/block/sda/queue/scheduler 2>/dev/null | grep -o "\[.*\]" | tr -d '[]')
```

**Issue**:
- Hardcoded "sda" - fails on systems with:
  - NVMe devices (nvme0n1)
  - Multiple drives
  - Different device names
  - Virtual environments
- If sda doesn't exist, function silently fails
- Should check all block devices

**Fix**:
```bash
# Option 1: Check multiple common devices
for device in sda sdb nvme0n1 vda; do
    if [ -f "/sys/block/$device/queue/scheduler" ]; then
        local scheduler=$(cat "/sys/block/$device/queue/scheduler" | grep -o "\[.*\]" | tr -d '[]')
        if [ "$scheduler" = "deadline" ] || [ "$scheduler" = "cfq" ]; then
            # Alert
            break
        fi
    fi
done

# Option 2: Find all block devices
local schedulers=$(find /sys/block/*/queue/scheduler 2>/dev/null | while read f; do
    grep -o "\[.*\]" "$f" | tr -d '[]'
done | sort -u)
```

**Impact**: May miss I/O scheduler issues on NVMe or multi-disk systems

---

### 5. P6.19 (Swap I/O) - vmstat Column Uncertainty
**File**: extended-analysis-functions.sh, Line 1309
**Severity**: 🟠 HIGH

**Problem**:
```bash
local swap_io=$(vmstat 1 3 | tail -1 | awk '{print $7}')  # si column
if [ "$swap_io" -gt 100 ]; then
```

**Issue**:
- vmstat column 7 should be "si" (swap in pages/sec)
- But `print $7` gets 7th field, which depends on:
  - vmstat version
  - System configuration
  - Whether procs section is included
- Comment says "si column" but doesn't verify
- "100" is compared but units are pages/sec, not MB/s
- Description claims "MB/s" but vmstat shows pages/sec

**Fix**:
```bash
# Option 1: Use named columns
local swap_io=$(vmstat -S m 1 2 | tail -1 | awk '{print $7}')
# But still verify column position

# Option 2: Parse column headers
local si_col=$(vmstat 1 1 | head -1 | tr -s ' ' | cut -d' ' -f7)
if [ "$si_col" != "si" ]; then
    # Column position differs, need to recalculate
    si_col=$(vmstat 1 1 | head -1 | tr -s ' ' | grep -o "si" | head -1)
fi

# Option 3: More robust - extract from full output
local swap_data=$(vmstat 1 2 | tail -1)
# Parse more carefully with field validation

# Option 4: Use -S flag for MB output
vmstat -S M 1 2 | tail -1 | awk '{if ($7 > 10) print "Alert"}'
```

**Impact**: May alert on normal conditions or miss severe swap issues (column mismatch)

---

### 6. P6.13 (Laravel Cache Driver) - Multiple Line Handling
**File**: extended-analysis-functions.sh, Line 1221
**Severity**: 🟠 HIGH

**Problem**:
```bash
local cache_driver=$(grep "CACHE_DRIVER=" "$docroot/.env" | cut -d= -f2)
```

**Issue**:
- If .env has multiple CACHE_DRIVER lines (unlikely but possible):
  - `grep` returns all matches
  - `cut` processes each line
  - Variable gets ALL values concatenated
  - Comparison `[ "$cache_driver" = "file" ]` may fail
- Whitespace not handled: "CACHE_DRIVER = redis" → " redis" (with leading space)

**Fix**:
```bash
# Option 1: Get first match, trim whitespace
local cache_driver=$(grep -m 1 "CACHE_DRIVER=" "$docroot/.env" 2>/dev/null | cut -d= -f2 | xargs)

# Option 2: More robust parsing
local cache_driver=$(grep -m 1 "^CACHE_DRIVER=" "$docroot/.env" 2>/dev/null | cut -d= -f2- | tr -d ' "\'')

# Option 3: With default value
local cache_driver=$(grep -m 1 "CACHE_DRIVER=" "$docroot/.env" 2>/dev/null | cut -d= -f2 | xargs || echo "file")
```

**Impact**: Whitespace in .env could cause false negatives

---

## MEDIUM SEVERITY ISSUES

### 7. P6.10 (Magento Extensions) - Count Off-by-One
**File**: extended-analysis-functions.sh, Line 1167
**Severity**: 🟡 MEDIUM

**Problem**:
```bash
local ext_count=$(find "$docroot/app/code" -maxdepth 2 -type d 2>/dev/null | wc -l)

if [ "$ext_count" -gt 50 ]; then
```

**Issue**:
- `find` includes the root directory "app/code" itself
- If there are 49 vendor/module combos, count = 50
- Threshold of 50 would NOT trigger
- If there are 50 vendor/module combos, count = 51
- Threshold of 50 WOULD trigger (off by one)

**Fix**:
```bash
# Option 1: Exclude root directory
local ext_count=$(find "$docroot/app/code" -maxdepth 2 -mindepth 1 -type d 2>/dev/null | wc -l)

# Option 2: Count only vendor directories
local ext_count=$(ls -d "$docroot/app/code"/*/ 2>/dev/null | wc -l)

# Option 3: Subtract 1
local ext_count=$(($(find "$docroot/app/code" -maxdepth 2 -type d 2>/dev/null | wc -l) - 1))
```

**Impact**: Alert threshold is off by 1 (may miss or falsely alert)

---

### 8. P6.15 (Custom Framework) - Arbitrary Threshold
**File**: extended-analysis-functions.sh, Line 1260
**Severity**: 🟡 MEDIUM

**Problem**:
```bash
if [ "$config_files" -gt 20 ]; then
```

**Issue**:
- Threshold of 20 seems arbitrary
- Many frameworks naturally have 20+ config files:
  - WordPress has wp-config.php
  - Laravel has config/*.php (5+ files)
  - Symfony has config/* (multiple files)
- This will trigger false positives on normal setups
- No real performance impact from having many config files

**Fix**:
```bash
# Option 1: Increase threshold to something more realistic
if [ "$config_files" -gt 50 ]; then
    # Alert only for extremely bloated configs
fi

# Option 2: Look for specific indicators instead
if find "$docroot" -maxdepth 3 -name "config_*.php" -type f 2>/dev/null | grep -q .; then
    # Alert for duplicate/redundant config patterns
fi

# Option 3: Remove this check as false positive
# Custom framework detection is too vague
```

**Impact**: False positive alerts on normal framework configurations

---

### 9. P6.1 (Drupal Module Count) - Database Dependency
**File**: extended-analysis-functions.sh, Line 1005
**Severity**: 🟡 MEDIUM

**Problem**:
```bash
local module_count=$(echo "SELECT COUNT(*) FROM system WHERE type='module' AND status=1;" | mysql_query_safe 2>/dev/null | tail -1 || echo 0)
```

**Issue**:
- Assumes `mysql_query_safe` function exists and is sourced
- If database not connected, silently returns 0
- If Drupal database table doesn't exist, silently returns 0
- No error indication that database check failed
- Should verify database connection first

**Fix**:
```bash
# Option 1: Check if function exists first
if ! declare -f mysql_query_safe &>/dev/null; then
    return 0
fi

local module_count=$(echo "SELECT COUNT(*) FROM system WHERE type='module' AND status=1;" | mysql_query_safe 2>&1)
if [ $? -ne 0 ] || [ -z "$module_count" ]; then
    # Database query failed
    return 0
fi

# Option 2: Get only numeric result
local module_count=$(echo "SELECT COUNT(*) FROM system WHERE type='module' AND status=1;" | mysql_query_safe 2>/dev/null | tail -1 | grep -o "[0-9]*" || echo 0)
```

**Impact**: May fail silently, producing unreliable results

---

### 10. P6.2 (Drupal Cache Config) - Case Sensitivity
**File**: extended-analysis-functions.sh, Line 1023-1024
**Severity**: 🟡 MEDIUM

**Problem**:
```bash
local has_redis=$(grep -c "redis" "$docroot/settings.php" 2>/dev/null || echo 0)
```

**Issue**:
- Case-sensitive grep
- Drupal settings might have "Redis" with capital R
- Would miss configuration if capitalized differently
- Should use case-insensitive grep

**Fix**:
```bash
local has_redis=$(grep -ci "redis" "$docroot/settings.php" 2>/dev/null || echo 0)
local has_memcache=$(grep -ci "memcache" "$docroot/settings.php" 2>/dev/null || echo 0)
```

**Impact**: May miss correctly configured Redis/Memcache backends (case sensitivity)

---

## SUMMARY TABLE

| ID | Function | Severity | Issue | Impact |
|----|----------|----------|-------|--------|
| 1 | P6.14 (Laravel Vendor) | 🔴 CRITICAL | Unit loss in size calculation | NEVER alerts |
| 2 | P6.22 (Load Average) | 🔴 CRITICAL | Integer comparison strips decimals | Misses 2.0-3.0 ratio |
| 3 | P6.18 (Process Limits) | 🔴 CRITICAL | Header line off-by-one | Threshold off by 1 |
| 4 | P6.17 (I/O Scheduler) | 🟠 HIGH | Hardcoded device | Fails on NVMe/multi-disk |
| 5 | P6.19 (Swap I/O) | 🟠 HIGH | vmstat column uncertainty | Column mismatch possible |
| 6 | P6.13 (Cache Driver) | 🟠 HIGH | Whitespace not trimmed | False negatives |
| 7 | P6.10 (Magento Extensions) | 🟡 MEDIUM | Count includes root dir | Off-by-one threshold |
| 8 | P6.15 (Custom Framework) | 🟡 MEDIUM | Arbitrary threshold | False positives |
| 9 | P6.1 (Drupal Modules) | 🟡 MEDIUM | No error handling | Silent failures |
| 10 | P6.2 (Drupal Cache) | 🟡 MEDIUM | Case-sensitive grep | Misses variations |

---

## ACTION REQUIRED

### Immediate (Block Deployment)
1. ✋ Fix P6.14 - Laravel vendor size detection broken
2. ✋ Fix P6.22 - Load average comparison broken
3. ✋ Fix P6.18 - Process count is off by 1

### Before Deployment
4. 🔧 Fix P6.17 - Hardcoded device (add NVMe support)
5. 🔧 Fix P6.19 - vmstat column validation
6. 🔧 Fix P6.13 - Whitespace trimming
7. 🔧 Fix P6.10 - Off-by-one counter

### Strongly Recommended
8. 🔧 Fix P6.15 - Reduce false positive threshold or remove
9. 🔧 Fix P6.1 - Add database connection validation
10. 🔧 Fix P6.2 - Use case-insensitive grep

---

## RECOMMENDATION

**Current Status**: Phase 6 is **NOT PRODUCTION READY** due to 3 critical bugs that prevent core functionality from working correctly.

**Required Actions**:
1. Fix all 3 CRITICAL issues immediately
2. Fix all 3 HIGH severity issues before deployment
3. Address MEDIUM issues for robustness

**Estimated Fix Time**: 1-2 hours for all issues

---

**Generated**: February 26, 2026
**Reviewer**: Logic Verification Pass
**Status**: Issues Identified - Code Review Needed