Merge pull request #203 from Yif-Yang/fix/aug06-post-merge-hardening

fix(sleep): harden merged outcome, grouping, and Copilot paths

Author: Yif-Yang

skillopt_sleep/harvest_copilot_cli.py

@@ -27,6 +27,7 @@
     _is_meta_prompt,
     _project_matches,
 )
+from skillopt_sleep.staging import redact_secrets
 from skillopt_sleep.types import SessionDigest
 
 # Bound per-session text so one pathological session cannot dominate a night's
@@ -46,7 +47,9 @@ def default_session_store() -> str:
 def _clip(text: Any) -> str:
     if not isinstance(text, str):
         return ""
-    text = text.strip()
+    # Redact before truncating: clipping first could retain and persist only a
+    # secret fragment that no longer matches the shared redaction patterns.
+    text = str(redact_secrets(text)).strip()
     return text[:_MAX_TEXT_CHARS]
 
 

skillopt_sleep/judges.py

@@ -74,12 +74,13 @@ def _is_refusal(response: str) -> bool:
     # Strip leading markdown markers -- blockquote (>), list bullets (-, *),
     # numbered items (1. / 1)), emphasis and headings -- BEFORE bounding the
     # head, so a refusal cannot hide behind >160 marker characters.
-    head = re.sub(r"^(?:[>\-*_#\s]|\d+[.)])+", "", text.lower())[:160]
+    content = re.sub(r"^(?:[>\-*_#\s]|\d+[.)])+", "", text)
+    head = content[:160].lower()
     if not any(head.startswith(p) for p in _REFUSAL_PREFIXES):
         return False
     # A long response that opens with an abstention still did the work of
     # explaining why; only terse dead-ends are refusals.
-    return len(text) < 600
+    return len(content) < 600
 
 
 def _check(op: str, arg: Any, response: str,

skillopt_sleep/multi_skill.py

@@ -70,6 +70,10 @@ def consolidate_groups(
     ``memory`` is the shared agent memory and is passed through read-only: group
     runs evolve skills only, so no group can rewrite another group's memory.
     """
+    # This wrapper's contract is stricter than consolidate(): shared memory is
+    # always read-only.  Override a caller-supplied value instead of passing a
+    # duplicate keyword (which would otherwise turn the group into a failure).
+    consolidate_kwargs["evolve_memory"] = False
     out: Dict[str, GroupConsolidation] = {}
     for group in groups:
         name = (group.skill_name or "").strip()
@@ -84,7 +88,7 @@ def consolidate_groups(
         try:
             result = consolidate_fn(
                 backend, list(group.tasks), group.skill, memory,
-                evolve_memory=False, **consolidate_kwargs,
+                **consolidate_kwargs,
             )
         except Exception as exc:  # one group's failure must not abort the night
             out[name] = GroupConsolidation(

tests/test_harvest_copilot_cli.py

@@ -90,6 +90,45 @@ def test_maps_session_and_turn_fields(tmp_path) -> None:
     assert d.raw_path.endswith("#s1")
 
 
+def test_redacts_user_and_assistant_secrets_before_harvesting(tmp_path) -> None:
+    user_secret = "sk-abcdefghijklmnopqrstuvwxyz1234567890"
+    assistant_secret = "super-secret-value-123456"
+    path = _store(
+        tmp_path,
+        [("s1", r"C:\proj", "repo", "main", "2026-01-01 10:00:00", "2026-01-01 10:30:00")],
+        [
+            (
+                "s1",
+                0,
+                f"Use Authorization: Bearer {user_secret} for this task",
+                f"Configured api_key={assistant_secret}",
+                "2026-01-01 10:00:00",
+            )
+        ],
+    )
+
+    [digest] = harvest_copilot_cli(path, scope="all")
+    harvested = "\n".join(digest.user_prompts + digest.assistant_finals)
+    assert user_secret not in harvested
+    assert assistant_secret not in harvested
+    assert "[REDACTED" in harvested
+
+
+def test_redacts_secrets_before_text_is_clipped(tmp_path) -> None:
+    secret = "sk-abcdefghijklmnopqrstuvwxyz1234567890"
+    # The token begins just before the 4000-char boundary. Clipping first would
+    # leave a secret fragment that no longer matches the redaction pattern.
+    prompt = "x" * (4000 - 5) + secret
+    path = _store(
+        tmp_path,
+        [("s1", r"C:\proj", "repo", "main", "2026-01-01 10:00:00", "2026-01-01 10:30:00")],
+        [("s1", 0, prompt, "done", "2026-01-01 10:00:00")],
+    )
+
+    [digest] = harvest_copilot_cli(path, scope="all")
+    assert "sk-" not in digest.user_prompts[0]
+
+
 def test_engine_self_calls_are_filtered(tmp_path) -> None:
     # SkillOpt's own Copilot backend writes to this same store; harvesting them
     # would train the engine on its own output.
@@ -269,4 +308,3 @@ def _boom(_store_path):
 
     monkeypatch.setattr("skillopt_sleep.harvest_copilot_cli._connect", _boom)
     assert harvest_copilot_cli(path, scope="all") == []
-

tests/test_outcome_judges.py

@@ -58,6 +58,23 @@ def test_no_refusal_passes_on_substantive_answer() -> None:
     assert score_rule_judge(judge, "Here is the context you asked for: ...")[0] == 1.0
 
 
+def test_no_refusal_ignores_markdown_prefix_length() -> None:
+    judge = {"kind": "rule", "checks": [{"op": "no_refusal"}]}
+    response = "> " * 350 + "I cannot help with that."
+    assert len(response) >= 600
+    assert score_rule_judge(judge, response)[0] == 0.0
+
+
+def test_no_refusal_length_is_measured_before_unicode_lowercasing() -> None:
+    judge = {"kind": "rule", "checks": [{"op": "no_refusal"}]}
+    # U+0130 lowercases to two code points. Case normalization must not turn a
+    # short refusal into an apparently substantive response over 600 chars.
+    response = "I cannot help. " + "İ" * 300
+    assert len(response) < 600
+    assert len(response.lower()) >= 600
+    assert score_rule_judge(judge, response)[0] == 0.0
+
+
 def test_no_refusal_accepts_a_refusal_that_still_does_the_work() -> None:
     # An abstention that explains what was searched and what is missing is a
     # useful answer, not a dead end.
@@ -383,5 +400,3 @@ def test_char_bound_rejects_non_integers(bad) -> None:
     with pytest.raises((ValueError, TypeError)):
         char_bound(bad)
 
-
-

tests/test_sleep_multi_skill.py

@@ -163,6 +163,21 @@ def _fake(backend, tasks, skill, memory, **kwargs):
         self.assertEqual(seen["memory"], "# shared memory\n")
         self.assertFalse(seen["evolve_memory"])
 
+    def test_caller_cannot_override_shared_memory_isolation(self):
+        seen = {}
+
+        def _fake(backend, tasks, skill, memory, **kwargs):
+            seen.update(kwargs)
+            return consolidate(backend, tasks, skill, memory, **kwargs)
+
+        outcomes = consolidate_groups(
+            MockBackend(),
+            [SkillGroup("research-skill", set_learned("", []), _tasks(researcher_persona))],
+            consolidate_fn=_fake, edit_budget=4, night=1, evolve_memory=True,
+        )
+        self.assertEqual(outcomes["research-skill"].status, CONSOLIDATED)
+        self.assertFalse(seen["evolve_memory"])
+
     def test_accepted_group_skills_lists_only_accepted_updates(self):
         outcomes = {
             "kept": GroupConsolidation(