feat: add model selection per task via global config

[zohar]

Users can configure which AI model is used for each spec-kitty
command by creating ~/.spec-kitty/config.yaml:

  models:
    specify: claude-opus-4-6
    implement: claude-sonnet-4-6

The model: field is injected into agent command frontmatter during
spec-kitty upgrade, supporting all configured agents automatically.

[robertDouglass]

Code Review — PR #265

Hey @zohar, thanks for working on this. I did a thorough review and want to share some findings and questions.

What's actually in the diff

The PR title says "add model selection per task via global config" and the description mentions ~/.spec-kitty/config.yaml with a models: mapping and frontmatter injection — but none of that code is present in the diff. There's no global_config.py, no migration file, no config.yaml reading, and no model injection.

What the PR actually delivers is a branch-contract refactoring — separating the concept of "planning branch" from "merge target branch." This is valuable work! But it's a different feature than what's described.

Can you clarify: Is the model selection feature planned as a follow-up PR on top of this branch infrastructure? Or was it intended to be here and something went wrong with the commits?

What works well

• The ADR (2026-03-20-1) documenting the planning-branch vs merge-target separation is excellent — clear problem statement, options considered, consequences listed
• get_feature_planning_branch() fallback chain is clean and backward-compatible
• _ensure_feature_branch_checked_out() handles edge cases well (existing branch, missing start point, dirty working tree)
• Deterministic branch enforcement with clear error messages is a real improvement
• Tests use real git repos, which is valuable

Suggestions to make this PR merge-ready

1. Retitle and rewrite the description to match the actual code. Something like "refactor: separate feature planning branch from merge target (ADR 2026-03-20-1)" would be accurate. The current title/description will persist as misleading git history.

2. Remove the kitty-specs/042-model-selection-per-task/ artifacts from this PR (1,628 lines of unrelated spec planning files). They inflate the diff from ~88 lines of real code to ~1,700 lines and obscure the actual change. These can go in their own PR when the model selection feature is ready.

3. The WP files claim "done" for undelivered work — WP01-core-implementation.md, WP02-tests.md, WP03-documentation.md are all marked lane: "done" and review_status: "approved", but the code they describe (global_config.py, m_2_0_4_model_injection.py) doesn't exist. This should be corrected.

4. Track base_branch / merge_target separately from target_branch. Right now meta.json records target_branch as the feature branch (e.g., 042-model-selection-per-task), but there's no record of which branch the feature should eventually merge back into (e.g., main or 2.x). This gap means the tool won't know where to open the PR when the feature is done. Adding a merge_into or upstream_branch field to meta.json would complete the git-flow story.

5. Complete the resolve_target_branch migration — implement.py at line 1018 still uses the old resolve_target_branch() while the rest of the system uses get_feature_planning_branch().

6. Replace the except Exception: pass in feature.py (lines 737-739) where a commit failure for task scaffolding is silently swallowed. The project policy is to fail intentionally when something isn't working — at minimum log a warning.

7. Fix base_branch semantics — in _inject_branch_contract(), base_branch is set equal to target_branch (the feature branch), not the integration branch. Any downstream code expecting base_branch to mean "where this merges into" will get wrong data.

The bigger question

Model selection per task (claude-opus-4-6 for specify, claude-sonnet-4-6 for implement, etc.) is a genuinely useful feature. What's the status of the actual implementation? Is the branch infrastructure in this PR a prerequisite, or could model selection be built independently? Understanding the roadmap would help figure out the best path forward.

---

🤖 Review generated with Claude Code