diff --git a/action.yml b/action.yml index b134aa2..447d8ca 100644 --- a/action.yml +++ b/action.yml @@ -98,6 +98,18 @@ inputs: description: 'Name of secret containing the API key for the configured opencode-provider' required: false default: 'OPENCODE_API_KEY' + fallback-opencode-provider: + description: 'Optional opencode-provider to fall back to when primary exhausts retries on retryable errors (e.g. 429/503/504/timeout). Empty disables fallback.' + required: false + default: '' + fallback-opencode-model: + description: 'Model ID for the fallback opencode-provider (required when fallback-opencode-provider is set)' + required: false + default: '' + fallback-opencode-api-key-secret: + description: 'Name of secret holding the fallback API key. Optional when fallback-opencode-provider equals opencode-provider (primary key is reused). Required when providers differ.' + required: false + default: '' outputs: validation-status: @@ -167,8 +179,32 @@ runs: exit 1 fi + # Validate fallback opencode-provider configuration + FALLBACK_PROV="${{ inputs.fallback-opencode-provider }}" + if [ -n "$FALLBACK_PROV" ]; then + if [ "${{ inputs.provider }}" != "opencode" ]; then + echo "::error::fallback-opencode-provider is only supported when provider=opencode" + exit 1 + fi + if [[ ! "$FALLBACK_PROV" =~ ^(openrouter|anthropic|openai)$ ]]; then + echo "::error::Invalid fallback-opencode-provider. Must be: openrouter, anthropic, or openai" + exit 1 + fi + if [ -z "${{ inputs.fallback-opencode-model }}" ]; then + echo "::error::fallback-opencode-model is required when fallback-opencode-provider is set" + exit 1 + fi + if [ "$FALLBACK_PROV" != "${{ inputs.opencode-provider }}" ] && [ -z "${{ inputs.fallback-opencode-api-key-secret }}" ]; then + echo "::error::fallback-opencode-api-key-secret is required when fallback-opencode-provider differs from opencode-provider" + exit 1 + fi + fi + echo "✅ All inputs validated successfully" echo " Provider: ${{ inputs.provider }}" + if [ -n "$FALLBACK_PROV" ]; then + echo " Fallback: ${FALLBACK_PROV}/${{ inputs.fallback-opencode-model }}" + fi echo "::endgroup::" - name: Set Artifact Name @@ -205,6 +241,15 @@ runs: run: | ${{ github.action_path }}/scripts/setup-opencode.sh + - name: Setup OpenCode Fallback Auth + if: inputs.provider == 'opencode' && inputs.fallback-opencode-provider != '' && inputs.fallback-opencode-api-key-secret != '' + shell: bash + env: + OPENCODE_PROVIDER: ${{ inputs.fallback-opencode-provider }} + OPENCODE_SECRET_NAME: ${{ inputs.fallback-opencode-api-key-secret }} + run: | + ${{ github.action_path }}/scripts/setup-opencode.sh + - name: Setup Git shell: bash env: @@ -326,6 +371,7 @@ runs: PROVIDER: ${{ inputs.provider }} OPENCODE_PROVIDER: ${{ inputs.opencode-provider }} OPENCODE_MODEL: ${{ inputs.opencode-model }} + FALLBACK_OPENCODE_PROVIDER: ${{ inputs.fallback-opencode-provider }} run: | ${{ github.action_path }}/scripts/setup-mcp.sh @@ -352,6 +398,8 @@ runs: GEMINI_MODEL: ${{ inputs.gemini-model }} OPENCODE_PROVIDER: ${{ inputs.opencode-provider }} OPENCODE_MODEL: ${{ inputs.opencode-model }} + FALLBACK_OPENCODE_PROVIDER: ${{ inputs.fallback-opencode-provider }} + FALLBACK_OPENCODE_MODEL: ${{ inputs.fallback-opencode-model }} MAX_RETRIES: ${{ inputs.max-retries }} PR_NUMBER: ${{ github.event.pull_request.number }} PR_TITLE: ${{ github.event.pull_request.title }} diff --git a/scripts/setup-mcp.sh b/scripts/setup-mcp.sh index 69eb3b7..6f98c39 100755 --- a/scripts/setup-mcp.sh +++ b/scripts/setup-mcp.sh @@ -27,12 +27,21 @@ if [ -z "$MCP_URL" ]; then fi if [ "$PROVIDER" = "opencode" ]; then + # Build provider block — include fallback as a second enabled provider when configured + # so opencode can route to it via `-m /` without re-config. + FALLBACK_OPENCODE_PROVIDER="${FALLBACK_OPENCODE_PROVIDER:-}" + if [ -n "$FALLBACK_OPENCODE_PROVIDER" ] && [ "$FALLBACK_OPENCODE_PROVIDER" != "$OPENCODE_PROVIDER" ]; then + PROVIDER_BLOCK="\"${OPENCODE_PROVIDER}\": {}, \"${FALLBACK_OPENCODE_PROVIDER}\": {}" + else + PROVIDER_BLOCK="\"${OPENCODE_PROVIDER}\": {}" + fi + # Create OpenCode configuration with MCP and provider settings cat > /tmp/opencode.json </dev/null 2>&1; then echo "⚠️ **Unable to generate diff summary**: Base ref not found: $base_ref" return 1 fi - + if ! git rev-parse "$head_ref" >/dev/null 2>&1; then echo "⚠️ **Unable to generate diff summary**: Head ref not found: $head_ref" return 1 fi - + local file_count file_count=$(git diff --name-only "$base_ref...$head_ref" 2>/dev/null | wc -l | tr -d ' ') - + if [ "$file_count" -eq 0 ]; then echo "ℹ️ **No files changed** in this PR" return 0 fi - + + # Build a temp file mapping `filepathSTATUS` so the numstat loop below + # can annotate each entry. We use a temp file instead of a bash associative + # array for portability (bash 3 on macOS, the while-read subshell, etc). + local status_file + status_file=$(mktemp) + # shellcheck disable=SC2064 + trap "rm -f '$status_file'" RETURN + + git diff --name-status "$base_ref...$head_ref" 2>/dev/null | \ + while IFS=$'\t' read -r status_code path_a path_b; do + local status_label="" + local display_path="$path_a" + case "$status_code" in + A) status_label="ADDED" ;; + M) status_label="MODIFIED" ;; + D) status_label="DELETED" ;; + T) status_label="TYPE-CHANGED" ;; + R*) status_label="RENAMED from $path_a"; display_path="$path_b" ;; + C*) status_label="COPIED from $path_a"; display_path="$path_b" ;; + *) status_label="CHANGED" ;; + esac + printf '%s\t%s\n' "$display_path" "$status_label" >> "$status_file" + done + + # Helper to look up a file's status; defaults to "CHANGED" if not found. + lookup_status() { + local needle="$1" + local line + line=$(grep -F -m 1 $'\t' <(awk -F'\t' -v p="$needle" '$1==p {print}' "$status_file")) + if [ -n "$line" ]; then + echo "$line" | cut -f2- + else + echo "CHANGED" + fi + } + echo "## 📋 Changed Files Summary" echo "" echo "**Total files changed**: $file_count" echo "" echo "**Instructions for Validation:**" echo "1. Review the list below to understand what changed" - echo "2. Read specific files using: \`cat path/to/file.ts\`" - echo "3. Check related files when needed (imports, configs, etc.)" - echo "4. Focus validation on the modified line ranges shown" + echo "2. Each file is annotated with its **Status** (ADDED / MODIFIED / DELETED / RENAMED)" + echo "3. **Read** files with Status ADDED, MODIFIED, RENAMED, or TYPE-CHANGED using: \`cat path/to/file.ts\`" + echo "4. **DO NOT** try to read files with Status DELETED — they no longer exist in the working tree. If your read fails with 'File not found', stop retrying and trust the status annotation." + echo "5. Check related files when needed (imports, configs, etc.)" + echo "6. Focus validation on the modified line ranges shown" echo "" echo "---" echo "" - + # Get list of changed files with their change stats git diff --numstat "$base_ref...$head_ref" 2>/dev/null | while read -r additions deletions filepath; do # Skip if filepath is empty [ -z "$filepath" ] && continue - + + # numstat uses "old => new" notation for renames; peel off the new path + # so the status lookup matches what name-status emitted. + case "$filepath" in + *" => "*) + filepath=$(printf '%s' "$filepath" | sed -E 's/.*=> *//' | tr -d '{}') + ;; + esac + + local file_status + file_status=$(lookup_status "$filepath") + echo "### \`$filepath\`" - + echo "- **Status**: $file_status" + # Handle binary files (shown as "-" in numstat) if [ "$additions" = "-" ]; then echo "- **Type**: Binary file" else echo "- **Changes**: +${additions} lines, -${deletions} lines" - - # Get the line ranges that changed (unified diff format gives us @@ markers) - # -U0 means no context, just the changed lines - local line_ranges - line_ranges=$(git diff -U0 "$base_ref...$head_ref" -- "$filepath" 2>/dev/null | \ - grep "^@@" | \ - sed 's/@@ -[0-9,]* +\([0-9,]*\) @@.*/Line \1/' | \ - head -10 | \ - tr '\n' ', ' | \ - sed 's/, $//') - - if [ -n "$line_ranges" ]; then - echo "- **Modified ranges**: $line_ranges" + + if [ "$file_status" = "DELETED" ]; then + echo "- **Note**: File was deleted — do NOT attempt to read it. Inspect the diff below (or \`git show HEAD^:$filepath\`) if you need to see its former contents." + else + # Get the line ranges that changed (unified diff format gives us @@ markers) + # -U0 means no context, just the changed lines + local line_ranges + line_ranges=$(git diff -U0 "$base_ref...$head_ref" -- "$filepath" 2>/dev/null | \ + grep "^@@" | \ + sed 's/@@ -[0-9,]* +\([0-9,]*\) @@.*/Line \1/' | \ + head -10 | \ + tr '\n' ', ' | \ + sed 's/, $//') + + if [ -n "$line_ranges" ]; then + echo "- **Modified ranges**: $line_ranges" + fi fi fi echo "" @@ -323,9 +382,13 @@ run_gemini() { # Error details are shown above in the output echo "⚠️ Check the output above for error details" - # Check if it's a retryable error + # Check if it's a retryable error. Includes transient upstream provider + # failures (OpenRouter 504/530, generic "Provider returned error", + # connection resets) in addition to the classic rate-limit signals, so + # flaky LLM backends don't burn the whole validation run. local is_retryable=false - if [ -f /tmp/validation-full-output.md ] && grep -q -E "(429|503|timeout|rate limit)" /tmp/validation-full-output.md; then + if [ -f /tmp/validation-full-output.md ] && \ + grep -q -i -E "(429|503|504|530|timeout|rate[- ]?limit|provider returned error|unmapped|ECONNRESET|EAI_AGAIN|socket hang up|deadline exceeded)" /tmp/validation-full-output.md; then is_retryable=true fi @@ -351,17 +414,20 @@ run_gemini() { return 1 } -# Run OpenCode validation with retry logic +# Run OpenCode validation with retry logic. +# Args: $1 prompt file, $2 opencode-provider (optional, defaults to OPENCODE_PROVIDER), +# $3 model id (optional, defaults to OPENCODE_MODEL). +# Returns: 0 success | 1 non-retryable failure | 2 retryable failure with retries exhausted (caller may fallback). run_opencode() { local prompt_file="$1" + local opencode_provider="${2:-${OPENCODE_PROVIDER:-openrouter}}" + local model="${3:-${OPENCODE_MODEL:-moonshotai/kimi-k2.5}}" local retry_count=0 local max_retries="${MAX_RETRIES:-2}" - local opencode_provider="${OPENCODE_PROVIDER:-openrouter}" - local model="${OPENCODE_MODEL:-moonshotai/kimi-k2.5}" local full_model="${opencode_provider}/${model}" while [ $retry_count -le "$max_retries" ]; do - echo "Attempt $((retry_count + 1))/$((max_retries + 1)): Running OpenCode validation..." + echo "Attempt $((retry_count + 1))/$((max_retries + 1)): Running OpenCode validation (${full_model})..." # Debug: Check prompt file if [ ! -f "$prompt_file" ]; then @@ -403,9 +469,13 @@ run_opencode() { echo "⚠️ Check the output above for error details" - # Check if it's a retryable error + # Check if it's a retryable error. Includes transient upstream provider + # failures (OpenRouter 504/530, generic "Provider returned error", + # connection resets) in addition to the classic rate-limit signals, so + # flaky LLM backends don't burn the whole validation run. local is_retryable=false - if [ -f /tmp/validation-full-output.md ] && grep -q -E "(429|503|timeout|rate limit)" /tmp/validation-full-output.md; then + if [ -f /tmp/validation-full-output.md ] && \ + grep -q -i -E "(429|503|504|530|timeout|rate[- ]?limit|provider returned error|unmapped|ECONNRESET|EAI_AGAIN|socket hang up|deadline exceeded)" /tmp/validation-full-output.md; then is_retryable=true fi @@ -417,18 +487,18 @@ run_opencode() { echo "⏳ Rate limit or timeout detected. Retrying after ${wait_time} seconds..." sleep $wait_time else - echo "::error::Maximum retries reached. Validation failed." - return 1 + echo "::error::Maximum retries reached. Validation failed (retryable)." + return 2 fi else - # Non-retryable error + # Non-retryable error — caller should NOT fall back to a different provider. echo "::error::Non-retryable error occurred. See error details above." return 1 fi fi done - return 1 + return 2 } # Extract validation report from AI output @@ -567,17 +637,29 @@ main() { echo "Prompt prepared: $prompt_with_replacements" - # Run validation with the configured provider + # Run validation with the configured provider. + # Use `|| rc=$?` (not `set +e`) because run_opencode/run_gemini toggle errexit + # internally to capture the CLI's exit code, which clobbers any outer set +e. local provider="${PROVIDER:-opencode}" + local rc=0 if [ "$provider" = "opencode" ]; then - run_func="run_opencode" + run_opencode "$prompt_with_replacements" "${OPENCODE_PROVIDER:-}" "${OPENCODE_MODEL:-}" || rc=$? + + # Fallback: only when primary exhausted retries on retryable errors (rc=2) + # AND a fallback opencode-provider is configured. Non-retryable failures (rc=1) + # bypass fallback because switching providers won't fix a malformed prompt/auth. + if [ "$rc" -eq 2 ] && [ -n "${FALLBACK_OPENCODE_PROVIDER:-}" ] && [ -n "${FALLBACK_OPENCODE_MODEL:-}" ]; then + echo "::warning::Primary opencode/${OPENCODE_PROVIDER:-}/${OPENCODE_MODEL:-} exhausted retries on retryable errors. Falling back to ${FALLBACK_OPENCODE_PROVIDER}/${FALLBACK_OPENCODE_MODEL}." + rc=0 + run_opencode "$prompt_with_replacements" "${FALLBACK_OPENCODE_PROVIDER}" "${FALLBACK_OPENCODE_MODEL}" || rc=$? + fi else - run_func="run_gemini" + run_gemini "$prompt_with_replacements" || rc=$? fi - if ! $run_func "$prompt_with_replacements"; then + if [ "$rc" -ne 0 ]; then echo "::error::Validation execution failed" - + # Set failed outputs using heredoc delimiter if [ -n "${GITHUB_OUTPUT:-}" ]; then { @@ -591,7 +673,7 @@ main() { echo "0" echo "EOF" } >> "$GITHUB_OUTPUT" - + echo "❌ Outputs set to error state" else echo "❌ Validation failed (local mode, no GITHUB_OUTPUT)" diff --git a/system-prompt.md b/system-prompt.md index 76a938d..313d167 100644 --- a/system-prompt.md +++ b/system-prompt.md @@ -197,14 +197,26 @@ After documenting the deviation: **You will receive a compact summary listing:** - All changed files + - A **Status** field per file: `ADDED`, `MODIFIED`, `DELETED`, `RENAMED`, `COPIED`, or `TYPE-CHANGED` - Number of additions/deletions per file - - Line ranges that were modified + - Line ranges that were modified (only for non-deleted files) + + **⚠️ DELETED files — STOP condition** + + Files annotated with `Status: DELETED` no longer exist in the working tree. Attempting to read them will fail with `File not found`. When that happens: + + - **Do NOT retry** the read with a different path format + - **Do NOT** loop through variations of the path + - **Trust** the `Status: DELETED` annotation in the summary + - If you need to see what was removed, use `git show origin/:path/to/deleted.ts` (where `` is the base branch) or inspect the diff with `git diff origin/...origin/ -- path/to/deleted.ts` — NEVER a bare `cat` or `Read` on the working-tree path + + Failing to stop on deleted files wastes tool-call budget and can trip the agent's loop detector, which will abort the entire validation run. **Your workflow:** a. **Review the summary** to understand scope and impact - b. **Read specific files** as needed using: + b. **Read specific files** (ADDED / MODIFIED / RENAMED / TYPE-CHANGED only) as needed using: ```bash # Read current state of a changed file @@ -232,26 +244,29 @@ After documenting the deviation: **⚠️ IMPORTANT: You do NOT need to run `git diff`** The summary already tells you what changed. Your job is to: - - ✅ **Read the current state** of files that changed + - ✅ **Read the current state** of files that changed (respecting Status) - ✅ **Focus on the modified line ranges** mentioned in the summary - ✅ **Check related files** when you need context - ❌ **Do NOT try to run `git diff`** - you already have the change list + - ❌ **Do NOT try to read DELETED files** - respect the Status annotation **Example workflow:** ```text - Summary shows: src/app/api/users/route.ts changed (lines 10-25) + Summary shows: + - src/app/api/users/route.ts Status: MODIFIED (lines 10-25) + - src/handlers/legacy.handler.ts Status: DELETED - 1. Read the file: cat src/app/api/users/route.ts + 1. Read the modified file: cat src/app/api/users/route.ts 2. Focus on lines 10-25 (that's what changed) - 3. Check if it imports a service: cat src/lib/services/user.service.ts - 4. Validate against standards + 3. SKIP src/handlers/legacy.handler.ts entirely — it's gone + 4. Check if route.ts imports a service: cat src/lib/services/user.service.ts + 5. Validate against standards ``` **If you cannot read the files:** - - Report which files you tried to access - - Explain what you were trying to validate - - Mark the validation as incomplete + - Check the Status annotation first — if the file is DELETED, that is expected, not an error + - For non-deleted files: report which files you tried to access, explain what you were trying to validate, and mark the validation as incomplete 3. **Validate Against Standards** - Use the knowledge base to find relevant standards