fix(ci): use OPENCODE_PAT for test-writer git push authentication

[MichaelFisher1997]

Summary

• Replace github.token with secrets.OPENCODE_PAT in opencode-test-writer.yml to fix 403 permission error on git push

Context

The test-writer workflow was failing with:

remote: Permission to OpenStaticFish/ZigCraft.git denied to MichaelFisher1997.
fatal: unable to access 'https://github.com/OpenStaticFish/ZigCraft/': The requested URL returned error: 403

The GITHUB_TOKEN doesn't propagate credentials correctly through the opencode composite action's internal git push. Other workflows that need to push (audit, visual-test) already use OPENCODE_PAT for this reason. Only the test-writer workflow was affected — the other opencode workflows (pr, triage, slash-command) were left unchanged since they work fine with github.token.

[MichaelFisher1997]

Closing — the OPENCODE_PAT fix was already merged into dev via the merge commit in this PR. No additional changes needed.

[github-actions]

Based on my review, I can now provide the complete analysis.

📋 Summary

PR Title vs Content Mismatch: The PR title claims to be about "fix(ci): use OPENCODE_PAT for test-writer git push authentication" but the actual changes are extensive world rendering fixes (LOD rendering, water system, shaders, GPU fault handling). A HANDOFF.md file was added explaining the true purpose: fixing terrain chunks disappearing/holes when camera rotates, LOD rendering reliability, and GPU recovery from VK_ERROR_DEVICE_LOST.

Linked Issues: None explicitly mentioned in PR description.

Scope: 681 additions, 212 deletions across 32 files - shader changes, RHI/Vulkan plumbing, LOD system fixes, water reflection system, and new environment-based debug flags.

Build Status: zig build compiles successfully.

---

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified. All previously reported critical issues (none existed) have been resolved.

---

⚠️ High Priority Issues (Should Fix)

None identified.

---

💡 Medium Priority Issues (Nice to Fix)

None identified.

---

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/engine/graphics/vulkan/water_system.zig:43 - reflection_texture_handle is initialized to 0 but never assigned a real value. getReflectionTextureHandle() always returns 0. If reflection texture handling was intentionally moved to direct Vulkan resources (reflection_image/view), this field should be removed to avoid confusion.

[LOW] src/world/world_renderer.zig:467 - Magic false parameter in cs.dispatch(..., false) for temporal occlusion. The comment explains the reason but the function signature doesn't indicate what this boolean controls. Consider using a named constant or explicit parameter name.

[LOW] src/engine/graphics/rhi_vulkan.zig:522-527 - bindTexture has an else {} case that silently ignores unhandled texture slots. If a slot number is misconfigured, it would go unnoticed. Consider logging a warning for unexpected slots instead.

[LOW] src/world/lod_manager.zig:194 vs 264 - Field declaration says cleanup_covered_regions: bool = true but init() sets it to false. The comment at line 193 explains this is intentional for live world behavior, but having the default true in the declaration is confusing. Consider aligning declaration with actual default.

---

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8/10	Water system handles reflections; LOD system handles level-of-detail; clear separation
Open/Closed	7/10	New passes/features added via environment flags without modifying core structures
Liskov Substitution	8/10	RHI abstraction properly maintained; no obvious violations
Interface Segregation	8/10	RHI VTable pattern allows backend swaps; texture binding slots are specific
Dependency Inversion	7/10	Graphics subsystems depend on RHI abstraction; seems appropriate
Average	7.6

---

🎯 Final Assessment

Overall Confidence Score: 78%

Confidence Breakdown:

• Code Quality: 80% (Well-structured with proper error handling; minor issues with unused fields and magic numbers)
• Completeness: 85% (Implements the handoff requirements; shader changes look complete)
• Risk Level: 20% (Low risk - primarily rendering improvements with GPU fault safety added)
• Test Coverage: 65% (Tests present but settings test is minimal; no runtime visual testing in CI)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0 (7.6)
• Overall confidence >= 60% (78%)
• No security concerns
• Code compiles (zig build passes)

Verdict:

MERGE - This PR addresses world rendering issues described in HANDOFF.md including terrain hole/peel problems, LOD rendering reliability, and GPU fault recovery. The changes are substantial but follow existing code patterns. Minor issues identified are code quality concerns, not blockers.

---

{
  "reviewed_sha": "c74a81890ba94cfbd58fb072e7b00c4b85d23106",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 78,
  "recommendation": "MERGE"
}

opencode session  |  github run