From 4a65cc5b2c3f453397e23afe349c91b4a5709fcf Mon Sep 17 00:00:00 2001 From: Murali Chillakuru Date: Sat, 25 Jul 2026 18:46:14 -0400 Subject: [PATCH] security: warn on model swap between nights; correct docs and CLI credential guidance F16: persist last_model_key in sleep state and warn at cycle start when the backend/model changed since the previous night (skill text may not transfer). F12: correct docs to say replay isolation varies by backend. F08: emit a DeprecationWarning when API keys are passed via train.py CLI args, pointing to env vars / managed identity. Adds tests for the state roundtrip, the warning conditions, and the CLI deprecation warning. --- docs/sleep/README.md | 2 +- scripts/train.py | 16 ++++++++ skillopt_sleep/cycle.py | 30 ++++++++++++++ skillopt_sleep/state.py | 9 ++++ tests/test_model_change_warning.py | 66 ++++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 tests/test_model_change_warning.py diff --git a/docs/sleep/README.md b/docs/sleep/README.md index 9ad55be3..2b740ac9 100644 --- a/docs/sleep/README.md +++ b/docs/sleep/README.md @@ -17,7 +17,7 @@ normal agent requests. One "night": ``` -harvest Claude Code / Codex / Cursor transcripts → mine recurring tasks → replay in isolated model calls +harvest Claude Code / Codex / Cursor transcripts → mine recurring tasks → replay via the configured backend (isolation varies by backend; mock/handoff make no network calls) → consolidate (reflect → bounded edit → GATE on real held-out tasks) → stage proposal → (you) adopt ``` diff --git a/scripts/train.py b/scripts/train.py index 63cad9e1..dbce3585 100644 --- a/scripts/train.py +++ b/scripts/train.py @@ -383,8 +383,24 @@ def parse_args() -> argparse.Namespace: def load_config(args: argparse.Namespace) -> dict: """Load config with _base_ inheritance, then apply CLI overrides.""" + import warnings from skillopt.config import load_config as _load, flatten_config, is_structured + # F08: Warn when API keys are supplied on the CLI — prefer env vars or managed identity. + for _cli_key in ( + "azure_api_key", "azure_openai_api_key", + "optimizer_azure_openai_api_key", "target_azure_openai_api_key", + "minimax_api_key", + ): + if getattr(args, _cli_key, None): + warnings.warn( + f"--{_cli_key} is deprecated: provide credentials via the " + f"AZURE_OPENAI_API_KEY environment variable or set " + f"--azure_openai_auth_mode=managed_identity instead.", + DeprecationWarning, + stacklevel=2, + ) + cfg = _load(args.config, overrides=args.cfg_options) structured = is_structured(cfg) diff --git a/skillopt_sleep/cycle.py b/skillopt_sleep/cycle.py index f177d4c4..dadccb2d 100644 --- a/skillopt_sleep/cycle.py +++ b/skillopt_sleep/cycle.py @@ -30,6 +30,34 @@ from skillopt_sleep.types import SessionDigest, SleepReport, TaskRecord +# ── Model-swap detection (F16) ─────────────────────────────── +def _make_model_key(cfg: SleepConfig) -> str: + """Stable string identifying the backend+model combination for this cycle.""" + return "{}::{}".format( + cfg.get("backend", "mock"), + cfg.get("model", ""), + ) + + +def _check_model_change(cfg: SleepConfig, state: SleepState) -> None: + """Warn when the backend/model has changed since the last night. + + Skill text is backend-specific; adopting edits from a different model's + reflections into a new model's skill file can cause regressions. + This is advisory only — the cycle continues either way. + """ + current_key = _make_model_key(cfg) + prior_key = state.last_model_key + if prior_key and prior_key != current_key: + print( + f"[sleep] WARNING: model changed since last night " + f"(was {prior_key!r}, now {current_key!r}). " + "Learned skill text may not transfer cleanly. " + "Consider starting from a fresh skill document.", + file=sys.stderr, + ) + + @dataclass class CycleOutcome: report: SleepReport @@ -127,6 +155,7 @@ def run_sleep_cycle( """ cfg = cfg or load_config() state = SleepState.load(cfg.state_path) + _check_model_change(cfg, state) # F16: warn if model changed between nights night = state.begin_night(clock) project = _project_paths(cfg) started = _now_iso(clock) @@ -409,6 +438,7 @@ def run_sleep_cycle( "baseline": result.baseline_score, "candidate": result.candidate_score, "n_tasks": len(tasks), "staging": staging_dir, }) + state.set_last_model_key(_make_model_key(cfg)) # F16: track model for next night # ── 6. adopt (opt-in) ──────────────────────────────────────────── if cfg.get("auto_adopt") and result.accepted: adopted_paths = adopt_staging(staging_dir) diff --git a/skillopt_sleep/state.py b/skillopt_sleep/state.py index 1e161571..905443a4 100644 --- a/skillopt_sleep/state.py +++ b/skillopt_sleep/state.py @@ -29,6 +29,7 @@ def _now_iso(clock: Optional[float] = None) -> str: "slow_memory": "", # cross-night consolidated lessons (meta-skill analogue) "history": [], # list of per-night summaries "task_archive": [], # capped list of past mined tasks (for associative recall) + "last_model_key": "", # "backend::model" string used in the last successful night (F16) } @@ -94,3 +95,11 @@ def add_to_archive(self, task_dicts: list, cap: int = 300) -> None: arc.extend(task_dicts) if len(arc) > cap: self.data["task_archive"] = arc[-cap:] + + # ── model-swap tracking (F16) ───────────────────────────────────────── + @property + def last_model_key(self) -> str: + return str(self.data.get("last_model_key", "")) + + def set_last_model_key(self, key: str) -> None: + self.data["last_model_key"] = key diff --git a/tests/test_model_change_warning.py b/tests/test_model_change_warning.py new file mode 100644 index 00000000..40319593 --- /dev/null +++ b/tests/test_model_change_warning.py @@ -0,0 +1,66 @@ +"""Tests for the F16 model-change warning and F08 CLI credential warning.""" +from __future__ import annotations + +import argparse +import warnings + +from skillopt_sleep.cycle import _check_model_change, _make_model_key +from skillopt_sleep.config import load_config +from skillopt_sleep.state import SleepState + + +def _cfg(): + cfg = load_config() + return cfg + + +def test_last_model_key_roundtrips(tmp_path) -> None: + state = SleepState.load(str(tmp_path / "state.json")) + assert state.last_model_key == "" + state.set_last_model_key("anthropic::claude") + assert state.last_model_key == "anthropic::claude" + + +def test_warns_when_model_changed(tmp_path, capsys) -> None: + cfg = _cfg() + state = SleepState.load(str(tmp_path / "state.json")) + state.set_last_model_key("some-other::model") + _check_model_change(cfg, state) + err = capsys.readouterr().err + assert "model changed since last night" in err + + +def test_no_warning_on_first_night(tmp_path, capsys) -> None: + cfg = _cfg() + state = SleepState.load(str(tmp_path / "state.json")) # last_model_key == "" + _check_model_change(cfg, state) + assert "model changed" not in capsys.readouterr().err + + +def test_no_warning_when_model_same(tmp_path, capsys) -> None: + cfg = _cfg() + state = SleepState.load(str(tmp_path / "state.json")) + state.set_last_model_key(_make_model_key(cfg)) + _check_model_change(cfg, state) + assert "model changed" not in capsys.readouterr().err + + +def test_cli_api_key_emits_deprecation_warning() -> None: + from scripts.train import load_config as train_load_config + + args = argparse.Namespace( + config="configs/_base_/default.yaml", + cfg_options=None, + azure_openai_api_key="sk-secret-value", + ) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + try: + train_load_config(args) + except Exception: + pass # config loading may fail; we only assert the warning fired + assert any( + issubclass(w.category, DeprecationWarning) + and "azure_openai_api_key" in str(w.message) + for w in caught + )