feat(backends): add resolveModel() to AgentEngine interface

[aaight]

Summary

• Adds optional resolveModel?(cascadeModel: string): string method to the AgentEngine interface in types.ts
• ClaudeCodeEngine, CodexEngine, and OpenCodeEngine implement resolveModel() delegating to their existing standalone resolve functions
• LlmistEngine does NOT implement resolveModel() (passes model through unchanged)
• adapter.ts calls engine.resolveModel?.(rawModel) during execution plan building, centralizing model validation in one place
• Standalone functions (resolveClaudeModel, resolveCodexModel, resolveOpenCodeModel) remain exported for backward compatibility

Card: https://trello.com/c/69b572b05754570c0c89fa4f

Test plan

• npm run typecheck — zero errors
• npm run lint — zero errors
• npm test — all backend unit tests pass (pre-existing failures in postComment/github gadget tests are unrelated to this change)
• registry.test.ts mock engine still compatible with updated interface (optional method)

🤖 Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · run details

[nhopeatall]

Summary

Clean interface addition — the optional resolveModel?() on AgentEngine is well-designed and correctly integrated. However, model resolution is now called twice for engines that implement it, which contradicts the stated goal of centralizing model validation in one place.

Should Fix

• Double model resolution: The adapter now calls engine.resolveModel(rawModel) at adapter.ts:191 and places the resolved model into AgentExecutionPlan.model. But each engine's execute() method still calls its standalone resolve function on input.model:

  • claude-code/index.ts:471: resolveClaudeModel(input.model)
  • codex/index.ts:495: resolveCodexModel(input.model)
  • opencode/index.ts:811: resolveOpenCodeModel(input.model)

  This is currently safe because all three resolve functions are idempotent (resolving an already-resolved model produces the same result). But it contradicts the PR description's goal of centralizing model validation in one place — validation actually runs in two places now. The execute() methods should either:

  1. Drop their internal resolve*Model() calls (relying on the adapter to pre-resolve), or
  2. Be left as-is but with a comment explaining the intentional redundancy for backward compatibility (in case execute() is called without going through the adapter).

  Option (2) seems more practical since removing the internal calls would be a behavior change for any code path that calls execute() directly.

No blocking issues — the change is safe to merge as-is, but addressing the double-resolution would deliver on the stated design intent.

🕵️ claude-code · claude-opus-4-6 · run details

[nhopeatall]

LGTM — Clean, well-scoped change that centralizes model resolution behind the AgentEngine interface.

The design is sound:

• Optional interface method preserves backward compatibility (LlmistEngine omits it, existing mocks/tests pass without it)
• The deliberate double-resolution (adapter pre-resolves + execute() re-resolves) is justified and well-commented — execute() may be called directly in tests
• All three resolve*Model() functions are genuinely idempotent (verified: repeated application produces the same output)
• The buildExecutionPlan signature addition threads the engine through cleanly to resolvePartialExecutionPlan
• Standalone resolve functions remain exported for backward compatibility

No issues found.

🕵️ claude-code · claude-opus-4-6 · run details