From 6c1899a2fbc4c63443254d82211193cbd0a04248 Mon Sep 17 00:00:00 2001 From: Kushal Dave Date: Sun, 8 Nov 2026 22:42:25 +0000 Subject: [PATCH] Support per-agent model selection and run overrides --- README.md | 3 ++ docs/REFERENCE.md | 19 +++++++++++ src/secfoo/agents/api.py | 4 ++- src/secfoo/agents/base.py | 16 +++++++++ src/secfoo/agents/claude.py | 2 ++ src/secfoo/agents/codex.py | 2 ++ src/secfoo/agents/copilot.py | 3 +- src/secfoo/agents/cursor.py | 3 +- src/secfoo/agents/droid.py | 2 ++ src/secfoo/agents/gemini.py | 3 +- src/secfoo/cli.py | 10 +++++- src/secfoo/config.example.toml | 7 ++++ src/secfoo/runner.py | 6 ++++ src/secfoo/settings.py | 16 ++++++++- src/secfoo/storage/models.py | 1 + src/secfoo/storage/repository.py | 6 ++-- src/secfoo/storage/schema.sql | 1 + tests/test_agents/test_api.py | 14 ++++++++ tests/test_agents/test_models.py | 49 +++++++++++++++++++++++++++ tests/test_cli.py | 26 ++++++++++++++ tests/test_runner.py | 19 +++++++++++ tests/test_settings.py | 14 ++++++++ tests/test_storage/test_repository.py | 21 ++++++++++++ 23 files changed, 239 insertions(+), 8 deletions(-) create mode 100644 tests/test_agents/test_models.py diff --git a/README.md b/README.md index 6a3aaf5..c220b43 100644 --- a/README.md +++ b/README.md @@ -114,3 +114,6 @@ Developing this repo: `pip install -e ".[dev]"`. PRs welcome — see [CONTRIBUTING.md](CONTRIBUTING.md) and [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md). Security issues go through [SECURITY.md](SECURITY.md), not a public issue. + +Choose a model for one assessment with `secfoo run --skill sast --agent claude --model `, +or set a per-agent default in [configuration](docs/REFERENCE.md#configuration-secfooconfigtoml). diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 42e21c2..97093ff 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -437,6 +437,9 @@ exclude = ["vendor/", "some-cloned-repo/"] fail_on = "high" max_cost_usd = 2.00 +[agents.claude] +model = "claude-sonnet-4-6" + [[mcp_servers]] name = "Atlassian-Rovo-MCP" command = "npx" @@ -447,6 +450,22 @@ CLI flags always override `[defaults]`. See [`src/secfoo/config.example.toml`](../src/secfoo/config.example.toml) for the full reference, including remote HTTP/SSE servers. +`secfoo run --model ` overrides `[agents.].model` for +that invocation without changing any CLI global settings. Use the registered +agent ID (`agent` for Cursor, `agy` for Antigravity). Model IDs must be +non-empty strings. Claude, Codex, Gemini, Copilot and Cursor receive `--model`; +Droid receives `-m`. Antigravity and Devin currently warn and ignore model +selection because no supported model flag is established for these adapters. +The `api` agent uses the selected model ahead of `SECFOO_API_MODEL`, then its +existing built-in default. With no selection, CLI commands remain unchanged. + +Each run stores the **requested** model (`runs.model`), not a provider-verified +model identity. It is NULL when no model was selected (including older runs). +A setting ignored by an unsupported adapter remains a requested label; its +warning explains that the CLI default was used. Run records do not infer the +CLI's global default or resolve provider aliases. Use model IDs supported by +your installed CLI version; an invalid ID is reported by that CLI. + How MCP servers reach each agent varies by what its CLI actually supports: | Agent | Mechanism | diff --git a/src/secfoo/agents/api.py b/src/secfoo/agents/api.py index 19b5ba6..aee86da 100644 --- a/src/secfoo/agents/api.py +++ b/src/secfoo/agents/api.py @@ -249,6 +249,8 @@ def is_available(self) -> bool: def build_command(self, prompt: str, *, workdir: Path) -> list[str]: return [] # required by the base class; there is no program to launch + supports_model = True + def run(self, prompt: str, *, workdir: Path, timeout: int | None = None, prev_commit: str | None = None) -> AgentResult: # Memory Bank: prev_commit triggers incremental diff scan. started = time.monotonic() _strip_key_whitespace() @@ -268,7 +270,7 @@ def run(self, prompt: str, *, workdir: Path, timeout: int | None = None, prev_co final = build_graph(litellm, StateGraph, START, END).invoke({ "prompt": prompt, "workdir": str(workdir), - "model": os.environ.get("SECFOO_API_MODEL", DEFAULT_MODEL), + "model": self.model or os.environ.get("SECFOO_API_MODEL", DEFAULT_MODEL), "timeout": timeout or self.default_timeout_seconds, "prev_commit": prev_commit, # Memory Bank: None = full scan, SHA = incremental diff. "scan_scope": None, diff --git a/src/secfoo/agents/base.py b/src/secfoo/agents/base.py index 44bb033..3a123d1 100644 --- a/src/secfoo/agents/base.py +++ b/src/secfoo/agents/base.py @@ -13,6 +13,7 @@ import signal import subprocess import time +import warnings from abc import ABC, abstractmethod from dataclasses import dataclass from pathlib import Path @@ -95,6 +96,21 @@ class AgentAdapter(ABC): # documents reading the prompt from stdin. prompt_via_stdin: ClassVar[bool] = False + model: str | None = None + model_flag: ClassVar[str | None] = None + supports_model: ClassVar[bool] = False + + def configure_model(self, model: str | None) -> None: + if model is not None and (not isinstance(model, str) or not model.strip()): + raise ValueError("model must be a non-empty string") + if model is not None and not (self.model_flag or self.supports_model): + warnings.warn(f"Agent {self.name!r} does not support model selection; ignoring model", stacklevel=2) + model = None + self.model = model + + def model_arguments(self) -> list[str]: + return [self.model_flag, self.model] if self.model_flag and self.model is not None else [] + def is_available(self) -> bool: return shutil.which(self.binary) is not None diff --git a/src/secfoo/agents/claude.py b/src/secfoo/agents/claude.py index b7c6147..f1325e0 100644 --- a/src/secfoo/agents/claude.py +++ b/src/secfoo/agents/claude.py @@ -52,6 +52,7 @@ class ClaudeAdapter(AgentAdapter): name = "claude" binary = "claude" default_timeout_seconds = 1800 + model_flag = "--model" # SECFOO-42: with no prompt argument, `claude -p` reads the prompt from # stdin. Passing it as argv breaks an npm install on Windows, where # `claude` is a `.cmd` shim and cmd.exe cuts the argument at its first @@ -78,6 +79,7 @@ def build_command(self, prompt: str, *, workdir: Path) -> list[str]: cmd = [ self.binary, + *self.model_arguments(), # No prompt here: it arrives on stdin (see prompt_via_stdin). "-p", "--output-format", diff --git a/src/secfoo/agents/codex.py b/src/secfoo/agents/codex.py index a8378b3..f8bed34 100644 --- a/src/secfoo/agents/codex.py +++ b/src/secfoo/agents/codex.py @@ -43,6 +43,7 @@ class CodexAdapter(AgentAdapter): name = "codex" binary = "codex" default_timeout_seconds = 1800 + model_flag = "--model" # `codex exec -` reads the prompt from stdin; passing it as argv breaks # on Windows, where `codex` is a `.cmd` shim and cmd.exe truncates the # argument at its first newline. @@ -68,6 +69,7 @@ def build_command(self, prompt: str, *, workdir: Path) -> list[str]: return [ self.binary, "exec", + *self.model_arguments(), # Read-only filesystem: the assessment only reads the target. "--sandbox", "read-only", diff --git a/src/secfoo/agents/copilot.py b/src/secfoo/agents/copilot.py index 522d65c..31b36dc 100644 --- a/src/secfoo/agents/copilot.py +++ b/src/secfoo/agents/copilot.py @@ -82,6 +82,7 @@ class CopilotAdapter(AgentAdapter): name = "copilot" binary = "copilot" default_timeout_seconds = 1800 + model_flag = "--model" # Piped stdin instead of `-p `: on Windows `copilot` is a `.bat` # shim, and cmd.exe cut a `-p` prompt off at its first newline # (reproduced Sep 2026, CLI 1.0.89). With stdin piped and no `-p`, the @@ -89,7 +90,7 @@ class CopilotAdapter(AgentAdapter): prompt_via_stdin = True def build_command(self, prompt: str, *, workdir: Path) -> list[str]: - return [self.binary, "--output-format", "json", "--allow-all-tools"] + return [self.binary, *self.model_arguments(), "--output-format", "json", "--allow-all-tools"] def extract_report(self, stdout: str) -> str: report = None diff --git a/src/secfoo/agents/cursor.py b/src/secfoo/agents/cursor.py index 66ab515..fdccb83 100644 --- a/src/secfoo/agents/cursor.py +++ b/src/secfoo/agents/cursor.py @@ -21,6 +21,7 @@ class CursorAdapter(AgentAdapter): name = "agent" binary = "agent" default_timeout_seconds = 900 + model_flag = "--model" def build_command(self, prompt: str, *, workdir: Path) -> list[str]: - return [self.binary, "-p", prompt] + return [self.binary, *self.model_arguments(), "-p", prompt] diff --git a/src/secfoo/agents/droid.py b/src/secfoo/agents/droid.py index b6fe50e..772a237 100644 --- a/src/secfoo/agents/droid.py +++ b/src/secfoo/agents/droid.py @@ -52,11 +52,13 @@ class DroidAdapter(AgentAdapter): name = "droid" binary = "droid" default_timeout_seconds = 1800 + model_flag = "-m" def build_command(self, prompt: str, *, workdir: Path) -> list[str]: return [ self.binary, "exec", + *self.model_arguments(), "--output-format", "json", "--cwd", diff --git a/src/secfoo/agents/gemini.py b/src/secfoo/agents/gemini.py index f864cbb..877f089 100644 --- a/src/secfoo/agents/gemini.py +++ b/src/secfoo/agents/gemini.py @@ -22,6 +22,7 @@ class GeminiAdapter(AgentAdapter): name = "gemini" binary = "gemini" default_timeout_seconds = 1800 + model_flag = "--model" def build_command(self, prompt: str, *, workdir: Path) -> list[str]: # Deliberately no --yolo: a read-only review never needs @@ -31,7 +32,7 @@ def build_command(self, prompt: str, *, workdir: Path) -> list[str]: # --skip-trust: secfoo always runs headless against arbitrary # targets (cloned repos, user paths) — without this, Gemini CLI # exits immediately in non-interactive mode on an untrusted cwd. - return [self.binary, "-p", prompt, "--output-format", "json", "--skip-trust"] + return [self.binary, *self.model_arguments(), "-p", prompt, "--output-format", "json", "--skip-trust"] def extract_report(self, stdout: str) -> str: try: diff --git a/src/secfoo/cli.py b/src/secfoo/cli.py index 4c0fff6..69cf57d 100644 --- a/src/secfoo/cli.py +++ b/src/secfoo/cli.py @@ -201,6 +201,7 @@ def run( help="Which agent CLI runs the assessment. Falls back to [defaults].agent in " "~/.secfoo/config.toml, then to claude.", ), + model: str | None = typer.Option(None, "--model", help="Model for this run; overrides [agents.].model."), depth: Optional[DepthId] = typer.Option( None, "--depth", @@ -306,7 +307,8 @@ def run( app_id = Prompt.ask("Application ID (optional)", default="", console=console) or None try: - file_defaults = load_config().defaults + file_config = load_config() + file_defaults = file_config.defaults except ConfigError as exc: err_console.print(f"[red]Error in {CONFIG_PATH}:[/] {exc}") raise typer.Exit(code=1) from None @@ -316,6 +318,11 @@ def run( if agent is None: agent = AgentId(file_defaults.agent) if file_defaults.agent else AgentId.CLAUDE + if model is not None and not model.strip(): + err_console.print("[red]Error:[/] --model must be a non-empty string") + raise typer.Exit(code=1) + if model is None: + model = file_config.agent_models.get(agent.value) if depth is None: depth = DepthId(file_defaults.depth) if file_defaults.depth else DepthId.QUICK if timeout is None: @@ -378,6 +385,7 @@ def handle_complete(skill_name: str, result_status: str) -> None: target=target, confluence_urls=confluence, agent_id=agent.value, + model=model, depth=depth.value, timeout=timeout, on_skill_start=handle_start, diff --git a/src/secfoo/config.example.toml b/src/secfoo/config.example.toml index 42d82db..fbfa3ef 100644 --- a/src/secfoo/config.example.toml +++ b/src/secfoo/config.example.toml @@ -25,6 +25,13 @@ depth = "quick" # quick | standard # cost noticeably on large trees. # exclude = ["vendor/", "third_party/", "some-cloned-repo/"] +# Optional per-agent model defaults. --model overrides one run; no CLI global +# settings are changed. Use the registered agent ID (agent = Cursor). +# [agents.claude] +# model = "sonnet" +# [agents.agent] +# model = "your-cursor-model-id" + [memory] # secfoo-memory: a hosted service of curated vulnerability patterns the # agent can consult mid-assessment (known trigger conditions, remediation, diff --git a/src/secfoo/runner.py b/src/secfoo/runner.py index a7e194b..5ed33fe 100644 --- a/src/secfoo/runner.py +++ b/src/secfoo/runner.py @@ -413,6 +413,7 @@ def _run_single_skill( assessment_id: int | None = None, exclude_paths: list[str] | None = None, stack: Stack | None = None, + model: str | None = None, ) -> RunOutcome: skill = load_skill(skill_id) memory_enabled = memory.is_active_for_agent(agent_id) @@ -431,6 +432,7 @@ def _run_single_skill( agent_id=agent_id, confluence_urls=confluence_urls, assessment_id=assessment_id, + model=model, ) started = time.monotonic() @@ -448,6 +450,8 @@ def _run_single_skill( on_skill_start(skill.name) adapter = get_adapter(agent_id) + if model is not None: + adapter.configure_model(model) # Memory Bank: look up the last successful commit so the adapter can diff and send only changed files. prev_commit = repo.get_last_commit(project_id=project_id, skill_id=skill.id) result = adapter.run(prompt, workdir=target_ctx.local_path, timeout=timeout, prev_commit=prev_commit) @@ -538,6 +542,7 @@ def execute_runs( target: str | None, confluence_urls: list[str], agent_id: str, + model: str | None = None, depth: Literal["quick", "standard"] = "quick", timeout: int | None = None, repo: RunRepository | None = None, @@ -637,6 +642,7 @@ def execute_runs( skill_id=skill_id, project_id=project_id, agent_id=agent_id, + model=model, target_ctx=target_ctx, confluence_urls=confluence_urls, depth=depth, diff --git a/src/secfoo/settings.py b/src/secfoo/settings.py index 9d0ba47..e5bca37 100644 --- a/src/secfoo/settings.py +++ b/src/secfoo/settings.py @@ -69,6 +69,7 @@ class SecfooConfig: # once a memory key exists (secfoo memory signup/login). On by default; # without a key there is nothing to wire in either way. memory_enabled: bool = True + agent_models: dict[str, str] = field(default_factory=dict) def _parse_mcp_server(name: str, raw: dict) -> MCPServerConfig: @@ -119,6 +120,19 @@ def load_config(path: Path | None = None) -> SecfooConfig: max_cost_usd=max_cost_usd, ) + agents_raw = data.get("agents", {}) + if not isinstance(agents_raw, dict): + raise ConfigError("[agents] must be a table") + agent_models = {} + for agent_id, options in agents_raw.items(): + if not isinstance(options, dict): + raise ConfigError(f"agents.{agent_id} must be a table") + model = options.get("model") + if model is not None: + if not isinstance(model, str) or not model.strip(): + raise ConfigError(f"agents.{agent_id}.model must be a non-empty string") + agent_models[agent_id] = model + servers = [] seen_names: set[str] = set() for entry in data.get("mcp_servers", []): @@ -137,4 +151,4 @@ def load_config(path: Path | None = None) -> SecfooConfig: if not isinstance(memory_enabled, bool): raise ConfigError("memory.enabled must be true or false") - return SecfooConfig(defaults=defaults, mcp_servers=servers, memory_enabled=memory_enabled) + return SecfooConfig(defaults=defaults, mcp_servers=servers, memory_enabled=memory_enabled, agent_models=agent_models) diff --git a/src/secfoo/storage/models.py b/src/secfoo/storage/models.py index a2e29d6..499a87f 100644 --- a/src/secfoo/storage/models.py +++ b/src/secfoo/storage/models.py @@ -41,6 +41,7 @@ class RunRecord: output_tokens: int | None = None cost_usd: float | None = None project_display_name: str | None = None + model: str | None = None target_commit: str | None = None # Memory Bank: git HEAD SHA at scan time. memory_enabled: int | None = None # 1/0: secfoo-memory tools wired in; NULL = unknown (older run). # Memory Bank: paths an incremental scan covered; None = full scan. diff --git a/src/secfoo/storage/repository.py b/src/secfoo/storage/repository.py index 13ed2c2..aa08b5a 100644 --- a/src/secfoo/storage/repository.py +++ b/src/secfoo/storage/repository.py @@ -60,6 +60,7 @@ def use_db_path(path: Path): # adds them there too, idempotently, so upgrading never requires wiping a # user's local store. _RUNS_MIGRATIONS = { + "model": "ALTER TABLE runs ADD COLUMN model TEXT", "assessment_id": "ALTER TABLE runs ADD COLUMN assessment_id INTEGER REFERENCES assessments(id)", "critical_count": "ALTER TABLE runs ADD COLUMN critical_count INTEGER NOT NULL DEFAULT 0", "high_count": "ALTER TABLE runs ADD COLUMN high_count INTEGER NOT NULL DEFAULT 0", @@ -257,12 +258,13 @@ def create_run( agent_id: str, confluence_urls: list[str], assessment_id: int | None = None, + model: str | None = None, ) -> str: run_uuid = str(uuid.uuid4()) self._conn.execute( "INSERT INTO runs (run_uuid, project_id, assessment_id, skill_id, skill_name, agent_id, " - "confluence_urls, status, started_at) VALUES (?, ?, ?, ?, ?, ?, ?, 'running', ?)", - (run_uuid, project_id, assessment_id, skill_id, skill_name, agent_id, json.dumps(confluence_urls), _now()), + "confluence_urls, status, started_at, model) VALUES (?, ?, ?, ?, ?, ?, ?, 'running', ?, ?)", + (run_uuid, project_id, assessment_id, skill_id, skill_name, agent_id, json.dumps(confluence_urls), _now(), model), ) self._conn.commit() return run_uuid diff --git a/src/secfoo/storage/schema.sql b/src/secfoo/storage/schema.sql index 4063277..cb9587b 100644 --- a/src/secfoo/storage/schema.sql +++ b/src/secfoo/storage/schema.sql @@ -43,6 +43,7 @@ CREATE TABLE IF NOT EXISTS runs ( skill_id TEXT NOT NULL, skill_name TEXT NOT NULL, agent_id TEXT NOT NULL, + model TEXT, -- requested model; NULL = CLI default/unknown confluence_urls TEXT NOT NULL DEFAULT '[]', status TEXT NOT NULL CHECK (status IN ('pending','running','success','failed','timeout','binary_not_found')), diff --git a/tests/test_agents/test_api.py b/tests/test_agents/test_api.py index ba33fd6..1efe508 100644 --- a/tests/test_agents/test_api.py +++ b/tests/test_agents/test_api.py @@ -180,3 +180,17 @@ def test_reformat_is_attempted_only_once(tmp_path, monkeypatch): assert result.status == "success" assert result.raw_report == "no rating here" assert len(fake.calls) == api.MAX_ATTEMPTS + + +def test_selected_model_overrides_api_environment_without_changing_it(tmp_path, monkeypatch): + import os + + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + monkeypatch.setenv("SECFOO_API_MODEL", "environment-model") + (tmp_path / "app.py").write_text("pass\n") + fake = _fake_litellm(monkeypatch) + adapter = ApiAdapter() + adapter.configure_model("selected-model") + assert adapter.run("Review this.", workdir=tmp_path).status == "success" + assert fake.calls[0]["model"] == "selected-model" + assert os.environ["SECFOO_API_MODEL"] == "environment-model" diff --git a/tests/test_agents/test_models.py b/tests/test_agents/test_models.py new file mode 100644 index 0000000..e9f7e35 --- /dev/null +++ b/tests/test_agents/test_models.py @@ -0,0 +1,49 @@ +"""Model selection stays per adapter and preserves execution boundaries.""" +from __future__ import annotations + +import pytest + +from secfoo.agents.registry import ADAPTERS, get_adapter + +MODEL_FLAGS = { + "claude": "--model", "codex": "--model", "gemini": "--model", + "copilot": "--model", "agent": "--model", "droid": "-m", +} + + +@pytest.mark.parametrize("agent_id", ADAPTERS) +def test_model_selection_command_contract(agent_id, tmp_path): + adapter = get_adapter(agent_id) + baseline = adapter.build_command("review this\ncarefully", workdir=tmp_path) + adapter.configure_model(None) + assert adapter.build_command("review this\ncarefully", workdir=tmp_path) == baseline + model = "custom-model;$(touch sentinel)" + if agent_id in MODEL_FLAGS: + adapter.configure_model(model) + command = adapter.build_command("review this\ncarefully", workdir=tmp_path) + i = command.index(MODEL_FLAGS[agent_id]) + assert command[i + 1] == model + assert command[:i] + command[i + 2:] == baseline + assert get_adapter(agent_id).model is None + elif agent_id == "api": + adapter.configure_model(model) + assert adapter.model == model + assert adapter.build_command("review", workdir=tmp_path) == [] + else: + with pytest.warns(UserWarning, match="ignoring model"): + adapter.configure_model(model) + assert adapter.model is None + assert adapter.build_command("review this\ncarefully", workdir=tmp_path) == baseline + + +@pytest.mark.parametrize("agent_id", MODEL_FLAGS) +def test_selected_model_preserves_stdin_and_process_options(agent_id, tmp_path, fake_popen): + adapter = get_adapter(agent_id) + adapter.configure_model("test-model") + fake = fake_popen() + adapter.run("multi\nline prompt", workdir=tmp_path, timeout=23) + command = fake.call_args[0] + assert command[command.index(MODEL_FLAGS[agent_id]) + 1] == "test-model" + assert fake.call_kwargs["cwd"] == tmp_path + assert fake.call_kwargs["start_new_session"] is True + assert fake.stdin_input == ("multi\nline prompt" if adapter.prompt_via_stdin else None) diff --git a/tests/test_cli.py b/tests/test_cli.py index acdd913..cb91f3c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -587,3 +587,29 @@ def test_mcp_sync_codex_is_reported_unsupported(monkeypatch): result = runner.invoke(app, ["mcp", "sync", "--agent", "codex"]) assert result.exit_code == 1 assert "codex" in result.output + + +@pytest.mark.parametrize("override,expected", [(None, "config-model"), ("override-model", "override-model")]) +def test_run_model_override_precedence(monkeypatch, override, expected): + captured = {} + config = SecfooConfig(defaults=Defaults(agent="codex"), mcp_servers=[], agent_models={"codex": "config-model", "claude": "other-model"}) + monkeypatch.setattr("secfoo.cli.load_config", lambda: config) + def execute(**kwargs): + captured.update(kwargs) + return [_outcome("success")] + monkeypatch.setattr("secfoo.cli.execute_runs", execute) + args = ["run", "--skill", "sast"] + if override is not None: + args += ["--model", override] + result = runner.invoke(app, args) + assert result.exit_code == 0, result.output + assert captured["model"] == expected + assert captured["agent_id"] == "codex" + assert config.agent_models["codex"] == "config-model" + + +def test_run_empty_model_rejected(monkeypatch): + monkeypatch.setattr("secfoo.cli.execute_runs", lambda **kw: pytest.fail("must not run")) + result = runner.invoke(app, ["run", "--skill", "sast", "--model", " "]) + assert result.exit_code == 1 + assert "non-empty string" in result.output diff --git a/tests/test_runner.py b/tests/test_runner.py index 10da493..1fed535 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -870,3 +870,22 @@ def test_execute_runs_does_not_touch_sast_findings_for_non_sast_skills(tmp_path, findings = repo.list_sast_findings() repo.close() assert findings == [] + + +def test_runner_applies_and_persists_selected_model(tmp_path, monkeypatch): + _patch_runner(monkeypatch, tmp_path) + observed = [] + + class ModelAdapter(_SleepyAdapter): + model_flag = "--model" + + def run(self, prompt, *, workdir, timeout=None, prev_commit=None): + observed.append(self.model) + return super().run(prompt, workdir=workdir, timeout=timeout, prev_commit=prev_commit) + + monkeypatch.setattr("secfoo.runner.get_adapter", lambda agent_id: ModelAdapter()) + repo = RunRepository(db_path=tmp_path / "models.sqlite") + outcomes = execute_runs(skill_ids=["sast"], target=str(tmp_path), confluence_urls=[], agent_id="claude", model="chosen-model", repo=repo) + assert observed == ["chosen-model"] + assert repo.get_run(outcomes[0].run_uuid).model == "chosen-model" + repo.close() diff --git a/tests/test_settings.py b/tests/test_settings.py index a78c98c..cc81b8a 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -125,3 +125,17 @@ def test_load_config_invalid_toml_raises_config_error(tmp_path): path.write_text("this is not [ valid toml") with pytest.raises(ConfigError): load_config(path) + + +def test_per_agent_models(tmp_path): + path = tmp_path / "config.toml" + path.write_text('[agents.claude]\nmodel = "claude-model"\n[agents.agent]\nmodel = "cursor-model"\n') + assert load_config(path).agent_models == {"claude": "claude-model", "agent": "cursor-model"} + + +@pytest.mark.parametrize("value", ['""', '" "', '42', 'false', '[]']) +def test_invalid_agent_model_is_rejected(tmp_path, value): + path = tmp_path / "config.toml" + path.write_text(f"[agents.claude]\nmodel = {value}\n") + with pytest.raises(ConfigError, match="non-empty string"): + load_config(path) diff --git a/tests/test_storage/test_repository.py b/tests/test_storage/test_repository.py index 5ee341a..8025eb0 100644 --- a/tests/test_storage/test_repository.py +++ b/tests/test_storage/test_repository.py @@ -204,3 +204,24 @@ def test_existing_db_without_assessment_uuid_column_migrates_cleanly(tmp_path): assessment_id = repo.create_assessment(project_id=project_id, assessment_type="internal") assert repo.get_assessment(assessment_id).assessment_uuid is not None repo.close() + + +def test_run_model_roundtrip_and_existing_store_upgrade(tmp_path): + import sqlite3 + + db_path = tmp_path / "models.sqlite" + repo = RunRepository(db_path=db_path) + project_id = repo.upsert_project(identifier="/models", display_name="models", kind="local") + old_uuid = repo.create_run(project_id=project_id, skill_id="sast", skill_name="SAST", agent_id="claude", confluence_urls=[]) + repo.close() + # Simulate the preceding schema, where runs had no model column. + with sqlite3.connect(db_path) as connection: + connection.execute("ALTER TABLE runs DROP COLUMN model") + repo = RunRepository(db_path=db_path) + assert repo.get_run(old_uuid).model is None + new_uuid = repo.create_run(project_id=project_id, skill_id="sast", skill_name="SAST", agent_id="claude", confluence_urls=[], model="selected-model") + repo.close() + repo = RunRepository(db_path=db_path) + assert repo.get_run(new_uuid).model == "selected-model" + assert next(r for r in repo.list_runs() if r.run_uuid == new_uuid).model == "selected-model" + repo.close()