diff --git a/coworker/agent.py b/coworker/agent.py index 9479718c8..8ac40ddc0 100644 --- a/coworker/agent.py +++ b/coworker/agent.py @@ -244,6 +244,8 @@ def build_engine( # are user-global, preserving the "a repo can't enable this" invariant. auto_approve: Optional[bool] = None, auto_approve_shadow: Optional[bool] = None, + # Dedicated reviewer model (Issue #615). None ⇒ config.toml or session model. + reviewer_model: Optional[str] = None, # Persona-carried skill folders (OPE-58): the bundle's skills/ dir joins the loader so # its skills are readable by load_skill, not just listed by the filter. extra_skill_dirs: Optional[list[str | Path]] = None, @@ -597,9 +599,14 @@ def _approval_extras(tool_name: str, _arguments: dict) -> dict: if live_on or shadow_on: from .reviewer import Reviewer + effective_reviewer_model = ( + reviewer_model + or getattr(config, "reviewer_model", None) + or model + ) engine.reviewer = Reviewer( provider=provider, - model=model, + model=effective_reviewer_model, known_world=engine.session_facts.world.render(), ) # Shadow evaluation (Part 6 step 3): with only the shadow flag on, the reviewer is diff --git a/coworker/config.py b/coworker/config.py index 43fe33fa8..17f76b3f0 100644 --- a/coworker/config.py +++ b/coworker/config.py @@ -52,6 +52,9 @@ class Config: # gates (zero false-allows; ≥30% fewer prompts) get measured on real sessions. Costs # one model call per card while on. Off by default; user-global only. auto_approve_shadow: bool = False + # Dedicated reviewer model for auto-approve mode (spec §1.5 / Issue #615). + # If None or empty, the reviewer inherits the session's model. User-global only. + reviewer_model: Optional[str] = None host: str = "127.0.0.1" port: int = 8765 # Web search provider: "duckduckgo" (keyless default) | "tavily" | "brave" (need a key). @@ -84,6 +87,7 @@ class Config: "allowed_domains", "auto_approve", "auto_approve_shadow", + "reviewer_model", "host", "port", "web_search_provider", @@ -104,6 +108,7 @@ class Config: "allowed_domains", "auto_approve", "auto_approve_shadow", + "reviewer_model", } _WORKSPACE_FIELDS = _FIELDS - _GLOBAL_ONLY_FIELDS @@ -139,9 +144,16 @@ def load_config( g = Path(global_path) if global_path is not None else global_config_path() if g.is_file(): - for key, value in _read(g).items(): + data = _read(g) + for key, value in data.items(): if key in _FIELDS: setattr(cfg, key, value) + if "reviewer" in data and isinstance(data["reviewer"], dict): + rm = data["reviewer"].get("model") + if isinstance(rm, str) and rm.strip(): + cfg.reviewer_model = rm.strip() + if cfg.reviewer_model is not None and not str(cfg.reviewer_model).strip(): + cfg.reviewer_model = None if workspace: w = Path(workspace).expanduser() / ".coworker" / "config.toml" if w.is_file(): diff --git a/coworker/server/app.py b/coworker/server/app.py index c2e60e159..596e4daeb 100644 --- a/coworker/server/app.py +++ b/coworker/server/app.py @@ -1944,6 +1944,11 @@ def settings_set_auto_approve_shadow(body: dict) -> dict[str, Any]: # every approval card while the human still decides. Independent of the live flag. return manager.set_auto_approve_shadow((body or {}).get("auto_approve_shadow", False)) + @app.post("/v1/settings/reviewer-model") + def settings_set_reviewer_model(body: dict) -> dict[str, Any]: + # Dedicated reviewer model for Auto-Approve (Issue #615). + return manager.set_reviewer_model((body or {}).get("reviewer_model")) + @app.post("/v1/settings/pdf") def settings_set_pdf(body: dict) -> dict[str, Any]: # Token savings (owner ask, 2026-07-17): fallback mode for models without native diff --git a/coworker/server/manager.py b/coworker/server/manager.py index cf9b740fa..cfcdfe195 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -724,6 +724,7 @@ def get_engine( # the next session build without a config.toml edit. auto_approve=self.auto_approve(), auto_approve_shadow=self.auto_approve_shadow(), + reviewer_model=self.reviewer_model(), ) # An automation run rebuilt here (manual "Run now" over WS, durable resume) still # carries its task's standing allowances — the rules live on the task record. @@ -3446,6 +3447,7 @@ def _selectable(m: str) -> bool: # Settings toggles and gate the composer's Auto-Approve mode entry. "auto_approve": self.auto_approve(), "auto_approve_shadow": self.auto_approve_shadow(), + "reviewer_model": self.reviewer_model(), "scratch_base": self._prefs.get("scratch_base") or self.DEFAULT_SCRATCH_BASE, # Real on-disk secrets location, so the UI shows the OS-native path instead of a @@ -3551,6 +3553,25 @@ def set_auto_approve_shadow(self, on: Any) -> dict[str, Any]: "auto_approve_shadow": self.auto_approve_shadow(), } + def reviewer_model(self) -> Optional[str]: + from ..config import load_config + + if "reviewer_model" in self._prefs and self._prefs["reviewer_model"]: + return str(self._prefs["reviewer_model"]).strip() or None + return load_config().reviewer_model + + def set_reviewer_model(self, model: Any) -> dict[str, Any]: + val = str(model).strip() if model else "" + if val: + self._prefs["reviewer_model"] = val + else: + self._prefs.pop("reviewer_model", None) + self._save_prefs() + return { + "ok": True, + "reviewer_model": self.reviewer_model(), + } + # -- PDF attachments / token savings (owner ask, 2026-07-17) ---------------- DEFAULT_PDF_MAX_PAGES = 20 DEFAULT_PDF_MAX_MB = 10 diff --git a/tests/test_auto_approve_settings.py b/tests/test_auto_approve_settings.py index a7a373158..7f25d6725 100644 --- a/tests/test_auto_approve_settings.py +++ b/tests/test_auto_approve_settings.py @@ -76,10 +76,73 @@ def test_build_engine_override_beats_config(tmp_path, monkeypatch): # config has it off by default; override to on → a reviewer is attached. engine = build_engine(agent=chat_agent(), auto_approve=True, auto_approve_shadow=False) assert engine.reviewer is not None - engine2 = build_engine(agent=chat_agent(), auto_approve=False, auto_approve_shadow=False) + engine2 = build_engine( + agent=chat_agent(), auto_approve=False, auto_approve_shadow=False + ) assert engine2.reviewer is None +def test_reviewer_model_endpoint_roundtrip(client): + s = client.get("/v1/settings").json() + assert s.get("reviewer_model") is None + + # Set dedicated reviewer model + r = client.post( + "/v1/settings/reviewer-model", + json={"reviewer_model": "anthropic:claude-3-5-haiku"}, + ).json() + assert r["ok"] and r["reviewer_model"] == "anthropic:claude-3-5-haiku" + assert ( + client.get("/v1/settings").json()["reviewer_model"] + == "anthropic:claude-3-5-haiku" + ) + + # Clear dedicated model + r_clear = client.post( + "/v1/settings/reviewer-model", json={"reviewer_model": ""} + ).json() + assert r_clear["ok"] and r_clear["reviewer_model"] is None + assert client.get("/v1/settings").json()["reviewer_model"] is None + + +def test_reviewer_model_falls_back_to_config(tmp_path, monkeypatch): + state = tmp_path / "state" + state.mkdir(parents=True) + (state / "config.toml").write_text('[reviewer]\nmodel = "ollama:llama3.2:3b"\n') + monkeypatch.setenv("COWORKER_STATE_DIR", str(state)) + mgr = SessionManager(data_dir=tmp_path / "data") + assert mgr.reviewer_model() == "ollama:llama3.2:3b" + + # Prefs override takes precedence + mgr.set_reviewer_model("anthropic:claude-3-5-haiku") + assert mgr.reviewer_model() == "anthropic:claude-3-5-haiku" + + +def test_build_engine_reviewer_model(tmp_path, monkeypatch): + monkeypatch.setenv("COWORKER_STATE_DIR", str(tmp_path / "state")) + from coworker.agent import build_engine + from coworker.agents.chat import chat_agent + + # When reviewer_model is supplied, engine.reviewer uses it + engine = build_engine( + agent=chat_agent(), + auto_approve=True, + model="gpt-5.6-sol", + reviewer_model="anthropic:claude-3-5-haiku", + ) + assert engine.reviewer is not None + assert engine.reviewer.model == "anthropic:claude-3-5-haiku" + + # When reviewer_model is omitted, engine.reviewer inherits session model + engine_default = build_engine( + agent=chat_agent(), + auto_approve=True, + model="gpt-5.6-sol", + ) + assert engine_default.reviewer is not None + assert engine_default.reviewer.model == "gpt-5.6-sol" + + # -- metering (§1.7): durable reviewer stats from the audit store ------------------ diff --git a/tests/test_config.py b/tests/test_config.py index 22915ba6f..a1a0a999b 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -142,3 +142,26 @@ def test_cloud_endpoints_default_to_production(): cfg = Config() assert cfg.cloud_base_url == "https://api.openworker.com" assert cfg.cloud_relay_ws_url.startswith("wss://") + + +def test_reviewer_model_config(tmp_path): + g1 = tmp_path / "g1.toml" + g1.write_text('[reviewer]\nmodel = "anthropic:claude-3-5-haiku"\n') + cfg1 = load_config(global_path=g1) + assert cfg1.reviewer_model == "anthropic:claude-3-5-haiku" + + g2 = tmp_path / "g2.toml" + g2.write_text('reviewer_model = "ollama:llama3.2:3b"\n') + cfg2 = load_config(global_path=g2) + assert cfg2.reviewer_model == "ollama:llama3.2:3b" + + # Workspace config cannot override reviewer_model (user-global security invariant) + ws = tmp_path / "ws" + (ws / ".coworker").mkdir(parents=True) + (ws / ".coworker" / "config.toml").write_text( + '[reviewer]\nmodel = "untrusted:rogue-model"\n' + 'reviewer_model = "untrusted:rogue"\n' + ) + cfg3 = load_config(ws, global_path=g1) + assert cfg3.reviewer_model == "anthropic:claude-3-5-haiku" +