From b9b6d8e019b63438568d07e43144f98b3f194e21 Mon Sep 17 00:00:00 2001 From: Yif-Yang <29210256+Yif-Yang@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:17:54 +0000 Subject: [PATCH 1/6] chore: drop unrelated OSC sync metadata --- .osc-metadata/sync.json | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .osc-metadata/sync.json diff --git a/.osc-metadata/sync.json b/.osc-metadata/sync.json deleted file mode 100644 index e4e5732d..00000000 --- a/.osc-metadata/sync.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "fork_synced_at": "2026-07-26T09:38:34.869801+00:00", - "commits_behind_before_sync": 210, - "action_taken": "synced" -} \ No newline at end of file From 39df792d31d3433d166bf231ef6e8b3ca809904f Mon Sep 17 00:00:00 2001 From: Yif-Yang <29210256+Yif-Yang@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:17:54 +0000 Subject: [PATCH 2/6] fix(spreadsheet): complete generated-code env isolation --- .../envs/spreadsheetbench/codegen_agent.py | 15 ++++++--- skillopt/envs/spreadsheetbench/executor.py | 7 ++-- tests/test_executor_env_isolation.py | 33 ++++++++++++++++++- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/skillopt/envs/spreadsheetbench/codegen_agent.py b/skillopt/envs/spreadsheetbench/codegen_agent.py index 113b311a..b67a5089 100644 --- a/skillopt/envs/spreadsheetbench/codegen_agent.py +++ b/skillopt/envs/spreadsheetbench/codegen_agent.py @@ -262,6 +262,7 @@ def _build_codex_driver() -> str: "import re\n" "import subprocess\n" "import sys\n\n" + "import tempfile\n\n" 'INPUT_PATH = "input.xlsx"\n' 'OUTPUT_PATH = "output.xlsx"\n' "code = pathlib.Path('solution.py').read_text(encoding='utf-8')\n" @@ -277,15 +278,21 @@ def _build_codex_driver() -> str: "_safe_env = {\n" " 'PATH': _os.environ.get('PATH', '/usr/bin:/bin'),\n" " 'HOME': str(pathlib.Path('_driver_runner.py').parent.resolve()),\n" + " 'TMPDIR': tempfile.gettempdir(),\n" "}\n" "if _os.name == 'nt':\n" " _safe_env['SYSTEMROOT'] = _os.environ.get('SYSTEMROOT', '')\n" - " _safe_env['TEMP'] = _safe_env['HOME']\n" - " _safe_env['TMP'] = _safe_env['HOME']\n" + " _safe_env['TEMP'] = tempfile.gettempdir()\n" + " _safe_env['TMP'] = tempfile.gettempdir()\n" "_safe_env = {k: v for k, v in _safe_env.items() if v}\n" - "_res = subprocess.run([sys.executable, str(_patched)], capture_output=True, text=True, env=_safe_env)\n" + "try:\n" + " _res = subprocess.run([sys.executable, str(_patched)], capture_output=True, text=True, env=_safe_env)\n" + "finally:\n" + " try:\n" + " _patched.unlink()\n" + " except OSError:\n" + " pass\n" "if _res.returncode != 0:\n" - " import traceback as _tb\n" " print(_res.stdout, end='')\n" " print(_res.stderr, end='')\n" " sys.exit(2)\n" diff --git a/skillopt/envs/spreadsheetbench/executor.py b/skillopt/envs/spreadsheetbench/executor.py index 6033c9c4..09b78807 100644 --- a/skillopt/envs/spreadsheetbench/executor.py +++ b/skillopt/envs/spreadsheetbench/executor.py @@ -46,8 +46,9 @@ def run_generated_code(code: str, input_path: str, output_path: str, timeout: in with tempfile.NamedTemporaryFile("w", suffix=".py", delete=False) as f: f.write(script) tmp = f.name - # Build a minimal environment so the generated code cannot read API keys, - # cloud credentials, or other secrets from the current process environment. + # Build a minimal environment so generated code does not directly inherit + # API keys, cloud credentials, or other parent-process environment values. + # This is environment isolation, not a filesystem or network sandbox. import platform as _platform _safe_env: dict[str, str] = { "PATH": os.environ.get("PATH", "/usr/bin:/bin"), @@ -58,7 +59,7 @@ def run_generated_code(code: str, input_path: str, output_path: str, timeout: in _safe_env["SYSTEMROOT"] = os.environ.get("SYSTEMROOT", "") _safe_env["TEMP"] = tempfile.gettempdir() _safe_env["TMP"] = tempfile.gettempdir() - # Drop empty entries (env dict values must be non-empty strings) + # Omit platform-specific entries that are absent in the parent. _safe_env = {k: v for k, v in _safe_env.items() if v} try: proc = subprocess.run( diff --git a/tests/test_executor_env_isolation.py b/tests/test_executor_env_isolation.py index 0fa5372f..9a8d4776 100644 --- a/tests/test_executor_env_isolation.py +++ b/tests/test_executor_env_isolation.py @@ -7,8 +7,10 @@ """ from __future__ import annotations -import os +import subprocess +import sys +from skillopt.envs.spreadsheetbench.codegen_agent import _build_codex_driver from skillopt.envs.spreadsheetbench.executor import run_generated_code @@ -44,3 +46,32 @@ def test_path_still_available_to_generated_code(tmp_path) -> None: assert ok, err assert out.read_text(encoding="utf-8") == "YES" + + +def test_codex_driver_scrubs_env_sets_tempdir_and_cleans_runner( + tmp_path, monkeypatch +) -> None: + monkeypatch.setenv("SUPER_SECRET_TOKEN", "do-not-inherit") + (tmp_path / "solution.py").write_text( + "import os\n" + "with open(OUTPUT_PATH, 'w', encoding='utf-8') as f:\n" + " f.write('|'.join([\n" + " os.environ.get('SUPER_SECRET_TOKEN', 'ABSENT'),\n" + " 'TMPDIR' if os.environ.get('TMPDIR') else 'NO_TMPDIR',\n" + " ]))\n", + encoding="utf-8", + ) + driver = tmp_path / "run_solution.py" + driver.write_text(_build_codex_driver(), encoding="utf-8") + + proc = subprocess.run( + [sys.executable, str(driver)], + cwd=tmp_path, + capture_output=True, + text=True, + timeout=30, + ) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert (tmp_path / "output.xlsx").read_text(encoding="utf-8") == "ABSENT|TMPDIR" + assert not (tmp_path / "_driver_runner.py").exists() From d2b569be30a80e52fb6957c6f047a7cd3b2342dd Mon Sep 17 00:00:00 2001 From: Yif-Yang <29210256+Yif-Yang@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:17:54 +0000 Subject: [PATCH 3/6] fix(spreadsheet): close ReAct Python allow-list gaps --- skillopt/envs/spreadsheetbench/react_agent.py | 27 +++++++++++++------ tests/test_react_agent_no_shell.py | 12 ++++++--- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/skillopt/envs/spreadsheetbench/react_agent.py b/skillopt/envs/spreadsheetbench/react_agent.py index f9646ac5..2ddbc301 100644 --- a/skillopt/envs/spreadsheetbench/react_agent.py +++ b/skillopt/envs/spreadsheetbench/react_agent.py @@ -11,6 +11,7 @@ import os import shlex import subprocess +import sys from skillopt.model import chat_target_messages from skillopt.prompts import load_prompt @@ -249,25 +250,35 @@ def _auto_verify(work_dir: str) -> str: return f"\n\n[AUTO-VERIFY] Could not inspect output: {e}" -# Commands that the ReAct agent is allowed to run (benchmark only needs Python). -_CMD_ALLOW = {"python", "python3"} +# Command aliases that the ReAct agent may request. Every accepted alias is +# resolved to this process's interpreter before execution, so behavior does not +# depend on PATH and similarly named executables cannot bypass the allow-list. +_PYTHON_ALIASES = {"python", "python3", "python.exe", "python3.exe"} # ── Bash execution ──────────────────────────────────────────────────────────── def _run_bash(cmd: str, work_dir: str, timeout: int = 60) -> str: try: - parts = shlex.split(cmd) + parts = shlex.split(cmd, posix=os.name != "nt") + if os.name == "nt": + # shlex in non-POSIX mode retains surrounding quotes. + parts = [ + part[1:-1] + if len(part) >= 2 and part[0] == part[-1] and part[0] in {'"', "'"} + else part + for part in parts + ] if not parts: return "[error: empty command]" - exe_name = os.path.basename(parts[0]).lower() - # Strip .exe suffix on Windows (python.exe → python) - exe_stem = exe_name.split(".")[0] - if exe_stem not in _CMD_ALLOW: + exe_name = parts[0].replace("\\", "/").rsplit("/", 1)[-1].lower() + if exe_name not in _PYTHON_ALIASES: return ( - f"[blocked: '{parts[0]}' not in allow-list {sorted(_CMD_ALLOW)}; " + f"[blocked: '{parts[0]}' not in allow-list " + f"{sorted(_PYTHON_ALIASES)}; " "use Python to manipulate spreadsheets]" ) + parts[0] = sys.executable proc = subprocess.run( parts, shell=False, diff --git a/tests/test_react_agent_no_shell.py b/tests/test_react_agent_no_shell.py index 0bb181d1..267bed46 100644 --- a/tests/test_react_agent_no_shell.py +++ b/tests/test_react_agent_no_shell.py @@ -16,13 +16,19 @@ def test_disallowed_command_is_blocked(tmp_path) -> None: assert "blocked" in out.lower() -def test_allowed_python_runs(tmp_path) -> None: - # Bare 'python' resolves via PATH; a full Windows path would be mangled by - # shlex.split (posix mode), which is expected agent-input behaviour here. +def test_allowed_python_runs_without_path_lookup(tmp_path, monkeypatch) -> None: + # Accepted aliases are mapped to the running interpreter, so an absent PATH + # must not make the benchmark depend on a system-level Python command. + monkeypatch.setenv("PATH", "") out = _run_bash('python -c "print(42)"', str(tmp_path)) assert "42" in out +def test_similarly_named_executable_is_blocked(tmp_path) -> None: + out = _run_bash('python.evil -c "print(42)"', str(tmp_path)) + assert "blocked" in out.lower() + + def test_shell_metacharacters_not_interpreted(tmp_path) -> None: # With shell=False the ';' and following tokens become arguments to python, # not a second shell command, so the marker file must NOT be created. From fbf92ca6b5f6e8c55cc9263c3eb1c20265d7aa50 Mon Sep 17 00:00:00 2001 From: Yif-Yang <29210256+Yif-Yang@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:17:54 +0000 Subject: [PATCH 4/6] fix(sleep): preserve context while redacting credentials --- skillopt_sleep/staging.py | 16 +++++++++++++--- tests/test_staging_redaction_azure.py | 13 +++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/skillopt_sleep/staging.py b/skillopt_sleep/staging.py index ed2d6177..705ef2c7 100644 --- a/skillopt_sleep/staging.py +++ b/skillopt_sleep/staging.py @@ -33,8 +33,20 @@ "[REDACTED_JWT]"), (re.compile(r"(?i)(Authorization:\s*Bearer\s+)[^\s\"']+"), r"\1[REDACTED]"), (re.compile(r"(?i)(Authorization:\s*Basic\s+)[^\s\"']+"), r"\1[REDACTED]"), + # Connection-string passwords. Handle quoted values (which may contain + # semicolons) before the generic name=value rule below, and retain the key + # plus all non-secret connection-string fields for useful diagnostics. ( - re.compile(r"(?i)\b(api[_-]?key|token|password|secret)\b(\s*[:=]\s*)[^\s\"']+"), + re.compile( + r'''(?i)(\bPassword\s*=\s*)(?:"[^"]+"|'[^']+'|[^;"'\s&]+)''' + ), + r"\1[REDACTED_DB_PASS]", + ), + ( + re.compile( + r"(?i)\b(api[_-]?key|token|password|secret)\b" + r"(\s*[:=]\s*)(?!\[REDACTED(?:_[A-Z_]+)?\])[^\s\"';&]+" + ), r"\1\2[REDACTED]", ), ( @@ -52,8 +64,6 @@ (re.compile(r"(?i)\bsig=[A-Za-z0-9%+/]{10,}"), "[REDACTED_SAS_SIG]"), # Azure Storage account keys (base64, typically 88 chars) (re.compile(r"(?i)AccountKey=[A-Za-z0-9+/=]{20,}"), "[REDACTED_STORAGE_KEY]"), - # Connection-string passwords (Password=...; up to semicolon/quote/whitespace) - (re.compile(r"(?i)\bPassword=[^;\"'\s]{6,}"), "[REDACTED_DB_PASS]"), ) diff --git a/tests/test_staging_redaction_azure.py b/tests/test_staging_redaction_azure.py index 8719b94f..6db50925 100644 --- a/tests/test_staging_redaction_azure.py +++ b/tests/test_staging_redaction_azure.py @@ -28,6 +28,19 @@ def test_connection_string_password_redacted() -> None: out = redact_secrets(conn) assert "[REDACTED_DB_PASS]" in out assert "Sup3rSecret" not in out + assert out == "Server=db;Password=[REDACTED_DB_PASS];Database=app" + + +def test_quoted_connection_string_password_redacted() -> None: + conn = 'Server=db;Password="Sup3r; Secret!";Database=app' + out = redact_secrets(conn) + assert out == "Server=db;Password=[REDACTED_DB_PASS];Database=app" + + +def test_generic_secret_redaction_preserves_following_fields() -> None: + text = "token=top-secret&request=42;status=failed" + out = redact_secrets(text) + assert out == "token=[REDACTED]&request=42;status=failed" def test_recurses_into_containers() -> None: From fa8ce5a041629b55b4a0ab7dc1539c9757de8bd8 Mon Sep 17 00:00:00 2001 From: Yif-Yang <29210256+Yif-Yang@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:17:54 +0000 Subject: [PATCH 5/6] fix(sleep): track role-specific model and credential settings --- scripts/train.py | 36 +++++++++---- skillopt_sleep/cycle.py | 24 +++++++-- tests/test_model_change_warning.py | 81 +++++++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 15 deletions(-) diff --git a/scripts/train.py b/scripts/train.py index dbce3585..66f1775f 100644 --- a/scripts/train.py +++ b/scripts/train.py @@ -386,17 +386,35 @@ def load_config(args: argparse.Namespace) -> dict: 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", - ): + # F08: Warn when API keys are supplied on the CLI. Keep the replacement + # guidance specific to each backend and, where applicable, each role. + _credential_guidance = { + "azure_api_key": ( + "AZURE_OPENAI_API_KEY or " + "--azure_openai_auth_mode=managed_identity" + ), + "azure_openai_api_key": ( + "AZURE_OPENAI_API_KEY or " + "--azure_openai_auth_mode=managed_identity" + ), + "optimizer_azure_openai_api_key": ( + "OPTIMIZER_AZURE_OPENAI_API_KEY or " + "--optimizer_azure_openai_auth_mode=managed_identity" + ), + "target_azure_openai_api_key": ( + "TARGET_AZURE_OPENAI_API_KEY or " + "--target_azure_openai_auth_mode=managed_identity" + ), + "qwen_chat_api_key": "QWEN_CHAT_API_KEY", + "optimizer_qwen_chat_api_key": "OPTIMIZER_QWEN_CHAT_API_KEY", + "target_qwen_chat_api_key": "TARGET_QWEN_CHAT_API_KEY", + "minimax_api_key": "MINIMAX_API_KEY", + } + for _cli_key, _guidance in _credential_guidance.items(): 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.", + f"--{_cli_key} is deprecated: provide credentials via " + f"{_guidance} instead.", DeprecationWarning, stacklevel=2, ) diff --git a/skillopt_sleep/cycle.py b/skillopt_sleep/cycle.py index 08547c5e..76a14eb8 100644 --- a/skillopt_sleep/cycle.py +++ b/skillopt_sleep/cycle.py @@ -32,10 +32,26 @@ # ── 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", ""), + """Stable string identifying the effective backend/model role(s).""" + backend = str(cfg.get("backend", "mock") or "mock") + model = str(cfg.get("model", "") or "") + split_keys = ( + "optimizer_backend", + "optimizer_model", + "target_backend", + "target_model", + ) + if not any(cfg.get(key, "") for key in split_keys): + # Preserve the original state format for ordinary single-backend runs. + return f"{backend}::{model}" + + optimizer_backend = str(cfg.get("optimizer_backend", "") or backend) + optimizer_model = str(cfg.get("optimizer_model", "") or model) + target_backend = str(cfg.get("target_backend", "") or backend) + target_model = str(cfg.get("target_model", "") or model) + return ( + f"optimizer={optimizer_backend}::{optimizer_model};" + f"target={target_backend}::{target_model}" ) diff --git a/tests/test_model_change_warning.py b/tests/test_model_change_warning.py index 40319593..943ccf9c 100644 --- a/tests/test_model_change_warning.py +++ b/tests/test_model_change_warning.py @@ -15,10 +15,12 @@ def _cfg(): def test_last_model_key_roundtrips(tmp_path) -> None: - state = SleepState.load(str(tmp_path / "state.json")) + path = str(tmp_path / "state.json") + state = SleepState.load(path) assert state.last_model_key == "" state.set_last_model_key("anthropic::claude") - assert state.last_model_key == "anthropic::claude" + state.save() + assert SleepState.load(path).last_model_key == "anthropic::claude" def test_warns_when_model_changed(tmp_path, capsys) -> None: @@ -45,6 +47,50 @@ def test_no_warning_when_model_same(tmp_path, capsys) -> None: assert "model changed" not in capsys.readouterr().err +def test_model_key_tracks_effective_optimizer_and_target_roles() -> None: + cfg = load_config( + backend="mock", + model="shared", + optimizer_backend="claude", + optimizer_model="opus", + target_backend="codex", + target_model="gpt", + ) + assert _make_model_key(cfg) == ( + "optimizer=claude::opus;target=codex::gpt" + ) + + inherited = load_config( + backend="mock", + model="shared", + optimizer_backend="claude", + ) + assert _make_model_key(inherited) == ( + "optimizer=claude::shared;target=mock::shared" + ) + + +def test_warns_when_one_split_backend_role_changes(tmp_path, capsys) -> None: + previous = load_config( + optimizer_backend="claude", + optimizer_model="opus", + target_backend="codex", + target_model="gpt", + ) + current = load_config( + optimizer_backend="claude", + optimizer_model="opus", + target_backend="cursor", + target_model="composer", + ) + state = SleepState.load(str(tmp_path / "state.json")) + state.set_last_model_key(_make_model_key(previous)) + + _check_model_change(current, state) + + assert "model changed since last night" in capsys.readouterr().err + + def test_cli_api_key_emits_deprecation_warning() -> None: from scripts.train import load_config as train_load_config @@ -64,3 +110,34 @@ def test_cli_api_key_emits_deprecation_warning() -> None: and "azure_openai_api_key" in str(w.message) for w in caught ) + + +def test_cli_key_warnings_name_the_correct_environment_variable() -> None: + from scripts.train import load_config as train_load_config + + cases = { + "optimizer_azure_openai_api_key": "OPTIMIZER_AZURE_OPENAI_API_KEY", + "target_azure_openai_api_key": "TARGET_AZURE_OPENAI_API_KEY", + "qwen_chat_api_key": "QWEN_CHAT_API_KEY", + "optimizer_qwen_chat_api_key": "OPTIMIZER_QWEN_CHAT_API_KEY", + "target_qwen_chat_api_key": "TARGET_QWEN_CHAT_API_KEY", + "minimax_api_key": "MINIMAX_API_KEY", + } + for flag, environment_variable in cases.items(): + args = argparse.Namespace( + config="configs/_base_/default.yaml", + cfg_options=None, + **{flag: "secret-value"}, + ) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + try: + train_load_config(args) + except Exception: + pass + messages = [ + str(w.message) + for w in caught + if issubclass(w.category, DeprecationWarning) + ] + assert any(environment_variable in message for message in messages), flag From 2d658ee2344e908b0931a2836b3b225e31f7dee1 Mon Sep 17 00:00:00 2001 From: Yif-Yang <29210256+Yif-Yang@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:17:54 +0000 Subject: [PATCH 6/6] fix(sleep): reject malformed judges and empty delete anchors --- skillopt_sleep/judges.py | 4 ++-- skillopt_sleep/memory.py | 3 +++ skillopt_sleep/tasks_file.py | 2 +- tests/test_judges.py | 36 ++++++++++++++++++++++++++++++++--- tests/test_unmatched_edits.py | 7 +++++++ 5 files changed, 46 insertions(+), 6 deletions(-) diff --git a/skillopt_sleep/judges.py b/skillopt_sleep/judges.py index fb6c7040..c7da1a12 100644 --- a/skillopt_sleep/judges.py +++ b/skillopt_sleep/judges.py @@ -75,7 +75,7 @@ def _check(op: str, arg: Any, response: str, }) -def validate_checks(judge: Dict[str, Any]) -> Tuple[List[str], List[str]]: +def validate_checks(judge: Any) -> Tuple[List[str], List[str]]: """Return ``(errors, warnings)`` for a rule judge's checks. An *error* means the check can never behave as written — a regex that does @@ -85,7 +85,7 @@ def validate_checks(judge: Dict[str, Any]) -> Tuple[List[str], List[str]]: """ errors: List[str] = [] warnings: List[str] = [] - if judge and not isinstance(judge, dict): + if judge is not None and not isinstance(judge, dict): return [f"judge must be an object, got {type(judge).__name__}"], warnings checks = (judge or {}).get("checks", []) or [] if not isinstance(checks, list): diff --git a/skillopt_sleep/memory.py b/skillopt_sleep/memory.py index d54022ee..e39ff1af 100644 --- a/skillopt_sleep/memory.py +++ b/skillopt_sleep/memory.py @@ -117,6 +117,9 @@ def apply_edits_detailed( applied.append(e) elif op == "delete": anchor = _norm(e.anchor or e.content) + if not anchor: + unmatched.append(e) + continue keep = [line for line in lines if anchor not in _norm(line)] if len(keep) != len(lines): lines = keep diff --git a/skillopt_sleep/tasks_file.py b/skillopt_sleep/tasks_file.py index a3c52d26..5f25f929 100644 --- a/skillopt_sleep/tasks_file.py +++ b/skillopt_sleep/tasks_file.py @@ -86,7 +86,7 @@ def load_tasks_file( # complies — the run looks legitimate while one dimension is dead. errors: List[str] = [] for task in tasks: - if task.reference_kind != "rule" or not task.judge: + if task.reference_kind != "rule": continue task_errors, task_warnings = validate_checks(task.judge) errors.extend(f"task {task.id}: {e}" for e in task_errors) diff --git a/tests/test_judges.py b/tests/test_judges.py index c3fcd10b..4d61be61 100644 --- a/tests/test_judges.py +++ b/tests/test_judges.py @@ -4,9 +4,13 @@ A regex that does not compile returns False on every rollout, so the affected dimension scores 0.0 forever while the run still looks healthy. """ +import json import unittest +import pytest + from skillopt_sleep.judges import KNOWN_OPS, score_rule_judge, validate_checks +from skillopt_sleep.tasks_file import load_tasks_file class TestCheckOperators(unittest.TestCase): @@ -56,7 +60,7 @@ def test_empty_checks_score_zero(self) -> None: class TestMalformedRegexIsDistinguishable(unittest.TestCase): """A pattern Python cannot parse must not read like a plain miss.""" - BAD = r"(?i)foo|(?i)bar" # inline flag not at the start -> re.error + BAD = r"foo(" # unclosed group is invalid on every supported Python version def test_bad_pattern_still_fails_closed(self) -> None: # the response does contain both alternatives; the pattern is the problem @@ -87,7 +91,7 @@ def test_sound_checks_produce_nothing(self) -> None: self.assertEqual(warnings, []) def test_uncompilable_regex_is_an_error(self) -> None: - errors, _warnings = validate_checks({"checks": [{"op": "regex", "arg": r"(?i)a|(?i)b"}]}) + errors, _warnings = validate_checks({"checks": [{"op": "regex", "arg": r"foo("}]}) self.assertEqual(len(errors), 1) self.assertIn("does not compile", errors[0]) @@ -108,7 +112,7 @@ def test_non_object_check_is_an_error(self) -> None: def test_malformed_judge_is_reported_not_raised(self) -> None: # a tasks file may carry any JSON here; validation must stay structured - for bad in (["not", "a", "dict"], "a string", 42): + for bad in (["not", "a", "dict"], [], "a string", "", 42, 0): errors, _warnings = validate_checks(bad) self.assertEqual(len(errors), 1, bad) self.assertIn("must be an object", errors[0]) @@ -129,5 +133,31 @@ def test_every_known_op_is_accepted(self) -> None: self.assertEqual((errors, warnings), ([], []), op) +@pytest.mark.parametrize("bad_judge", [[], "", 0]) +def test_tasks_file_rejects_falsy_non_object_judges( + tmp_path, bad_judge +) -> None: + path = tmp_path / "tasks.json" + path.write_text( + json.dumps( + { + "tasks": [ + { + "id": "malformed-rule", + "project": "test", + "intent": "test", + "reference_kind": "rule", + "judge": bad_judge, + } + ] + } + ), + encoding="utf-8", + ) + + with pytest.raises(ValueError, match="judge must be an object"): + load_tasks_file(str(path)) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_unmatched_edits.py b/tests/test_unmatched_edits.py index 899e9ac8..116e2724 100644 --- a/tests/test_unmatched_edits.py +++ b/tests/test_unmatched_edits.py @@ -29,6 +29,13 @@ def test_delete_with_absent_anchor_is_unmatched(self) -> None: _new, applied, unmatched = apply_edits_detailed(doc, [e]) self.assertEqual((applied, unmatched), ([], [e])) + def test_delete_with_empty_anchor_is_unmatched_and_preserves_all_lines(self) -> None: + doc = _doc("keep one", "keep two") + e = EditRecord(target="skill", op="delete", content="", anchor="") + new_doc, applied, unmatched = apply_edits_detailed(doc, [e]) + self.assertEqual((applied, unmatched), ([], [e])) + self.assertEqual(new_doc, doc) + def test_duplicate_add_is_unmatched_not_applied(self) -> None: doc = _doc("existing rule") e = EditRecord(target="skill", op="add", content="Existing Rule")