fix(routing): exhaustive fallback and specificity signing for peer router - #105
google-labs-jules[bot] wants to merge 3 commits into
Conversation
…uter - Implement fail-fast behavior in `.github/actions/http-llm-invoke/action.yml` by raising an error on empty completions or Curl transport failures. - Add structured specificity signing to the http-llm-invoke action and Gemini CLI prompts to log the mechanism, provider, model, and configurations. - Update `.github/actions/model-router/action.yml` to replace speculative Gemini names with standard verified Gemini 2.5 flash models. - Implement exhaustive step-by-step fallback logic in gemini-review, gemini-triage, and gemini-invoke workflows so that failure of a routed model falls back sequentially to alternative free candidates and the residual Gemini route. Implements: TER-103 Signed-off-by: Grok <grok@x.ai>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Mention Blocks like a regular teammate with your question or request: @blocks review this pull request Run |
| with: | ||
| gemini_api_key: ${{ secrets.GEMINI_API_KEY }} | ||
| gemini_model: ${{ steps.router.outputs.model }} | ||
| gemini_model: ${{ steps.router.outputs.model == 'qwen/qwen3-coder:free' && 'gemini-2.5-flash' || steps.router.outputs.model }} |
There was a problem hiding this comment.
🔴 Backup AI reviewer is asked for by a name it does not recognize, so the backup never runs
When the primary assistant fails, the backup is requested using the failed assistant's model name (gemini_model: ${{ steps.router.outputs.model == 'qwen/qwen3-coder:free' && ... }} at .github/workflows/gemini-review.yml:173), which is only translated for one specific name, so every other fallback request is rejected and no answer is posted.
Impact: Whenever the primary path fails, the intended safety net also fails and the PR/issue silently receives no comment at all.
Peer model IDs leak into the Gemini CLI model input on the fallback path
The Gemini step now runs both for provider == 'gemini' and as a fallback when the omni/openrouter peer steps failed (.github/workflows/gemini-review.yml:155-161). In the fallback case steps.router.outputs.model holds a peer model id, e.g. auto/best-free (omni) or meta-llama/llama-3.3-70b-instruct:free / google/gemma-3-12b-it:free / deepseek/deepseek-r1:free (openrouter). The ternary only remaps qwen/qwen3-coder:free; everything else is passed straight through as gemini_model, which the Gemini CLI cannot resolve.
Additionally, for the invoke and triage roles the qwen/qwen3-coder:free slot does not exist in .github/actions/model-router/action.yml:87,95, so the remap there is dead code and 100% of fallbacks pass an invalid model.
Same pattern at .github/workflows/gemini-invoke.yml:163 and .github/workflows/gemini-triage.yml:205.
A robust fix is to select a known Gemini model explicitly on the fallback path (e.g. a role-appropriate constant) rather than remapping a single peer id.
Prompt for agents
The Gemini CLI step in .github/workflows/gemini-review.yml, gemini-invoke.yml and gemini-triage.yml is now also reached as a fallback when the omni/openrouter peer steps fail. In that case steps.router.outputs.model contains a peer model id (auto/best-free, meta-llama/llama-3.3-70b-instruct:free, google/gemma-3-12b-it:free, deepseek/deepseek-r1:free, qwen/qwen3-coder:free), but gemini_model only remaps the qwen id and otherwise forwards the peer id verbatim to the Gemini CLI, which will reject it. Note also that qwen is only a slot for the review role (see .github/actions/model-router/action.yml), so for invoke/triage the remap is dead code. Fix by making gemini_model resolve to a valid Gemini model whenever the router's provider is not 'gemini' (e.g. use a role-appropriate constant such as gemini-2.5-flash for review and gemini-2.5-flash-lite for triage/invoke), rather than special-casing a single peer model id.
Was this helpful? React with 👍 or 👎 to provide feedback.
| uses: ./.github/actions/http-llm-invoke | ||
| with: | ||
| provider: openrouter | ||
| model: 'meta-llama/llama-3.3-70b-instruct:free' |
There was a problem hiding this comment.
🟡 Backup attempt can retry the exact same assistant that just failed
The backup attempt is hard-coded to one fixed assistant (model: 'meta-llama/llama-3.3-70b-instruct:free' at .github/workflows/gemini-review.yml:146), which can be the very same one that just failed, so the retry repeats the same failure and wastes a full attempt.
Impact: In those cases the extra retry adds delay and quota usage without any chance of producing a result.
Fallback model overlaps with router-selectable models
For the review role the router can select meta-llama/llama-3.3-70b-instruct:free (third openrouter slot in .github/actions/model-router/action.yml:91). If that selection is what failed, peer_or_fallback retries the identical model with the identical prompt.
The same overlap exists in .github/workflows/gemini-invoke.yml:129 and .github/workflows/gemini-triage.yml:169, where the fallback is google/gemma-3-12b-it:free, which is also a router-selectable slot for those roles.
A fix would be to pick a fallback model that differs from steps.router.outputs.model (e.g. conditional expression choosing an alternative when they match).
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: OpenRouter review fallback (peer) | ||
| id: peer_or_fallback | ||
| if: | | ||
| steps.router.outputs.skip != 'true' && | ||
| steps.router.outputs.provider == 'openrouter' && | ||
| steps.peer_or.outcome == 'failure' | ||
| continue-on-error: true | ||
| uses: ./.github/actions/http-llm-invoke | ||
| with: | ||
| provider: openrouter | ||
| model: 'meta-llama/llama-3.3-70b-instruct:free' | ||
| role: review | ||
| target_number: ${{ inputs.pr_number || github.event.pull_request.number }} | ||
| api_key: ${{ secrets.OPENROUTER_API_KEY }} | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| repository: ${{ github.repository }} | ||
| prompt: ${{ steps.rev_ctx.outputs.prompt }} |
There was a problem hiding this comment.
🔍 Fallback attempts bypass the soft-budget counters
The new peer_or_fallback steps call ./.github/actions/http-llm-invoke directly with a hard-coded model, so the per-model counters maintained by .github/actions/model-router/action.yml:79-83 are never incremented for those calls. Likewise, the Gemini peer-fallback path consumes Gemini quota without any bump for the Gemini model. Over time this makes the "soft budget" accounting under-count real usage, which is the exact mechanism the router uses to avoid exhausting free tiers.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Run Gemini CLI PR review (residual / peer-fallback) | ||
| if: | | ||
| steps.router.outputs.skip != 'true' && | ||
| ( | ||
| steps.router.outputs.provider == 'gemini' || | ||
| (steps.router.outputs.provider == 'omni' && steps.peer_omni.outcome == 'failure') || | ||
| (steps.router.outputs.provider == 'openrouter' && steps.peer_or.outcome == 'failure' && steps.peer_or_fallback.outcome == 'failure') | ||
| ) |
There was a problem hiding this comment.
📝 Info: Gemini fallback runs even when no Gemini key is configured
The Gemini step's if: only checks the router's provider and the peer steps' outcomes; it does not check that secrets.GEMINI_API_KEY is non-empty (the router's has-gemini input already encodes this). If no Gemini key exists, a peer failure will start the run-gemini-cli action with an empty API key, burning a job step that can only fail. It is masked by continue-on-error: true, so the impact is noise rather than breakage, but adding a secrets.GEMINI_API_KEY != '' clause would make the exhaustive-fallback chain honest. Same in .github/workflows/gemini-invoke.yml:143-150 and .github/workflows/gemini-triage.yml:186-193.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Run Gemini CLI assistant (residual / peer-fallback) | ||
| if: | | ||
| steps.router.outputs.skip != 'true' && | ||
| ( | ||
| steps.router.outputs.provider == 'gemini' || | ||
| (steps.router.outputs.provider == 'omni' && steps.peer_omni.outcome == 'failure') || | ||
| (steps.router.outputs.provider == 'openrouter' && steps.peer_or.outcome == 'failure' && steps.peer_or_fallback.outcome == 'failure') | ||
| ) |
There was a problem hiding this comment.
📝 Info: outcome vs conclusion semantics with continue-on-error are used correctly
I verified that the fallback conditions rely on steps.<id>.outcome, which retains failure even though continue-on-error: true forces conclusion to success. Skipped steps evaluate to skipped, so steps.peer_or_fallback.outcome == 'failure' is false when it never ran — the compound condition therefore cannot mis-trigger the Gemini path. Note however that the omni branch has no intermediate fallback, so it degrades straight to Gemini while the openrouter branch gets two peer attempts; this asymmetry looks intentional but is worth confirming against the "exhaustive fallback" goal.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if [ -z "$TEXT" ]; then | ||
| echo "::warning::${PROVIDER} empty response: ${ERR:-no content}" | ||
| TEXT="⚠️ ${PROVIDER} returned no content. ${ERR}" | ||
| echo "::error::${PROVIDER} empty response: ${ERR:-no content}" | ||
| exit 1 |
There was a problem hiding this comment.
📝 Info: Empty-response now fails the action instead of posting a placeholder comment
Switching from a warning + placeholder comment to exit 1 is what enables the new fallback chain, but it also changes the contract of this composite action for any caller that does not set continue-on-error: true. All current callers in the three gemini-* workflows do set it (the triage peer steps gained it in this PR), so no job currently turns red; future callers must remember it.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Build review prompt (API diff — independent of checkout ref) | ||
| id: rev_ctx | ||
| if: steps.router.outputs.skip != 'true' && (steps.router.outputs.provider == 'omni' || steps.router.outputs.provider == 'openrouter') | ||
| if: steps.router.outputs.skip != 'true' |
There was a problem hiding this comment.
📝 Info: Context-building steps now run for the Gemini-only path too
Relaxing the if: to skip != 'true' means rev_ctx (and issue_ctx in .github/workflows/gemini-triage.yml:76) now execute an extra gh api call even when the router selected Gemini and no peer step will consume the output. This is required so the prompt exists for the peer-fallback path, but it adds an unconditional API call per run; the Gemini CLI step ignores these outputs entirely.
Was this helpful? React with 👍 or 👎 to provide feedback.
| MARKER="<!-- openrouter-${ROLE} -->" | ||
| HEAD="### 🔀 OpenRouter ${ROLE} (\`${MODEL}\`)" | ||
| EGG="<!-- matrix: peer free path; public boards=features; 3L0=our labels -->" | ||
| SIGNATURE="@openRouter{provider: openrouter; model: ${MODEL}; settings: [connect-timeout=15, max-time=90]; mechanism: peer-router-auto}" |
There was a problem hiding this comment.
📝 Info: Signature string is only defined inside provider branches
SIGNATURE is assigned in each arm of the provider case, which has no *) default. The script runs under set -euo pipefail, so an unset SIGNATURE would abort the step. This is safe today only because the earlier provider case at .github/actions/http-llm-invoke/action.yml:67-71 exits on unknown providers; if a third provider is ever added to only one of the two case statements, the step will die with an obscure unbound-variable error.
(Refers to lines 100-111)
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@jules Auto-resolve (GHA agent-review-auto-jules) — do not wait for a human ping. Feedback excerptInstructions
|
|
head_sha: ddd0fd5 Peer review gate (ready for second-pass agents)External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot. Peer activity (truncated): Downstream: |
🔀 OpenRouter review (
|
…dling - Implement fail-fast behavior in `.github/actions/http-llm-invoke/action.yml` by raising an error on empty completions or Curl transport failures. - Add structured specificity signing to the http-llm-invoke action and Gemini CLI prompts to log the mechanism, provider, model, and configurations. - Update `.github/actions/model-router/action.yml` to replace speculative Gemini names with standard verified Gemini 2.5 flash models. - Implement exhaustive step-by-step fallback logic in gemini-review, gemini-triage, and gemini-invoke workflows so that failure of a routed model falls back sequentially to alternative free candidates and the residual Gemini route. - Handle USAGE_LIMIT_EXCEEDED / usage limit exceeded gracefully inside `agent-feedback-linear-sync.yml` GHA script as a warning instead of a fatal failure. Implements: TER-103 Signed-off-by: Grok <grok@x.ai>
| if [ -z "$TEXT" ]; then | ||
| echo "::warning::${PROVIDER} empty response: ${ERR:-no content}" | ||
| TEXT="⚠️ ${PROVIDER} returned no content. ${ERR}" | ||
| echo "::error::${PROVIDER} empty response: ${ERR:-no content}" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🔍 Non-error early exits bypass the new fallback chain
The action still exits with status 0 for several non-success conditions: missing API key (.github/actions/http-llm-invoke/action.yml:54-57), invalid target number (:58-61), non-https base URL (:79-82), and OpenRouter non-:free model (:102-106). Only the empty-response case now exits 1. Because the new fallback steps key on outcome == 'failure', any of these paths results in no comment being posted and no fallback being attempted — e.g. if OMNI_API_KEY is unset but has-omni was computed true, the run ends silently. If the intent is exhaustive fallback, these should also exit non-zero (except perhaps the invalid-target case where no comment can be posted anyway).
Was this helpful? React with 👍 or 👎 to provide feedback.
| uses: ./.github/actions/http-llm-invoke | ||
| with: | ||
| provider: openrouter | ||
| model: 'meta-llama/llama-3.3-70b-instruct:free' |
There was a problem hiding this comment.
📝 Info: OpenRouter fallback may retry the exact model that just failed
The hardcoded fallback models are also primary router slots: google/gemma-3-12b-it:free is a triage/invoke slot and meta-llama/llama-3.3-70b-instruct:free is a review slot (.github/actions/model-router/action.yml:87-96). When the router already selected that model (e.g. after the earlier slots exhausted their soft budget), the fallback step re-issues the identical request against the same provider/model that just failed, so it adds latency and an extra API call without changing the outcome. Consider picking a fallback model different from steps.router.outputs.model.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } catch (error) { | ||
| if (error.message.includes('usage limit exceeded') || error.message.includes('USAGE_LIMIT_EXCEEDED')) { | ||
| core.warning(`Linear workspace quota exceeded. Skipping sync: ${error.message}`); | ||
| return; | ||
| } | ||
| throw error; |
There was a problem hiding this comment.
📝 Info: Linear quota handling relies on error message text
Wrapping the whole sync in try/catch and swallowing only quota errors depends on error.message containing usage limit exceeded / USAGE_LIMIT_EXCEEDED. The thrown error is new Error(JSON.stringify(json.errors)) (.github/workflows/agent-feedback-linear-sync.yml:86), so matching depends on Linear's GraphQL error payload wording/casing; a payload that only sets an extension code like USAGE_LIMIT or lowercase text would still fail the job. A structured check on the parsed error extensions would be more robust.
Was this helpful? React with 👍 or 👎 to provide feedback.
| GEM_CANDIDATES="gemini-2.5-flash-lite:450 gemini-2.5-flash:15" | ||
| ;; | ||
| review) | ||
| PEER_SLOTS="omni|auto/best-free|120 openrouter|qwen/qwen3-coder:free|30 openrouter|meta-llama/llama-3.3-70b-instruct:free|40 openrouter|deepseek/deepseek-r1:free|20" | ||
| GEM_CANDIDATES="gemini-3.5-flash:15 gemini-2.5-flash:15 gemini-3-flash:15 gemini-3.1-flash-lite:450" | ||
| GEM_CANDIDATES="gemini-2.5-flash:15 gemini-2.5-flash-lite:450" | ||
| ;; | ||
| invoke) | ||
| PEER_SLOTS="omni|auto/best-free|200 openrouter|meta-llama/llama-3.3-70b-instruct:free|40 openrouter|google/gemma-3-12b-it:free|40" | ||
| GEM_CANDIDATES="gemini-3.1-flash-lite:450 gemini-3.5-flash-lite:450" | ||
| GEM_CANDIDATES="gemini-2.5-flash-lite:450 gemini-2.5-flash:15" |
There was a problem hiding this comment.
🔍 Gemini soft-budget counters are shared across roles
Gemini candidates are counted by bare model name (count "$model" / bump "$model" at .github/actions/model-router/action.yml:142-144) while peers use provider/model keys. After collapsing all roles onto the same two models (gemini-2.5-flash-lite, gemini-2.5-flash), triage, review and invoke now all draw from the same counter files, so the effective per-role headroom is much smaller than the listed limits suggest (e.g. gemini-2.5-flash:15 is now shared by all three roles instead of being review-specific).
Was this helpful? React with 👍 or 👎 to provide feedback.
|
head_sha: 03c6a92 Peer review gate (ready for second-pass agents)External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot. Peer activity (truncated): Downstream: |
🔀 OpenRouter review (
|
…imit mitigation - Implement fail-fast behavior in `.github/actions/http-llm-invoke/action.yml` by raising an error on empty completions or Curl transport failures. - Add structured specificity signing to the http-llm-invoke action and Gemini CLI prompts to log the mechanism, provider, model, and configurations. - Update `.github/actions/model-router/action.yml` to replace speculative Gemini names with standard verified Gemini 2.5 flash models. - Implement exhaustive step-by-step fallback logic in gemini-review, gemini-triage, and gemini-invoke workflows so that failure of a routed model falls back sequentially to alternative free candidates and the residual Gemini route. - Handle USAGE_LIMIT_EXCEEDED / usage limit exceeded gracefully inside `agent-feedback-linear-sync.yml` GHA script as a warning instead of a fatal failure. Implements: TER-103 Signed-off-by: Grok <grok@x.ai>
| GEM_CANDIDATES="gemini-2.5-flash-lite:450 gemini-2.5-flash:15" | ||
| ;; | ||
| review) | ||
| PEER_SLOTS="omni|auto/best-free|120 openrouter|qwen/qwen3-coder:free|30 openrouter|meta-llama/llama-3.3-70b-instruct:free|40 openrouter|deepseek/deepseek-r1:free|20" | ||
| GEM_CANDIDATES="gemini-3.5-flash:15 gemini-2.5-flash:15 gemini-3-flash:15 gemini-3.1-flash-lite:450" | ||
| GEM_CANDIDATES="gemini-2.5-flash:15 gemini-2.5-flash-lite:450" | ||
| ;; | ||
| invoke) | ||
| PEER_SLOTS="omni|auto/best-free|200 openrouter|meta-llama/llama-3.3-70b-instruct:free|40 openrouter|google/gemma-3-12b-it:free|40" | ||
| GEM_CANDIDATES="gemini-3.1-flash-lite:450 gemini-3.5-flash-lite:450" | ||
| GEM_CANDIDATES="gemini-2.5-flash-lite:450 gemini-2.5-flash:15" |
There was a problem hiding this comment.
📝 Info: Gemini candidate lists changed to currently-available models
The residual Gemini candidate lists were rewritten to only 2.5-series models with different per-day limits (e.g. triage now gemini-2.5-flash-lite:450 gemini-2.5-flash:15). Counters are keyed by model name in the cache, so the removed 3.x entries simply stop being consulted; no migration issue. Worth confirming the 15/450 daily limits still match the current free-tier quotas.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@jules Auto-resolve (GHA agent-review-auto-jules) — do not wait for a human ping. Feedback excerptInstructions
|
|
head_sha: dee0f6f Peer review gate (ready for second-pass agents)External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot. Peer activity (truncated): Downstream: |
🔀 OpenRouter review (
|
timerloggedout-spec
left a comment
There was a problem hiding this comment.
Approved as OPERATOR. Exhaustive fallback + specificity signing for peer router. Complements #104.
|
OPERATOR status after #123 merge
Action: @google-labs-jules rebase onto |
|
Closed as superseded by #123. Exhaustive peer fallback / specificity handled by the Python model router on |
Integrate Rust-based MCP Agent Mail (mcp_agent_mail_rust) into the termux-monorepo GitHub Actions workflows. - Registered 'agent-mailbox' active proposal in docs/proposals/registry.yaml (linked to issues #90, #105, #109). - Created docs/proposals/active/agent-mailbox/MANIFEST.md and ITEMS.md. - Created composite action .github/actions/mcp-agent-mail/action.yml to setup and run the Rust mcp-agent-mail server/CLI. - Created demonstration workflow .github/workflows/agent-coordination-demo.yml showcasing parallel job coordination and file reservation. - Implemented robust, pyyaml-independent unit tests in tests/test_mcp_agent_mail.py, all verified and passing 100%.
Implement fail-fast behavior, specificity signing, and exhaustive sequential fallbacks across OpenRouter, OmniRoute, and Gemini paths in the termux-monorepo CI/CD workflows to solve PR 103/104 failures.
PR created automatically by Jules for task 5450365102026496177 started by @timerloggedout-spec