diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index b0f38796..93899764 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -36,8 +36,8 @@ concurrency: group: >- opencode-review-${{ github.event_name }}-${{ github.event.pull_request.base.repo.full_name || github.event.inputs.target_repository || github.repository }}-${{ - github.event_name == 'pull_request_target' && format('pr-{0}-{1}', github.event.pull_request.number, github.event.pull_request.head.sha) || - github.event.inputs.pr_number != '' && github.event.inputs.pr_head_sha != '' && format('pr-{0}-{1}', github.event.inputs.pr_number, github.event.inputs.pr_head_sha) || + github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number) || + github.event.inputs.pr_number != '' && format('pr-{0}', github.event.inputs.pr_number) || github.event.inputs.pr_number || github.run_id }} cancel-in-progress: true @@ -2497,12 +2497,6 @@ jobs: fi } - gh_error_is_rate_limited() { - local error_file="$1" - [ -s "$error_file" ] || return 1 - grep -Eiq '(API rate limit exceeded|rate limit exceeded|secondary rate limit)' "$error_file" - } - emit_change_flow_mermaid_graph() { local merge_state="${1:-UNKNOWN}" local changed_files_file surfaces_file idx next_node @@ -2760,6 +2754,18 @@ jobs: APPROVAL_CHECK_WAIT_SLEEP_SECONDS: "30" CHECK_LOOKUP_RETRY_ATTEMPTS: "5" CHECK_LOOKUP_RETRY_SLEEP_SECONDS: "5" + OPENCODE_MODEL_CANDIDATES: "github-models/openai/gpt-5-nano" + OPENCODE_MODEL_ATTEMPTS: "1" + OPENCODE_RUN_TIMEOUT_SECONDS: "240" + OPENCODE_EXPORT_TIMEOUT_SECONDS: "120" + OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "360" + OPENCODE_BACKOFF_INITIAL_SECONDS: "30" + OPENCODE_BACKOFF_MAX_SECONDS: "30" + OPENCODE_FIRST_ATTEMPT_AGENT: ci-review + OPENCODE_AGENT: ci-review-fallback + OPENCODE_EXHAUSTED_REKICK_INITIAL_SLEEP_SECONDS: "15" + OPENCODE_EXHAUSTED_REKICK_MAX_SLEEP_SECONDS: "300" + OPENCODE_EXHAUSTED_REKICK_MAX_TOTAL_SECONDS: "4200" run: | set -euo pipefail echo "::group::OpenCode Review Approval Gate" @@ -3029,26 +3035,12 @@ jobs: fi warn_gh_publication_failure "pull review with fallback review token" "$gh_error_file" fi - if [ "$event" = "APPROVE" ] && gh_error_is_rate_limited "$gh_error_file"; then - rm -f "$gh_error_file" "$review_payload_file" - update_review_overview "$event" "$body" || true - if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then - { - printf '## OpenCode approve review publication skipped\n\n' - printf -- '- Head SHA: `%s`\n' "$HEAD_SHA" - printf -- '- Workflow run: %s\n' "$RUN_ID" - printf -- '- Workflow attempt: %s\n\n' "$RUN_ATTEMPT" - printf 'OpenCode completed the approval gate, but GitHub rejected the pull-review write due to API rate limiting. The required workflow remains successful because failed checks, mergeability, and unresolved review threads were already gated before approval.\n\n' - printf '%s\n' "$body" - } >>"$GITHUB_STEP_SUMMARY" - fi - printf '::warning::OpenCode could not publish the APPROVE pull review for head %s because the GitHub API rate limit was exceeded; keeping the successful approval gate result because pre-approval source, check, mergeability, and review-thread gates passed.\n' "$HEAD_SHA" - return 0 - fi rm -f "$gh_error_file" "$review_payload_file" update_review_overview "$event" "$body" || true printf '::error::OpenCode could not publish the pull review for head %s, so the review state was not changed.\n' "$HEAD_SHA" - echo "::endgroup::" + case "$event" in + REQUEST_CHANGES | INLINE_COMMENT_PUBLISH_FAILED) echo "::endgroup::" ;; + esac exit 1 fi rm -f "$gh_error_file" "$review_payload_file" @@ -4724,6 +4716,64 @@ jobs: scripts/ci/collect_failed_check_evidence.sh "$evidence_file" } + rekick_model_pool_on_exhaustion() { + local rekick_attempt=1 + local rekick_output outcome model rekick_status + local sleep_seconds="${OPENCODE_EXHAUSTED_REKICK_INITIAL_SLEEP_SECONDS:-15}" + local max_sleep_seconds="${OPENCODE_EXHAUSTED_REKICK_MAX_SLEEP_SECONDS:-300}" + local max_total_seconds="${OPENCODE_EXHAUSTED_REKICK_MAX_TOTAL_SECONDS:-4200}" + local started_at now elapsed + + started_at="$(date +%s)" + + while [ "${OPENCODE_MODEL_POOL_OUTCOME:-}" = "exhausted" ]; do + if [ "$max_total_seconds" -gt 0 ]; then + now="$(date +%s)" + elapsed="$((now - started_at))" + if [ "$elapsed" -ge "$max_total_seconds" ]; then + printf 'OpenCode model pool remained exhausted for %s seconds; stopping re-kicks and continuing with fail-closed handling.\n' "$elapsed" >&2 + break + fi + fi + printf 'OpenCode model pool exhausted; re-kicking model pool (attempt %s).\n' "$rekick_attempt" + rekick_output="$(mktemp)" + rekick_status=0 + GITHUB_OUTPUT="$rekick_output" OPENCODE_OUTPUT_FILE="$OPENCODE_MODEL_POOL_OUTPUT_FILE" \ + bash "$GITHUB_WORKSPACE/scripts/ci/run_opencode_review_model_pool.sh" || rekick_status=$? + if [ "$rekick_status" -ne 0 ]; then + printf 'OpenCode model pool re-kick command exited with status %s; continuing with fail-closed handling.\n' "$rekick_status" >&2 + fi + outcome="$(awk -F= '/^review_status=/{v=$2} END{print v}' "$rekick_output")" + model="$(awk -F= '/^review_model=/{v=$2} END{print v}' "$rekick_output")" + rm -f "$rekick_output" + + if [ -z "$outcome" ]; then + printf 'OpenCode model pool re-kick produced no review_status output; treating outcome as exhausted.\n' >&2 + outcome="exhausted" + fi + OPENCODE_MODEL_POOL_OUTCOME="$outcome" + OPENCODE_MODEL_POOL_MODEL="$model" + if [ "$outcome" = "success" ]; then + if [ -z "$model" ]; then + printf 'OpenCode model pool re-kick succeeded but published an empty review_model.\n' >&2 + fi + printf 'OpenCode model pool re-kick recovered with model: %s\n' "${model:-unknown}" + break + fi + if [ "$sleep_seconds" -gt 0 ]; then + printf 'OpenCode model pool still exhausted after re-kick attempt %s; retrying in %s seconds.\n' "$rekick_attempt" "$sleep_seconds" + sleep "$sleep_seconds" + fi + if [ "$sleep_seconds" -lt "$max_sleep_seconds" ]; then + sleep_seconds=$((sleep_seconds * 2)) + if [ "$sleep_seconds" -gt "$max_sleep_seconds" ]; then + sleep_seconds="$max_sleep_seconds" + fi + fi + rekick_attempt=$((rekick_attempt + 1)) + done + } + live_head_sha="$(gh api -X GET "repos/${GH_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.head.sha')" if [ "$live_head_sha" != "$HEAD_SHA" ]; then echo "stale OpenCode run: event head=${HEAD_SHA}, live head=${live_head_sha}; skipping review side effects." @@ -4735,6 +4785,7 @@ jobs: request_changes_for_coverage_evidence_failure fi + rekick_model_pool_on_exhaustion opencode_review_outcome="${OPENCODE_MODEL_POOL_OUTCOME:-unknown}" if [ "$opencode_review_outcome" != "success" ]; then diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 621e4506..cb039dba 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -299,9 +299,6 @@ def call_llm(repo: str, number: int, pr: dict[str, Any], diff: str, truncated: b if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_unspecified: raise ValueError("URL cannot target internal IP addresses") - if not (api_url.startswith("http://") or api_url.startswith("https://")): - raise ValueError(f"NOEMA_LLM_API_URL must start with http:// or https:// to prevent SSRF vulnerabilities, got: {api_url}") - prompt = { "role": "user", "content": "\n".join( diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index f51cfff5..8caa69ce 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -632,7 +632,7 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_not_contains "$workflow_file" "no model produced a valid review control block" "opencode model-failure path no longer documents a final exhausted state" assert_file_contains "$workflow_file" 'OPENCODE_MODEL_ATTEMPTS: "1"' "opencode primary and fallback paths avoid multi-attempt stalls on one model" assert_file_contains "$workflow_file" 'OPENCODE_MODEL_ATTEMPTS: "1"' "opencode catalog fallback tries each model once before moving on" - assert_file_contains "$workflow_file" 'OPENCODE_RUN_TIMEOUT_SECONDS: "600"' "opencode catalog fallback has a bounded model review timeout before step timeout" + assert_file_contains "$workflow_file" 'OPENCODE_RUN_TIMEOUT_SECONDS: "180"' "opencode catalog fallback has a bounded model review timeout before step timeout" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "OpenCode %s attempt %s/%s failed" "opencode catalog fallback records per-model retry failures" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "exponential backoff" "opencode model retry paths use exponential backoff instead of fixed sleeps" assert_file_contains "$workflow_file" "github-models/openai/o4-mini github-models/openai/o3-mini" "opencode review tries compact OpenAI reasoning model fallbacks early" @@ -810,10 +810,10 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" 'warn_gh_publication_failure "initial review overview comment"' "opencode initial overview comment soft-fails permission-denied publication errors" assert_file_contains "$workflow_file" 'warn_gh_publication_failure "pull review with primary review token"' "opencode approval explains primary review publication failures" assert_file_contains "$workflow_file" 'warn_gh_publication_failure "pull review with fallback review token"' "opencode approval explains fallback review publication failures" - assert_file_contains "$workflow_file" 'gh_error_is_rate_limited()' "opencode approval detects rate-limited publication failures" - assert_file_contains "$workflow_file" '[ "$event" = "APPROVE" ] && gh_error_is_rate_limited "$gh_error_file"' "opencode approval only soft-fails rate-limited approve publication failures" - assert_file_contains "$workflow_file" 'OpenCode could not publish the APPROVE pull review for head %s because the GitHub API rate limit was exceeded' "opencode approval keeps successful gate results for rate-limited approval review publication" - assert_file_contains "$workflow_file" 'OpenCode could not publish the pull review for head %s, so the review state was not changed.' "opencode approval fails when review publication fails" + assert_file_contains "$workflow_file" 'OpenCode could not publish the pull review for head %s, so the review state was not changed.' "opencode approval fails closed when review publication fails" + assert_file_contains "$workflow_file" 'REQUEST_CHANGES | INLINE_COMMENT_PUBLISH_FAILED) echo "::endgroup::" ;;' "opencode only closes a review-body log group for events that opened one" + assert_file_not_contains "$workflow_file" 'OpenCode approve review publication skipped' "opencode approval does not report skipped approval publication as success" + assert_file_not_contains "$workflow_file" 'keeping the successful approval gate result' "opencode approval does not soft-pass without publishing an approving review" assert_file_contains "$workflow_file" 'warn_gh_publication_failure "review overview comment"' "opencode approval soft-fails permission-denied overview publication" assert_file_not_contains "$workflow_file" 'gh api -X DELETE "repos/${GH_REPOSITORY}/issues/comments/${comment_id}"' "opencode review must not delete Review Overview gate evidence" assert_file_not_contains "$workflow_file" '--file "$OPENCODE_EVIDENCE_FILE"' "opencode review must not attach evidence content to GitHub Models requests" diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index cc68ff28..8285fceb 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -206,7 +206,7 @@ def test_call_llm_handles_configuration_and_verdicts(monkeypatch): monkeypatch.setenv("NOEMA_LLM_API_URL", "file:///etc/passwd") monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") - with pytest.raises(ValueError, match="must start with http:// or https://"): + with pytest.raises(ValueError, match="URL scheme must be http or https"): noema.call_llm("owner/repo", 1, pr, "diff", False) monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index ece0a6bc..797f41de 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -236,6 +236,9 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert "Run OpenCode PR Review model pool" in workflow assert "opencode_review_model_pool" in workflow assert "run_opencode_review_model_pool.sh" in workflow + assert "rekick_model_pool_on_exhaustion" in workflow + assert "format('pr-{0}', github.event.pull_request.number)" in workflow + assert "format('pr-{0}-{1}', github.event.pull_request.number, github.event.pull_request.head.sha)" not in workflow 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 @@ -280,7 +283,7 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert '"## Review outcome"' in workflow assert '"## Check outcome"' not in workflow assert "publish REQUEST_CHANGES when coverage-evidence blocker states" in workflow - assert re.search(r"opencode-review-target:[\s\S]{0,240}timeout-minutes: 360", workflow) + assert re.search(r"opencode-review-target:[\s\S]{0,360}timeout-minutes: 360", workflow) assert 'timeout-minutes: 75' in workflow assert re.search(r"Run OpenCode PR Review model pool[\s\S]{0,240}timeout-minutes: 350", workflow) assert 'APPROVAL_CHECK_WAIT_ATTEMPTS: "81"' in workflow @@ -305,6 +308,9 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert 'OPENCODE_EXPORT_TIMEOUT_SECONDS: "120"' in workflow assert 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "0"' in workflow assert 'OPENCODE_BACKOFF_MAX_SECONDS: "30"' in workflow + assert 'OPENCODE_EXHAUSTED_REKICK_INITIAL_SLEEP_SECONDS: "15"' in workflow + assert 'OPENCODE_EXHAUSTED_REKICK_MAX_SLEEP_SECONDS: "300"' in workflow + assert 'OPENCODE_EXHAUSTED_REKICK_MAX_TOTAL_SECONDS: "4200"' in workflow assert "while :" in model_pool_runner assert "OpenCode model pool has no configured model candidates." in model_pool_runner assert 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS:-18000' in model_pool_runner @@ -412,7 +418,7 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): assert "steps.scheduler_app_token.outputs.token" in workflow assert "SCHEDULER_READ_TOKEN: ${{ github.token }}" in workflow assert "SCHEDULER_MUTATION_TOKEN_SOURCE" in workflow - assert 'default: "-1"' in workflow + assert 'default: "1"' in workflow assert 'review_dispatch_limit="-1"' in workflow