Use actual agent for workflow model fallback

[wesm]

Summary

• stop applying generic default_model based on the configured workflow agent when the actual runnable agent differs
• use the actual resolved agent for fix, review, refine, security, and design model fallback across CLI, daemon, CI, and batch paths
• add regressions for workflow-agent fallback and unavailable-agent fallback cases

Testing

• go fmt ./...
• go test ./...
• go vet ./...

[roborev-ci]

roborev: Combined Review (9c07d7a)

Summary: The PR centralizes
workflow model resolution and adds fallback tests, but introduces a few medium-severity issues regarding custom ACP aliases, backup models in batch processing, and a test script typo on Windows.

Medium

• [internal/agent/model_resolution.go:32](/home/roborev/repos/roborev
  /internal/agent/model_resolution.go#L32)

  • Problem: ResolveWorkflowModelForAgent compares agent identity using CanonicalName, which only covers static aliases (claude, agent). It does not treat configured ACP aliases (cfg.ACP.Name) as equivalent
    to runtime "acp".
  • Impact: With default_agent = "<custom ACP alias>", the actual selected agent name is "acp", so the function incorrectly treats it as a “different agent” and skips the default_model.
  • Suggested fix: Make comparison ACP
    -aware (e.g., resolve both sides through GetAvailableWithConfig(...).Name() before comparing, or add ACP-name equivalence in a shared normalization helper). Add tests for configured ACP alias + default_model.

• [internal/review/batch.go:125](/home
  /roborev/repos/roborev/internal/review/batch.go#L125)

  • Problem: After backup-agent handling, runSingle now always calls ResolveWorkflowModelForAgent when model == "". If the backup was selected and the backup
    model is unset, this can incorrectly apply the workflow/default model to the backup agent.
  • Impact: Behavioral regression vs. other paths (fix/refine/server/ci_poller) where a selected backup with no backup model keeps its own default model. This can assign an incompatible model to the
    backup agent.
  • Suggested fix: Track usingBackup and skip the fallback ResolveWorkflowModelForAgent call when the backup is selected and no explicit model is set. Add a batch test covering “backup selected + empty backup model” (especially with a configured workflow model).

• internal/
  daemon/server_jobs_test.go:1761 (approx)

  • Problem: The Windows mock script generation for the claude binary contains a copy-paste error ( @internal/prompt/analyze/complexity.txt /b 0\r\n) instead of a valid batch exit command. This will cause the mock binary to fail execution on Windows.
  • Suggested fix: Change to use a valid batch exit command (e.
    g., claudeScript = []byte("@exit /b 0\r\n")).

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (7c4d588)

Verdict: The model-selection refactoring is logically sound, but introduces high-severity syntax and formatting errors in the test mock scripts that will cause compilation and runtime failures.

High

• internal/daemon/server_jobs_test.go (
  line ~1761)
  • Problem: Malformed mock script content appears to be a mangled replacement error, which will cause the mock batch script to fail on Windows.
  • Fix: Change to a valid exit command.
• internal/daemon/server_jobs_ test.go (line ~1762)
  • Problem: Unescaped double quotes inside the string literal fmt.Appendf(nil, " @"%s" %%*\r\n", gitPath) will cause a compilation error.
  • Fix: Escape the quotes
    properly.
• internal/daemon/server_jobs_test.go (line ~1765)
  • Problem: Malformed shell script syntax in fmt.Appendf(nil, "#!/bin/sh\nexec %q \"$ @"n", gitPath ). It improperly formats the arguments variable and is missing a backslash for the newline.
  • Fix: Correct the string to properly pass arguments.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (6f32c8f)

Verdict: The model-selection logic refactoring is secure and centralizes workflow model fallback resolution, but introduces a high-severity
syntax error in test files that must be corrected.

High

• Location: internal/daemon/server_jobs_test.go (inside TestHandleEnqueueFallbackAgentUsesDefaultModelForActualAgent)

  • Problem: Invalid Go syntax and corrupted script contents due to escaping/copy-
    paste errors. Unescaped quotes in @"%s", mangled shell variables (\"$ @"n"), and an accidental file path ( @internal/prompt/analyze/complexity.txt) instead of the exit command.
  • Fix: Correct the string literals to valid Go/
    shell syntax:
    • claudeScript = []byte(" @internal/prompt/analyze/complexity.txt /b 0\r\n")
    • gitWrapper = fmt.Appendf(nil, " @"%s" %%*\r\n", gitPath)

• gitWrapper = fmt.Appendf(nil, "#!/bin/sh\nexec %q \"$ @"n", gitPath)

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)