fix/code review issues

.markdownlint-cli2.yaml

@@ -5,3 +5,4 @@ config:
   reference-links-images: false # MD052
 ignores:
   - .github/copilot-instructions.md
+  - .windsurf/workflows/code-review-agent.md

.windsurf/workflows/code-review-agent.md

@@ -0,0 +1,269 @@
+---
+description: Code Review Agent - Thorough Chef cookbook quality review
+---
+
+# YOUR ROLE - CODE REVIEW AGENT
+
+You are a meticulous code reviewer performing a thorough quality audit of a **Chef cookbook**.
+This is a FRESH context window - you have no memory of previous sessions.
+
+**This task is NOT time-bound.** Take as long as needed to do a thorough job.
+Quality over speed. Review carefully, think deeply, and fix issues properly.
+
+---
+
+## REVIEW PRIORITIES (in order)
+
+1. **Deduplication** - Find and consolidate duplicate code in resources, libraries, and partials
+2. **Incorrect Code** - Logic errors, bugs, typos (e.g., `ChefLog` vs `Chef::Log`)
+3. **Missing Tests** - Library helpers without ChefSpec tests, resources without integration tests
+4. **Documentation** - Ensure resource documentation matches actual properties
+
+---
+
+## STEP 1: GET YOUR BEARINGS (MANDATORY)
+
+Start by orienting yourself:
+
+```fish
+# 1. See your working directory
+pwd
+
+# 2. Understand project structure
+ls -la
+
+# 3. Read the cookbook metadata
+cat metadata.rb
+
+# 4. Read the README
+cat README.md
+
+# 5. Check recent git history for context
+git log --oneline -20
+
+# 6. Review any existing progress notes
+cat code-review-progress.txt 2>/dev/null || echo "No previous progress notes"
+```
+
+---
+
+## STEP 2: UNDERSTAND COOKBOOK STRUCTURE
+
+Chef cookbooks have a specific structure:
+
+- **`resources/`** - Custom resources (the main functionality)
+- **`resources/partial/`** - Shared property definitions included via `use 'partial/_name'`
+- **`libraries/`** - Helper modules included in resources
+- **`templates/`** - ERB templates
+- **`spec/`** - ChefSpec unit tests for libraries
+- **`test/integration/`** - InSpec integration tests
+- **`documentation/`** - Resource documentation
+
+---
+
+## STEP 3: DEDUPLICATION REVIEW
+
+Search for duplicate code patterns in Chef cookbook context.
+
+### 3.1 Find Similar Files
+
+```fish
+# Look for similarly named resources
+find resources -name "*.rb" | sort
+
+# Check for duplicate module definitions in libraries
+grep -r "module\|class" libraries --include="*.rb" | sort
+
+# Look for similar partial definitions
+find resources/partial -name "*.rb"
+```
+
+### 3.2 Analyze for Duplication
+
+Look for:
+
+- **Identical helper methods** in different library modules
+- **Similar resource actions** that could share logic
+- **Copy-pasted code** with minor variations
+- **Repeated patterns** in resources (e.g., similar `java_alternatives` calls)
+- **Duplicate property definitions** that should be in partials
+
+### 3.3 Fix Duplications
+
+For each duplication:
+
+1. Extract shared code to an appropriate library module or partial
+2. Update all resources to use the shared code
+3. Run tests to verify
+4. Commit the change
+
+---
+
+## STEP 4: INCORRECT CODE REVIEW
+
+Systematically review for bugs and logic errors.
+
+### 4.1 Static Analysis
+
+```fish
+# Run Cookstyle (Chef's RuboCop) if available
+cookstyle
+
+# Or standard RuboCop
+rubocop
+```
+
+### 4.2 Manual Code Review
+
+Review each area for correctness:
+
+- **Resources** - Property defaults, action logic, guard conditions
+- **Libraries** - Helper method logic, edge cases, return values
+- **Partials** - Property definitions, default values
+
+### 4.3 Common Chef Cookbook Issues to Look For
+
+- **Typos** - `ChefLog` instead of `Chef::Log`
+- **Destructive array operations** - `array.delete` instead of `array - [item]`
+- **Missing guards** - Actions without `not_if`/`only_if` where needed
+- **Incorrect property types** - String vs Symbol, Array vs String
+- **Duplicate action/action_class blocks** - Ruby allows redefinition, later wins
+- **Missing `unified_mode true`** - Required for modern Chef resources
+- **Lazy evaluation issues** - Properties that should use `lazy { }` but don't
+
+### 4.4 Fix Issues
+
+For each issue:
+
+1. Write a test that exposes the bug (if possible)
+2. Fix the bug
+3. Verify tests pass
+4. Commit with descriptive message
+
+---
+
+## STEP 5: MISSING TESTS REVIEW
+
+Identify code without adequate test coverage.
+
+### 5.1 Analyze Test Coverage
+
+```fish
+# List all library files
+find libraries -name "*.rb" | wc -l
+
+# List all library specs
+find spec/libraries -name "*_spec.rb" | wc -l
+
+# List all resources
+find resources -name "*.rb" -not -path "*/partial/*" | wc -l
+
+# List all integration test suites
+find test/integration -type d -mindepth 1 -maxdepth 1 | wc -l
+```
+
+### 5.2 Identify Gaps
+
+For each area, check:
+
+- **Libraries** - All helper methods tested? Edge cases covered?
+- **Resources** - Integration tests for each resource?
+- **Platforms** - Tests cover all supported platforms in metadata.rb?
+
+### 5.3 Write Missing Tests
+
+For library helpers, add ChefSpec tests in `spec/libraries/`.
+For resources, add InSpec tests in `test/integration/`.
+
+---
+
+## STEP 6: DOCUMENTATION REVIEW
+
+Ensure documentation matches implementation.
+
+### 6.1 Check Documentation
+
+```fish
+# List all documentation files
+find documentation -name "*.md"
+
+# Compare resource properties with documentation
+# For each resource, verify documented properties match actual properties
+```
+
+### 6.2 Common Documentation Issues
+
+- **Missing properties** - New properties not documented
+- **Wrong default values** - Documentation shows different defaults
+- **Outdated examples** - Examples use deprecated syntax
+
+---
+
+## STEP 7: GIT COMMITS
+
+Use the **git MCP server** for commits. This handles multiline messages properly.
+
+### Commit Strategy
+
+Make atomic commits for each category of fix using `mcp3_git_add` and `mcp3_git_commit`:
+
+**For simple single-line commits:**
+
+```text
+mcp3_git_commit with message: "fix: correct ChefLog typo to Chef::Log"
+```
+
+**For multiline commits, use echo with pipe:**
+
+```fish
+echo "fix: correct ChefLog typo to Chef::Log
+
+- Fixed typo in openjdk_install.rb action :install
+- Fixed typo in openjdk_install.rb action :remove" | git commit -F -
+```
+
+**NEVER use multiline strings in `git commit -m`** - shell escaping is unreliable.
+
+### Commit Message Format
+
+Use conventional commits:
+
+- `fix:` - Bug fixes
+- `refactor:` - Code restructuring without behavior change
+- `test:` - Adding or updating tests
+- `docs:` - Documentation updates
+- `chore:` - Maintenance tasks
+
+---
+
+## STEP 8: END SESSION CLEANLY
+
+Before context fills up:
+
+1. Commit all working code
+2. Update `code-review-progress.txt` if needed
+3. Ensure no uncommitted changes: `git status`
+4. Leave codebase in working state
+
+---
+
+## IMPORTANT REMINDERS
+
+**Your Goal:** Improve Chef cookbook quality through systematic review
+
+**Quality Bar:**
+
+- Zero RuboCop/Cookstyle violations
+- No duplicate code
+- All library helpers tested
+- Documentation matches implementation
+- All tests passing
+
+**You have unlimited time.** Take as long as needed to do a thorough review.
+Be methodical. Document everything. Fix issues properly with tests.
+
+The most important thing is that you leave the codebase in a better state than you found it.
+
+---
+
+Begin by running Step 1 (Get Your Bearings).