pr-review engine: COPILOT_API_MODEL unbound var silently disables the cross-engine rubber-duck tier (engine.sh:431) #881

Activity

1. don-petry

   added

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   on Jun 21, 2026
2. github-actions

   added

   bugBug reportsBug reports

   needs-human-reviewFlagged by automated PR review agentFlagged by automated PR review agent

   on Jun 21, 2026
3. github-actions commented on Jun 21, 2026

   

   github-actionsBot

   on Jun 21, 2026 – with GitHub Actions

   Contributor

   More actions

   Thanks for the exceptionally thorough report — the run ID, line references, and root-cause trace make this very easy to follow! Since both proposed approaches (hoisting the COPILOT_API_MODEL default unconditionally vs. defaulting at point-of-use in _copilot_invoke) would fix the unbound variable, could you share which you'd recommend and whether either has been validated against the existing shell test suite?
4. don-petry commented on Jun 21, 2026

   

   don-petry

   on Jun 21, 2026

   CollaboratorAuthor

   More actions

   Dev-Lead Implementation Plan

   Issue: #881 — pr-review engine: COPILOT_API_MODEL unbound var silently disables the cross-engine rubber-duck tier (engine.sh:431)

   Scope

   Give COPILOT_API_MODEL an unconditional default in set_engine_config so the copilot-backed rubber-duck path stays bound (and doesn't trip set -u) when a non-copilot engine (e.g. claude) is primary.

   Implementation Checklist

   • scripts/engine.sh — hoist the COPILOT_API_MODEL="${COPILOT_API_MODEL:-openai/o4-mini}" default out of the copilot) branch so it is set unconditionally (alongside the other unconditional exports near the end of set_engine_config)

   Tests to Write

   • New tests/dev-lead/unit/test_engine_duck_model.bats:
     • claude-primary config leaves COPILOT_API_MODEL bound to openai/o4-mini
     • sourcing engine.sh as claude under set -u and referencing $COPILOT_API_MODEL does not abort (regression for engine.sh:431)
     • explicit COPILOT_API_MODEL override is still honored under claude
     • copilot-primary still binds COPILOT_API_MODEL (no regression)

   Test Command

   bats tests/dev-lead/unit/
5. don-petry commented on Jun 21, 2026

   

   don-petry

   on Jun 21, 2026

   CollaboratorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Plan Execution

   • scripts/engine.sh — hoisted the COPILOT_API_MODEL="${COPILOT_API_MODEL:-openai/o4-mini}" default out of the copilot) branch so it is set unconditionally after the case, before the export block. The existing export ... COPILOT_API_MODEL line now always exports a bound value.
   • tests/dev-lead/unit/test_engine_duck_model.bats — new regression suite (5 tests).

   Root cause / fix

   The claude) branch wires the cross-engine duck to copilot (DUCK_ENGINE=copilot), but COPILOT_API_MODEL's default was assigned only inside the copilot) branch. Under a claude-primary review (the default) that branch never runs, leaving COPILOT_API_MODEL unset; copilot_chat's bare $COPILOT_API_MODEL reference (engine.sh:431/436) then aborted under set -u, silently skipping tier-2. The default is now unconditional, so the duck path stays bound for any non-copilot primary.

   Test Results

   New regression suite (red before fix on tests 1/2/5, green after):

   1..5
   ok 1 duck-model: claude-primary config leaves COPILOT_API_MODEL bound to default
   ok 2 duck-model: sourcing as claude under set -u does not trip on COPILOT_API_MODEL (engine.sh:431 regression)
   ok 3 duck-model: explicit COPILOT_API_MODEL override is honored under claude
   ok 4 duck-model: copilot-primary still binds COPILOT_API_MODEL to default (no regression)
   ok 5 duck-model: gemini-primary also leaves COPILOT_API_MODEL bound (latent non-copilot duck path)
   

   Full dev-lead unit suite: 577 passed, 0 failed (bats tests/dev-lead/unit/).

   Files Changed

   • scripts/engine.sh: moved the COPILOT_API_MODEL default + explanatory comment out of the copilot) branch to an unconditional assignment in set_engine_config (before the exports). No model values changed; copilot-primary behavior is byte-identical.
   • tests/dev-lead/unit/test_engine_duck_model.bats: new — asserts claude/gemini/copilot primaries all leave COPILOT_API_MODEL bound to openai/o4-mini, that an explicit override is honored, and that sourcing as claude under set -u + a bare $COPILOT_API_MODEL reference does not abort.

   Verification

   • bash .dev-lead/scripts/dev-lead-lint.sh → all checks passed (shellcheck on 72 files + agent frontmatter + CODEOWNERS).
   • shellcheck scripts/engine.sh → exit 0 (only the two pre-existing SC2015/SC1091 info notes at line 276, unrelated to this change).

   Notes

   • scripts/review-batch.sh already guards its reads with ${COPILOT_API_MODEL:-...}, so it was unaffected.
   • Pre-existing unrelated failure: tests/test_batch_fallback.bats test "Gemini trust error (55) falls back to Copilot" fails on the clean tree as well (verified via git checkout scripts/engine.sh); it concerns gemini fallback, not COPILOT_API_MODEL, and is out of scope for pr-review engine: COPILOT_API_MODEL unbound var silently disables the cross-engine rubber-duck tier (engine.sh:431) #881.
6. don-petry

   closed this as completedin #894on Jun 21, 2026
7. don-petry

   added a commit that references this issue on Jun 21, 2026

   feat: implement issue #881 — pr-review engine: `COPILOT_API_MODEL` un…

   ...

   8714816
8. don-petry

   closed this as a duplicateof pr-review: rubber-duck tier broken under default claude engine — COPILOT_API_MODEL: unbound variable #773 on Jun 22, 2026