diff --git a/README.md b/README.md index c158e4d..9e0ab63 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,7 @@ Global options (before the command): | `-l`, `--language` | Output language for agent comments (`english`, `japanese`) | | `-r`, `--runner` | Runner to execute the task (default: `claude`) | | `--effort` | Claude thinking effort (`low`, `medium`, `high`, `xhigh`, `max`) | +| `--model` | Claude model (`opus`, `sonnet`, `haiku`, `inherit`) | | `--max-thinking-tokens N` | Thinking token budget | | `--disable-thinking` | Force-disable extended thinking | | `--[no-]disable-adaptive-thinking` | Toggle adaptive reasoning (Opus 4.6, Sonnet 4.6) | @@ -138,6 +139,7 @@ Command-specific options: | `ASKCC_HOME` | Root directory for askcc configuration, templates, and logs | `~/.askcc` | | `ASKCC_LANGUAGE` | Default output language for agent comments (`english`, `japanese`) | `english` | | `ASKCC_CLAUDE_EFFORT_LEVEL` | Default thinking effort (`low`, `medium`, `high`, `xhigh`, `max`) | `xhigh` | +| `ASKCC_CLAUDE_MODEL` | Claude model (`opus`, `sonnet`, `haiku`, `inherit`) | (frontmatter) | | `ASKCC_CLAUDE_MAX_THINKING_TOKENS` | Thinking token budget | `21000` | | `ASKCC_CLAUDE_DISABLE_THINKING` | Force-disable extended thinking (`1`/`true`) | `false` | | `ASKCC_CLAUDE_DISABLE_ADAPTIVE_THINKING` | Disable adaptive reasoning (Opus 4.6, Sonnet 4.6) | `true` | @@ -235,12 +237,12 @@ Note: askcc runs `claude` with `--dangerously-skip-permissions` so it can execut ##### Override Precedence -For `effort` and `max_thinking_tokens`, askcc resolves the effective value in this order (highest wins): +For `effort`, `max_thinking_tokens`, and `model`, askcc resolves the effective value in this order (highest wins): -1. Explicit CLI flag (`--effort`, `--max-thinking-tokens`) -2. Environment variable (`ASKCC_CLAUDE_EFFORT_LEVEL`, `ASKCC_CLAUDE_MAX_THINKING_TOKENS`) +1. Explicit CLI flag (`--effort`, `--max-thinking-tokens`, `--model`) +2. Environment variable (`ASKCC_CLAUDE_EFFORT_LEVEL`, `ASKCC_CLAUDE_MAX_THINKING_TOKENS`, `ASKCC_CLAUDE_MODEL`) 3. Template frontmatter (per-action default in `~/.askcc/templates/`) -4. Built-in default (`xhigh`, `21000`) +4. Built-in default (`xhigh`, `21000`; `model` has no built-in default — when unset everywhere, no `--model` flag is emitted and `claude` picks its own) ### Post-Develop Verification diff --git a/askcc/cli.py b/askcc/cli.py index ed4e07f..2787465 100644 --- a/askcc/cli.py +++ b/askcc/cli.py @@ -8,7 +8,7 @@ from string import Template from . import __version__, settings -from .definitions import AgentAction, AgentConfig +from .definitions import VALID_FRONTMATTER_MODELS, AgentAction, AgentConfig from .functions import ( CheckResult, _parse_issue_url, @@ -83,6 +83,26 @@ def _resolve_max_thinking_tokens(cli_value: int | None, frontmatter_value: int | return settings.DEFAULT_MAX_THINKING_TOKENS +def _resolve_model(cli_value: str | None, frontmatter_value: str | None) -> str | None: + """Precedence: CLI flag > env var > template frontmatter. No built-in default. + + Returns None when no source is set so the caller can suppress the `--model` flag + and let claude pick its own default. + """ + if cli_value is not None: + return cli_value + env_value = os.environ.get("ASKCC_CLAUDE_MODEL", "") + if env_value: + if env_value in VALID_FRONTMATTER_MODELS: + return env_value + logger.warning( + "Invalid ASKCC_CLAUDE_MODEL=%r (valid: %s) — falling through to frontmatter/default", + env_value, + ", ".join(VALID_FRONTMATTER_MODELS), + ) + return frontmatter_value + + def _print_validation_report(issue_url: str, checks: list[CheckResult]) -> None: """Print a structured pass/fail validation report.""" passed_count = sum(1 for c in checks if c.passed) @@ -144,6 +164,14 @@ def main() -> None: # noqa: PLR0912, PLR0915, C901 f"Precedence: CLI > env (ASKCC_CLAUDE_EFFORT_LEVEL) > template frontmatter > " f"built-in default ({settings.DEFAULT_EFFORT_LEVEL}).", ) + parser.add_argument( + "--model", + choices=VALID_FRONTMATTER_MODELS, + default=None, + help="Claude model. " + "Precedence: CLI > env (ASKCC_CLAUDE_MODEL) > template frontmatter > " + "claude's built-in default.", + ) parser.add_argument( "--max-thinking-tokens", type=int, @@ -285,6 +313,7 @@ def main() -> None: # noqa: PLR0912, PLR0915, C901 runner = get_runner(args.runner) effort_level = _resolve_effort(args.effort, config.effort) max_thinking_tokens = _resolve_max_thinking_tokens(args.max_thinking_tokens, config.max_thinking_tokens) + model = _resolve_model(args.model, config.model) try: try: return_code, usage = runner.run( @@ -296,6 +325,7 @@ def main() -> None: # noqa: PLR0912, PLR0915, C901 max_thinking_tokens=max_thinking_tokens, disable_thinking=args.disable_thinking, disable_adaptive_thinking=args.disable_adaptive_thinking, + model=model, ) except OAuthTokenNotFoundError as e: logger.error("[%s] %s", issue_url, e) # noqa: TRY400 diff --git a/askcc/runners.py b/askcc/runners.py index 124164d..ff3760e 100644 --- a/askcc/runners.py +++ b/askcc/runners.py @@ -42,15 +42,21 @@ def run( max_thinking_tokens: int | None = None, disable_thinking: bool = False, disable_adaptive_thinking: bool = False, + model: str | None = None, ) -> tuple[int, dict | None]: """Execute a prompt and return (exit_code, usage_dict_or_none).""" -def _frontmatter_cli_flags(config: AgentConfig) -> list[str]: - """Translate AgentConfig frontmatter fields into claude CLI flags.""" +def _frontmatter_cli_flags(config: AgentConfig, model: str | None = None) -> list[str]: + """Translate AgentConfig frontmatter fields into claude CLI flags. + + `model` overrides `config.model` when supplied (used by the CLI > env > frontmatter + precedence chain resolved in `askcc.cli`). + """ flags: list[str] = [] - if config.model: - flags.extend(["--model", config.model]) + effective_model = model if model is not None else config.model + if effective_model: + flags.extend(["--model", effective_model]) if config.tools: flags.extend(["--allowedTools", ",".join(config.tools)]) if config.disallowed_tools: @@ -167,6 +173,7 @@ def run( max_thinking_tokens: int | None = None, disable_thinking: bool = False, disable_adaptive_thinking: bool = False, + model: str | None = None, ) -> tuple[int, dict | None]: agent_definition = {config.action_name: {"description": config.description, "prompt": config.system_prompt}} @@ -183,7 +190,7 @@ def run( ] if effort_level: cmd.extend(["--effort", effort_level]) - cmd.extend(_frontmatter_cli_flags(config)) + cmd.extend(_frontmatter_cli_flags(config, model=model)) # Remove CLAUDECODE env var so the child claude process doesn't think it's nested inside Claude Code env = os.environ.copy() diff --git a/tests/test_askcc.py b/tests/test_askcc.py index 398b26f..dcaefa1 100644 --- a/tests/test_askcc.py +++ b/tests/test_askcc.py @@ -9,12 +9,13 @@ import pytest -from askcc.cli import main +from askcc.cli import _resolve_model, main from askcc.definitions import ( AGENT_CONFIGS, DEVELOP_AGENT_PROMPT, FIXCI_AGENT_PROMPT, REVIEWPR_AGENT_PROMPT, + VALID_FRONTMATTER_MODELS, AgentAction, AgentConfig, ) @@ -2700,3 +2701,153 @@ def test_no_frontmatter_flags_when_unset(self): cmd = self._run(self._config()) for flag in ("--model", "--allowedTools", "--disallowedTools", "--max-turns"): assert flag not in cmd + + def test_model_override_emitted(self): + runner = ClaudeRunner() + mock_result = subprocess.CompletedProcess(args=[], returncode=0, stdout="{}", stderr="") + config = self._config(model="opus") + with patch("askcc.runners.subprocess.run", return_value=mock_result) as mock_run: + runner.run("prompt", config=config, issue_url=self.ISSUE_URL, cwd=Path.cwd(), model="sonnet") + cmd = mock_run.call_args[0][0] + assert "--model" in cmd + assert cmd[cmd.index("--model") + 1] == "sonnet" + + def test_model_falls_back_to_config_when_unset(self): + runner = ClaudeRunner() + mock_result = subprocess.CompletedProcess(args=[], returncode=0, stdout="{}", stderr="") + config = self._config(model="opus") + with patch("askcc.runners.subprocess.run", return_value=mock_result) as mock_run: + runner.run("prompt", config=config, issue_url=self.ISSUE_URL, cwd=Path.cwd()) + cmd = mock_run.call_args[0][0] + assert "--model" in cmd + assert cmd[cmd.index("--model") + 1] == "opus" + + +class TestModelPrecedence: + """Tests for CLI > env > frontmatter > unset precedence on the `model` field.""" + + ISSUE_URL = "https://github.com/monkut/askcc-cli/issues/1" + + def _run_main(self, args: list[str], frontmatter_model: str | None = "opus") -> MagicMock: + mock_runner = _mock_runner() + base = AGENT_CONFIGS[AgentAction.PLAN] + fake_config = AgentConfig( + action_name=base.action_name, + description=base.description, + system_prompt="body", + user_prompt_template=base.user_prompt_template, + system_prompt_file=base.system_prompt_file, + user_prompt_file=base.user_prompt_file, + required_variables=base.required_variables, + model=frontmatter_model, + ) + with ( + patch("askcc.cli.bootstrap_templates"), + patch("askcc.cli.validate_issue_labels", return_value=[]), + patch("askcc.cli.fetch_github_issue", return_value="issue body"), + patch("askcc.cli.load_agent_config", return_value=fake_config), + patch("askcc.cli.transition_issue_to_development"), + patch("askcc.cli.get_runner", return_value=mock_runner), + patch("sys.argv", ["askcc", *args, "plan", "-g", self.ISSUE_URL]), + pytest.raises(SystemExit), + ): + main() + return mock_runner + + def test_cli_overrides_frontmatter_and_env(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("ASKCC_CLAUDE_MODEL", "haiku") + runner = self._run_main(["--model", "sonnet"], frontmatter_model="opus") + assert runner.run.call_args.kwargs["model"] == "sonnet" + + def test_env_overrides_frontmatter(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("ASKCC_CLAUDE_MODEL", "sonnet") + runner = self._run_main([], frontmatter_model="opus") + assert runner.run.call_args.kwargs["model"] == "sonnet" + + def test_frontmatter_used_when_no_cli_or_env(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.delenv("ASKCC_CLAUDE_MODEL", raising=False) + runner = self._run_main([], frontmatter_model="opus") + assert runner.run.call_args.kwargs["model"] == "opus" + + def test_none_when_no_cli_env_or_frontmatter(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.delenv("ASKCC_CLAUDE_MODEL", raising=False) + runner = self._run_main([], frontmatter_model=None) + assert runner.run.call_args.kwargs["model"] is None + + +class TestModelCLIFlags: + """Tests for the --model CLI flag and ASKCC_CLAUDE_MODEL env var forwarding.""" + + ISSUE_URL = "https://github.com/monkut/askcc-cli/issues/1" + + def _run_main_with_args(self, extra_args: list[str]) -> MagicMock: + mock_runner = _mock_runner() + with ( + patch("askcc.cli.bootstrap_templates"), + patch("askcc.cli.validate_issue_labels", return_value=[]), + patch("askcc.cli.fetch_github_issue", return_value="issue body"), + patch("askcc.cli.get_runner", return_value=mock_runner), + patch("sys.argv", ["askcc", *extra_args, "plan", "-g", self.ISSUE_URL]), + pytest.raises(SystemExit), + ): + main() + return mock_runner + + def test_model_flag_passed_to_runner(self): + mock_runner = self._run_main_with_args(["--model", "sonnet"]) + assert mock_runner.run.call_args.kwargs["model"] == "sonnet" + + def test_model_env_passed_to_runner(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("ASKCC_CLAUDE_MODEL", "sonnet") + mock_runner = self._run_main_with_args([]) + assert mock_runner.run.call_args.kwargs["model"] == "sonnet" + + def test_model_flag_invalid_rejected_by_argparse(self): + with ( + pytest.raises(SystemExit, match="2"), + patch("sys.argv", ["askcc", "--model", "opuz", "plan", "-g", self.ISSUE_URL]), + ): + main() + + +class TestResolveModel: + """Tests for the _resolve_model helper (CLI > env > frontmatter > None).""" + + def test_cli_value_wins(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("ASKCC_CLAUDE_MODEL", "haiku") + assert _resolve_model("sonnet", "opus") == "sonnet" + + def test_env_value_used_when_no_cli(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("ASKCC_CLAUDE_MODEL", "sonnet") + assert _resolve_model(None, "opus") == "sonnet" + + def test_frontmatter_value_used_when_no_cli_or_env(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.delenv("ASKCC_CLAUDE_MODEL", raising=False) + assert _resolve_model(None, "opus") == "opus" + + def test_returns_none_when_all_unset(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.delenv("ASKCC_CLAUDE_MODEL", raising=False) + assert _resolve_model(None, None) is None + + def test_invalid_askcc_claude_model_logged_and_ignored( + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ): + monkeypatch.setenv("ASKCC_CLAUDE_MODEL", "opuz") + with caplog.at_level("WARNING", logger="askcc.cli"): + result = _resolve_model(None, "opus") + assert result == "opus" + assert "Invalid ASKCC_CLAUDE_MODEL" in caplog.text + + def test_invalid_env_with_no_frontmatter_returns_none( + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ): + monkeypatch.setenv("ASKCC_CLAUDE_MODEL", "opuz") + with caplog.at_level("WARNING", logger="askcc.cli"): + result = _resolve_model(None, None) + assert result is None + assert "Invalid ASKCC_CLAUDE_MODEL" in caplog.text + + def test_valid_models_match_frontmatter_choice_set(self): + # _resolve_model accepts the same choice set used for frontmatter validation + for valid in VALID_FRONTMATTER_MODELS: + assert _resolve_model(valid, None) == valid