diff --git a/skillopt_sleep/harvest_copilot_cli.py b/skillopt_sleep/harvest_copilot_cli.py index eeb88f2a..13c081ee 100644 --- a/skillopt_sleep/harvest_copilot_cli.py +++ b/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] diff --git a/skillopt_sleep/judges.py b/skillopt_sleep/judges.py index eb5d6471..6cdb4cb7 100644 --- a/skillopt_sleep/judges.py +++ b/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, diff --git a/skillopt_sleep/multi_skill.py b/skillopt_sleep/multi_skill.py index e752956f..256ce6ab 100644 --- a/skillopt_sleep/multi_skill.py +++ b/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( diff --git a/tests/test_harvest_copilot_cli.py b/tests/test_harvest_copilot_cli.py index b33a3ee4..2d70908b 100644 --- a/tests/test_harvest_copilot_cli.py +++ b/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") == [] - diff --git a/tests/test_outcome_judges.py b/tests/test_outcome_judges.py index ab916074..cf6e0e92 100644 --- a/tests/test_outcome_judges.py +++ b/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) - - diff --git a/tests/test_sleep_multi_skill.py b/tests/test_sleep_multi_skill.py index ff57adb0..9aee18fe 100644 --- a/tests/test_sleep_multi_skill.py +++ b/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(