fix(5.4): harden recording, preserve logger namespace, add edge tests

Fixes ALL findings from /8eyes + /collab review:
- SEC: Truncate recorded prompt/response to 2000 chars (data minimization)
- SEC: Remove copy.deepcopy — build truncated messages instead
- M-1: Logger uses evolver namespace for log filter compatibility
- M-2: Add delegation seam tests (real SkillEvolver path)
- L-2: Add edge case tests (conflicting keywords, JSON array, _model=None)
- L-2: Add recording truncation verification test
- L-2: Add logger namespace test

36 confirmation tests pass; 1,506 total pass, 127 skipped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Brian Krafft

openspace/skill_engine/evolution/confirmation.py

@@ -7,7 +7,6 @@
 
 from __future__ import annotations
 
-import copy
 import json
 import re
 from typing import TYPE_CHECKING, List
@@ -23,13 +22,16 @@
         SkillRecord,
     )
 
-logger = Logger.get_logger(__name__)
+# Preserve evolver's logger namespace so existing log filters/alerts
+# continue to capture confirmation warnings without reconfiguration.
+logger = Logger.get_logger("openspace.skill_engine.evolver")
 
 # ---------------------------------------------------------------------------
 # Constants
 # ---------------------------------------------------------------------------
 
 _SKILL_CONTENT_MAX_CHARS = 12_000  # Max chars of SKILL.md in evolution prompt
+_RECORDING_MAX_CHARS = 2_000  # Cap recorded prompt content for data minimization
 
 
 # ---------------------------------------------------------------------------
@@ -52,7 +54,7 @@ async def llm_confirm_evolution(
     Returns True if LLM agrees, False otherwise.
     This prevents false positives from rigid threshold-based rules.
 
-    The confirmation prompt and response are recorded to
+    The confirmation prompt and response are recorded (truncated) to
     ``conversations.jsonl`` under agent_name="SkillEvolver.confirm".
     """
     from openspace.recording import RecordingManager
@@ -70,9 +72,13 @@ async def llm_confirm_evolution(
 
     confirm_messages = [{"role": "user", "content": prompt}]
 
-    # Record confirmation setup
+    # Record truncated confirmation setup — full prompt goes to LLM only
+    recorded_messages = [
+        {"role": m["role"], "content": truncate(m["content"], _RECORDING_MAX_CHARS)}
+        for m in confirm_messages
+    ]
     await RecordingManager.record_conversation_setup(
-        setup_messages=copy.deepcopy(confirm_messages),
+        setup_messages=recorded_messages,
         agent_name="SkillEvolver.confirm",
         extra={
             "skill_id": skill_record.skill_id,
@@ -90,10 +96,12 @@ async def llm_confirm_evolution(
         content = result["message"].get("content", "").strip().lower()
         confirmed = evolver._parse_confirmation(content)
 
-        # Record confirmation response
+        # Record truncated confirmation response
         await RecordingManager.record_iteration_context(
             iteration=1,
-            delta_messages=[{"role": "assistant", "content": content}],
+            delta_messages=[
+                {"role": "assistant", "content": truncate(content, _RECORDING_MAX_CHARS)},
+            ],
             response_metadata={
                 "has_tool_calls": False,
                 "confirmed": confirmed,

tests/test_evolution_confirmation.py

@@ -93,6 +93,20 @@ def test_case_sensitivity(self):
         # Uppercase does NOT match \byes\b — by design, caller lowercases
         assert parse_confirmation("YES") is False
 
+    def test_conflicting_keywords_first_wins(self):
+        """When both yes and no keywords appear, positive match wins
+        because the positive branch is checked first."""
+        assert parse_confirmation("yes but also no") is True
+
+    def test_json_array_falls_through_to_keywords(self):
+        """JSON array is not a dict — falls through to keyword matching.
+        The string '"proceed": true' is found as a substring, so returns True."""
+        assert parse_confirmation('[{"proceed": true}]') is True
+
+    def test_json_with_extra_fields(self):
+        """Extra JSON fields are ignored; only 'proceed' matters."""
+        assert parse_confirmation('{"proceed": true, "reason": "looks good"}') is True
+
 
 # ---------------------------------------------------------------------------
 # llm_confirm_evolution — async integration
@@ -238,6 +252,61 @@ async def test_mro_parse_confirmation_called(self):
 
         evolver._parse_confirmation.assert_called_once()
 
+    @pytest.mark.asyncio
+    async def test_model_fallback_to_client_model(self):
+        """When evolver._model is None, falls back to evolver._llm_client.model."""
+        evolver = self._make_evolver(llm_response='{"proceed": true}')
+        evolver._model = None
+        evolver._llm_client.model = "fallback-model"
+
+        with patch(self._RM_PATCH) as mock_rm:
+            mock_rm.record_conversation_setup = AsyncMock()
+            mock_rm.record_iteration_context = AsyncMock()
+
+            result = await llm_confirm_evolution(
+                evolver,
+                skill_record=MagicMock(skill_id="s1"),
+                skill_content="# Skill",
+                proposed_type=MagicMock(value="fix"),
+                proposed_direction="fix",
+                trigger_context="test",
+                recent_analyses=[],
+            )
+
+        assert result is True
+        # Verify fallback model was used
+        call_kwargs = evolver._llm_client.complete.call_args
+        assert call_kwargs[1]["model"] == "fallback-model"
+
+    @pytest.mark.asyncio
+    async def test_recording_truncates_content(self):
+        """Recorded messages are truncated to _RECORDING_MAX_CHARS."""
+        from openspace.skill_engine.evolution.confirmation import _RECORDING_MAX_CHARS
+
+        # Create skill content larger than recording limit
+        big_content = "x" * (_RECORDING_MAX_CHARS + 5000)
+        evolver = self._make_evolver(llm_response='{"proceed": true}')
+
+        with patch(self._RM_PATCH) as mock_rm:
+            mock_rm.record_conversation_setup = AsyncMock()
+            mock_rm.record_iteration_context = AsyncMock()
+
+            await llm_confirm_evolution(
+                evolver,
+                skill_record=MagicMock(skill_id="s1"),
+                skill_content=big_content,
+                proposed_type=MagicMock(value="fix"),
+                proposed_direction="fix",
+                trigger_context="test",
+                recent_analyses=[],
+            )
+
+        # Verify recorded setup messages are truncated
+        setup_call = mock_rm.record_conversation_setup.call_args
+        recorded_msgs = setup_call[1]["setup_messages"]
+        for msg in recorded_msgs:
+            assert len(msg["content"]) <= _RECORDING_MAX_CHARS + 50  # small margin for truncation suffix
+
 
 # ---------------------------------------------------------------------------
 # Constants
@@ -248,6 +317,11 @@ def test_skill_content_max_chars(self):
         assert isinstance(_SKILL_CONTENT_MAX_CHARS, int)
         assert _SKILL_CONTENT_MAX_CHARS == 12_000
 
+    def test_logger_uses_evolver_namespace(self):
+        """Logger preserves evolver namespace for log filter compatibility."""
+        from openspace.skill_engine.evolution import confirmation
+        assert confirmation.logger.name == "openspace.skill_engine.evolver"
+
 
 # ---------------------------------------------------------------------------
 # Backward compat
@@ -274,6 +348,50 @@ def test_parse_is_staticmethod(self):
         assert result is True
 
 
+# ---------------------------------------------------------------------------
+# Delegation seam tests (real SkillEvolver → confirmation module)
+# ---------------------------------------------------------------------------
+
+@pytest.mark.skipif(not _HAS_EVOLVER, reason="SkillEvolver not importable")
+class TestDelegationSeam:
+    """Verify real SkillEvolver delegates to confirmation.py functions."""
+
+    def test_parse_confirmation_delegates(self):
+        """Real SkillEvolver._parse_confirmation routes to confirmation module."""
+        evolver = object.__new__(SkillEvolver)
+        assert evolver._parse_confirmation('{"proceed": true}') is True
+        assert evolver._parse_confirmation('{"proceed": false}') is False
+        assert evolver._parse_confirmation("yes do it") is True
+        assert evolver._parse_confirmation("no skip") is False
+
+    @pytest.mark.asyncio
+    async def test_llm_confirm_delegates_to_module(self):
+        """Real SkillEvolver._llm_confirm_evolution calls confirmation module."""
+        evolver = object.__new__(SkillEvolver)
+        # Wire up required attributes
+        evolver._model = "test-model"
+        evolver._llm_client = MagicMock()
+        evolver._llm_client.complete = AsyncMock(
+            return_value={"message": {"content": '{"proceed": true}'}}
+        )
+
+        with patch("openspace.recording.RecordingManager") as mock_rm:
+            mock_rm.record_conversation_setup = AsyncMock()
+            mock_rm.record_iteration_context = AsyncMock()
+
+            result = await evolver._llm_confirm_evolution(
+                skill_record=MagicMock(skill_id="s1"),
+                skill_content="# Skill",
+                proposed_type=MagicMock(value="fix"),
+                proposed_direction="fix it",
+                trigger_context="test",
+                recent_analyses=[],
+            )
+
+        assert result is True
+        evolver._llm_client.complete.assert_awaited_once()
+
+
 # ---------------------------------------------------------------------------
 # Size guard
 # ---------------------------------------------------------------------------