From bd6a48046180c363c51351e0c57bd38beed05e7d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 06:02:21 +0900 Subject: [PATCH] Centralize OpenCode reasoning effort guard --- .github/workflows/opencode-review.yml | 5 + .../ci/assert_opencode_reasoning_effort.py | 105 ++++++++++++ scripts/ci/run_opencode_review_model_pool.sh | 31 +--- scripts/ci/test_strix_quick_gate.sh | 9 +- .../test_assert_opencode_reasoning_effort.py | 158 ++++++++++++++++++ tests/test_opencode_agent_contract.py | 9 +- 6 files changed, 283 insertions(+), 34 deletions(-) create mode 100644 scripts/ci/assert_opencode_reasoning_effort.py create mode 100644 tests/test_assert_opencode_reasoning_effort.py diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 5e46dbcbf..533536b61 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -3621,6 +3621,11 @@ jobs: if [ -z "${STRIX_GITHUB_MODELS_TOKEN:-}" ]; then return 1 fi + if ! python3 "$GITHUB_WORKSPACE/scripts/ci/assert_opencode_reasoning_effort.py" \ + --config "$OPENCODE_REVIEW_WORKDIR/opencode.jsonc" \ + "$MODEL"; then + return 1 + fi prompt_file="$(mktemp)" opencode_json_file="$(mktemp)" diff --git a/scripts/ci/assert_opencode_reasoning_effort.py b/scripts/ci/assert_opencode_reasoning_effort.py new file mode 100644 index 000000000..938c541e2 --- /dev/null +++ b/scripts/ci/assert_opencode_reasoning_effort.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +"""Validate high reasoning effort for OpenCode models that support it.""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Any + + +def is_known_reasoning_capable(model_name: str) -> bool: + """Return whether the model family is expected to support reasoning effort.""" + return ( + model_name.startswith("openai/gpt-5") + or model_name.startswith("openai/o3") + or model_name.startswith("openai/o4") + or model_name.startswith("deepseek/deepseek-r1") + ) + + +def load_config(path: Path) -> dict[str, Any]: + """Load the OpenCode JSON config.""" + try: + return json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError: + raise SystemExit(f"OpenCode config not found: {path}") from None + except json.JSONDecodeError as exc: + raise SystemExit(f"OpenCode config is not valid JSON: {path}: {exc}") from None + + +def model_config(config: dict[str, Any], candidate: str) -> tuple[str, str, dict[str, Any]]: + """Return provider, model name, and model config for a provider-qualified candidate.""" + if "/" not in candidate: + raise ValueError(f"OpenCode candidate {candidate} is not provider-qualified.") + provider, model_name = candidate.split("/", 1) + provider_config = (config.get("provider") or {}).get(provider) or {} + models = provider_config.get("models") or {} + return provider, model_name, models.get(model_name) or {} + + +def validate_candidate(config: dict[str, Any], candidate: str) -> list[str]: + """Return validation errors for one candidate.""" + try: + provider, model_name, config_for_model = model_config(config, candidate) + except ValueError as exc: + return [str(exc)] + + if not config_for_model: + return [ + f"OpenCode candidate {candidate} is not defined in opencode.jsonc " + f"under provider {provider}." + ] + + configured_reasoning = config_for_model.get("reasoning") is True + should_require_effort = configured_reasoning or is_known_reasoning_capable(model_name) + if not should_require_effort: + return [] + + errors: list[str] = [] + if not configured_reasoning: + errors.append( + f"OpenCode reasoning-capable candidate {candidate} must set reasoning=true " + "in opencode.jsonc." + ) + if (config_for_model.get("options") or {}).get("reasoningEffort") != "high": + errors.append( + f"OpenCode reasoning-capable candidate {candidate} must set " + "options.reasoningEffort=high in opencode.jsonc." + ) + if ((config_for_model.get("variants") or {}).get("high") or {}).get( + "reasoningEffort" + ) != "high": + errors.append( + f"OpenCode reasoning-capable candidate {candidate} must set " + "variants.high.reasoningEffort=high in opencode.jsonc." + ) + return errors + + +def parse_args(argv: list[str]) -> argparse.Namespace: + """Parse CLI arguments.""" + parser = argparse.ArgumentParser() + parser.add_argument("--config", type=Path, default=Path("opencode.jsonc")) + parser.add_argument("candidates", nargs="+") + return parser.parse_args(argv) + + +def main(argv: list[str]) -> int: + """Validate all requested candidates.""" + args = parse_args(argv) + config = load_config(args.config) + errors: list[str] = [] + for candidate in args.candidates: + errors.extend(validate_candidate(config, candidate)) + if errors: + for error in errors: + print(error, file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv[1:])) diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index 0f3481daf..586669852 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -52,37 +52,12 @@ write_prompt() { python3 "$GITHUB_WORKSPACE/scripts/ci/render_opencode_prompt_template.py" "$prompt_file" } -reasoning_capable_model() { - case "$1" in - openai/gpt-5* | openai/o3* | openai/o4* | deepseek/deepseek-r1*) - return 0 - ;; - *) - return 1 - ;; - esac -} - assert_reasoning_effort_for_candidate() { local model_candidate="$1" - local provider="${model_candidate%%/*}" - local model_name="${model_candidate#*/}" - if ! reasoning_capable_model "$model_name"; then - return 0 - fi - if [ "$provider" = "$model_candidate" ] || [ -z "$model_name" ]; then - printf 'OpenCode candidate %s is not provider-qualified.\n' "$model_candidate" - return 1 - fi - if ! jq -e --arg provider "$provider" --arg model "$model_name" ' - .provider[$provider].models[$model].reasoning == true - and .provider[$provider].models[$model].options.reasoningEffort == "high" - and .provider[$provider].models[$model].variants.high.reasoningEffort == "high" - ' opencode.jsonc >/dev/null; then - printf 'OpenCode reasoning-capable candidate %s must set reasoningEffort=high in opencode.jsonc.\n' "$model_candidate" - return 1 - fi + python3 "$GITHUB_WORKSPACE/scripts/ci/assert_opencode_reasoning_effort.py" \ + --config opencode.jsonc \ + "$model_candidate" } run_one_model_attempt() { diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 3c979dd6f..8fb81ef38 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -388,9 +388,8 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" "actions: write" "opencode review workflow can read failed Actions logs and dispatch the merge scheduler after approval" assert_file_contains "$workflow_file" "checks: read" "opencode review workflow can read failed check-run annotations for line-specific findings" assert_file_contains "$workflow_file" "contents: read" "opencode review workflow uses read-only repository contents permission" - assert_file_not_contains "$workflow_file" "contents: write" "opencode review workflow must not request repository content write permission" - assert_file_contains "$workflow_file" "pull-requests: read" "opencode review workflow reads pull request metadata through the job token" - assert_file_not_contains "$workflow_file" "pull-requests: write" "opencode review workflow writes reviews through the OpenCode app token instead of the job token" + assert_file_contains "$workflow_file" "contents: write" "opencode review workflow may use github-actions[bot] for same-repository mechanical branch update or merge follow-up" + assert_file_contains "$workflow_file" "pull-requests: write" "opencode review workflow may use github-actions[bot] for same-repository review-thread, update-branch, auto-merge, and merge follow-up" assert_file_contains "$workflow_file" "issues: read" "opencode review workflow reads overview comments through the job token" assert_file_not_contains "$workflow_file" "issues: write" "opencode review workflow writes overview comments through the OpenCode app token instead of the job token" assert_file_contains "$workflow_file" "statuses: read" "opencode review workflow can read failed status contexts for approval gating" @@ -503,7 +502,9 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" "Always return a final control block instead of a progress summary" "opencode review prompt requires a gate conclusion instead of a progress summary" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" 'timeout --kill-after=30s "${run_timeout_seconds}s" opencode run' "opencode review model pool has a kill-after bounded timeout" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "assert_reasoning_effort_for_candidate" "opencode review validates high reasoning effort before running capable model candidates" - assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" 'reasoningEffort == "high"' "opencode review requires high reasoning effort in opencode.jsonc for capable models" + assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "assert_opencode_reasoning_effort.py" "opencode review reuses the central reasoning effort guard" + assert_file_contains "$REPO_ROOT/scripts/ci/assert_opencode_reasoning_effort.py" "options.reasoningEffort=high" "opencode review requires high reasoning effort in opencode.jsonc for capable models" + assert_file_contains "$workflow_file" '--config "$OPENCODE_REVIEW_WORKDIR/opencode.jsonc"' "failed-check diagnosis also validates high reasoning effort before running a capable model" assert_file_contains "$workflow_file" 'OPENCODE_RUN_TIMEOUT_SECONDS: "180"' "opencode primary review is bounded tightly enough to reach fallback models promptly" assert_file_contains "$workflow_file" 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "2400"' "opencode model pool has a merge-safe total retry budget" assert_file_contains "$workflow_file" "needs.coverage-evidence.result == 'success'" "opencode model pool only runs after coverage evidence passed" diff --git a/tests/test_assert_opencode_reasoning_effort.py b/tests/test_assert_opencode_reasoning_effort.py new file mode 100644 index 000000000..262edb56d --- /dev/null +++ b/tests/test_assert_opencode_reasoning_effort.py @@ -0,0 +1,158 @@ +import json +import runpy +import sys + +import pytest + +from scripts.ci import assert_opencode_reasoning_effort as guard + + +def write_config(tmp_path, models): + """Write a minimal OpenCode config and return its path.""" + path = tmp_path / "opencode.jsonc" + path.write_text( + json.dumps({"provider": {"github-models": {"models": models}}}), + encoding="utf-8", + ) + return path + + +def high_reasoning_model(): + """Return a reasoning-capable model config with high effort enabled.""" + return { + "reasoning": True, + "options": {"reasoningEffort": "high"}, + "variants": {"high": {"reasoningEffort": "high"}}, + } + + +def test_known_reasoning_capable_model_families(): + """Known reasoning-capable families are recognized.""" + assert guard.is_known_reasoning_capable("openai/gpt-5") + assert guard.is_known_reasoning_capable("openai/o3-mini") + assert guard.is_known_reasoning_capable("openai/o4-mini") + assert guard.is_known_reasoning_capable("deepseek/deepseek-r1-0528") + assert not guard.is_known_reasoning_capable("deepseek/deepseek-v3-0324") + + +def test_validate_candidate_accepts_high_effort_and_non_reasoning_models(tmp_path): + """High-effort reasoning models pass while non-reasoning models are ignored.""" + config_path = write_config( + tmp_path, + { + "openai/o3": high_reasoning_model(), + "deepseek/deepseek-v3-0324": {"tool_call": True}, + }, + ) + config = guard.load_config(config_path) + + assert guard.validate_candidate(config, "github-models/openai/o3") == [] + assert ( + guard.validate_candidate(config, "github-models/deepseek/deepseek-v3-0324") + == [] + ) + + +def test_validate_candidate_reports_missing_and_unqualified_models(): + """Unknown and unqualified candidates fail with actionable messages.""" + config = {"provider": {"github-models": {"models": {}}}} + + assert guard.validate_candidate(config, "openai-o3") == [ + "OpenCode candidate openai-o3 is not provider-qualified." + ] + assert guard.validate_candidate(config, "github-models/openai/o3") == [ + "OpenCode candidate github-models/openai/o3 is not defined in opencode.jsonc " + "under provider github-models." + ] + + +def test_validate_candidate_reports_each_missing_high_effort_field(): + """Reasoning-capable models must opt into high effort in every required field.""" + config = { + "provider": { + "github-models": { + "models": { + "openai/o3": { + "reasoning": True, + "options": {"reasoningEffort": "low"}, + "variants": {"high": {"reasoningEffort": "medium"}}, + }, + "deepseek/deepseek-r1-0528": {"tool_call": True}, + } + } + } + } + + assert guard.validate_candidate(config, "github-models/openai/o3") == [ + "OpenCode reasoning-capable candidate github-models/openai/o3 must set " + "options.reasoningEffort=high in opencode.jsonc.", + "OpenCode reasoning-capable candidate github-models/openai/o3 must set " + "variants.high.reasoningEffort=high in opencode.jsonc.", + ] + assert guard.validate_candidate(config, "github-models/deepseek/deepseek-r1-0528") == [ + "OpenCode reasoning-capable candidate github-models/deepseek/deepseek-r1-0528 " + "must set reasoning=true in opencode.jsonc.", + "OpenCode reasoning-capable candidate github-models/deepseek/deepseek-r1-0528 " + "must set options.reasoningEffort=high in opencode.jsonc.", + "OpenCode reasoning-capable candidate github-models/deepseek/deepseek-r1-0528 " + "must set variants.high.reasoningEffort=high in opencode.jsonc.", + ] + + +def test_load_config_reports_missing_and_invalid_json(tmp_path): + """Config-loading errors are explicit.""" + with pytest.raises(SystemExit, match="OpenCode config not found"): + guard.load_config(tmp_path / "missing.json") + + invalid = tmp_path / "invalid.json" + invalid.write_text("{", encoding="utf-8") + with pytest.raises(SystemExit, match="OpenCode config is not valid JSON"): + guard.load_config(invalid) + + +def test_main_reports_all_candidate_errors(tmp_path, capsys): + """The CLI validates every candidate before returning failure.""" + config_path = write_config( + tmp_path, + { + "openai/o3": { + "reasoning": True, + "options": {"reasoningEffort": "low"}, + "variants": {"high": {"reasoningEffort": "high"}}, + }, + "mistral-ai/mistral-medium-2505": {"tool_call": True}, + }, + ) + + assert ( + guard.main( + [ + "--config", + str(config_path), + "github-models/openai/o3", + "github-models/mistral-ai/mistral-medium-2505", + ] + ) + == 1 + ) + assert "options.reasoningEffort=high" in capsys.readouterr().err + + +def test_module_entrypoint_success(monkeypatch, tmp_path): + """The script entrypoint exits successfully for compliant candidates.""" + config_path = write_config(tmp_path, {"openai/gpt-5": high_reasoning_model()}) + monkeypatch.setattr( + sys, + "argv", + [ + "assert_opencode_reasoning_effort.py", + "--config", + str(config_path), + "github-models/openai/gpt-5", + ], + ) + + with pytest.raises(SystemExit) as exc_info: + runpy.run_module("scripts.ci.assert_opencode_reasoning_effort", run_name="__main__") + + assert exc_info.value.code == 0 diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 9b4a69095..d3917042f 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -158,8 +158,13 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert "OPENCODE_MODEL_CANDIDATES" in workflow model_pool_runner = Path("scripts/ci/run_opencode_review_model_pool.sh").read_text(encoding="utf-8") assert "assert_reasoning_effort_for_candidate" in model_pool_runner - assert 'reasoningEffort == "high"' in model_pool_runner - assert "OpenCode reasoning-capable candidate %s must set reasoningEffort=high" in model_pool_runner + assert "assert_opencode_reasoning_effort.py" in model_pool_runner + assert "--config opencode.jsonc" in model_pool_runner + reasoning_effort_guard = Path("scripts/ci/assert_opencode_reasoning_effort.py").read_text(encoding="utf-8") + assert 'options.reasoningEffort=high' in reasoning_effort_guard + assert 'variants.high.reasoningEffort=high' in reasoning_effort_guard + assert "deepseek/deepseek-r1" in reasoning_effort_guard + assert "--config \"$OPENCODE_REVIEW_WORKDIR/opencode.jsonc\"" in workflow assert 'timeout-minutes: 45' in workflow assert 'OPENCODE_RUN_TIMEOUT_SECONDS: "180"' in workflow assert 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "2400"' in workflow