feat: harness-specific model selection in orchestration config (QUALITY-643)#10491
feat: harness-specific model selection in orchestration config (QUALITY-643)#10491cephalonaut merged 1 commit intomasterfrom
Conversation
7c3fea2 to
0420431
Compare
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR moves orchestration harness/model pickers toward server-provided harness availability and harness-specific model ids, and wires selected model ids into third-party harness launch paths.
Concerns
- Harness model catalog refreshes can leave a stale
model_idin state while the UI displaysDefault model, causing launches to submit a model that is no longer present in the server catalog. - The plan-card orchestration config still has non-harness-aware model picker initialization/LLM refresh paths, so non-Oz harnesses can show the Warp LLM catalog instead of the harness catalog.
- The run_agents card can reset streamed non-Oz model ids to default if
LLMPreferencesupdates beforeHarnessAvailabilityModelhas the harness model catalog.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| let is_local = !state.execution_mode.is_remote(); | ||
| populate_model_picker_for_harness( | ||
| handle, | ||
| &state.model_id, |
There was a problem hiding this comment.
repopulate_all_pickers refreshes the menu after harness catalogs change but cannot update state.model_id; when a previously selected harness model disappears, the picker can fall back to Default model while the stale id is still submitted. Revalidate/reset the owning edit state before repopulating, or have this helper return the reset id.
| |me, _, event, ctx| { | ||
| if let HarnessAvailabilityEvent::Changed = event { | ||
| if me.pickers_initialized { | ||
| oc::repopulate_all_pickers(&me.edit_state, &me.pickers, ctx); |
There was a problem hiding this comment.
LLMPreferences updates after selecting Claude/Codex—the picker can show the Warp catalog for a non-Oz harness. Update ensure_pickers and the LLMPreferences subscription to use populate_model_picker_for_harness with the current harness/execution mode.
| // This handles the case where harness was changed while | ||
| // models hadn't loaded yet. | ||
| let is_local = !me.state.orch.execution_mode.is_remote(); | ||
| if !oc::is_model_in_filtered_choices( |
There was a problem hiding this comment.
LLMPreferences refresh now validates non-Oz harness ids against HarnessAvailabilityModel; if that catalog has not loaded yet, a streamed model_id such as opus is reset to default and is not restored. Only run this revalidation for Oz/empty harnesses, or distinguish catalog-not-loaded from id-invalid.
|
This and to a lesser extent https://github.com/warpdotdev/warp-server/pull/11003 fix the multi-harness orchestration breakage |
| pickers_initialized: false, | ||
| toggle_mouse_state: MouseStateHandle::default(), | ||
| details_mouse_state: MouseStateHandle::default(), | ||
| saved_model_per_harness: std::collections::HashMap::new(), |
There was a problem hiding this comment.
Nit: import this at top of file (used in two spots)
| ctx.notify(); | ||
| } | ||
| RunAgentsCardViewAction::HarnessChanged { harness_type } => { | ||
| // Save the current model for the old harness before switching. |
There was a problem hiding this comment.
Nit: this is very similar to the orchestration config block logic - maybe worth pulling out into helper? Thinking signature could be
pub fn apply_harness_change<A: OrchestrationControlAction, V: View>(
state: &mut OrchestrationEditState,
memory: &mut PerHarnessModelMemory,
handles: &OrchestrationPickerHandles<A>,
new_harness_type: &str,
fallback_base_model_id: impl FnOnce(&mut ViewContext<V>) -> Option<String>,
ctx: &mut ViewContext<V>,
)
called like this
oc::apply_harness_change(
&mut self.state.orch,
&mut self.saved_model_per_harness,
&self.handles.pickers,
harness_type,
|ctx| block_model.base_model(ctx).map(|id| id.to_string()),
ctx,
);
| } | ||
| RunAgentsCardViewAction::ExecutionModeToggled { is_remote } => { | ||
| self.state.orch.toggle_execution_mode_to_remote(*is_remote); | ||
| // Repopulate model picker since Codex's available models |
There was a problem hiding this comment.
Nit: also very similar to orchestration config block - perhaps worth pulling into helper for shared logic?
Thinking signature is something like
pub fn apply_execution_mode_change<A: OrchestrationControlAction, V: View>(
state: &mut OrchestrationEditState,
handles: &OrchestrationPickerHandles<A>,
ctx: &mut ViewContext<V>,
)
called like this
ExecutionModeToggled { is_remote } => {
state.toggle_execution_mode_to_remote(*is_remote);
oc::apply_execution_mode_change(&mut state, &handles.pickers, ctx);
oc::sync_picker_selections(&state, &handles.pickers, ctx);
// (apply_field_change + ctx.notify as appropriate per view)
}
| let is_local = !self.state.orch.execution_mode.is_remote(); | ||
| oc::populate_model_picker_for_harness( | ||
| handle, | ||
| &self.state.orch.model_id, |
There was a problem hiding this comment.
we probably want to do this after setting below? self.state.orch.model_id
I guess it doesn't really matter due to sync_picker_selections but might still be a good idea
|
oh also @cephalonaut we should remove |
1fc54be to
eafaae9
Compare
…TY-643) Adds harness and model picker controls to the orchestration confirmation card and plan card config block, allowing users to select which harness (Oz, Claude Code, Codex, OpenCode) and model each orchestrated agent run uses. Key changes: - Server-driven harness picker populated from HarnessAvailabilityModel with display_name labels, brand icons, and Gemini filtered out - Harness-specific model picker: Oz shows Warp LLM catalog, non-Oz harnesses show 'Default model' + server-provided model catalog, Local Codex shows only 'Default model' - Model delivery: third_party_harness_model_id on build_runner trait, ANTHROPIC_MODEL env var for Claude, config.toml model key for Codex - Per-harness model memory via saved_model_per_harness HashMap so switching harnesses preserves previous selections - Conversation base model as fallback when model_id is empty for Oz - Config inheritance: resolve_from_config fills empty fields from the active OrchestrationConfig when the LLM omits them - OrchestrationConfigSnapshot handling in UpdateTaskMessage action (was only handled in AddMessagesToTask) - Shared helpers: apply_harness_change, apply_execution_mode_change, repopulate_all_pickers, harness_save_key — deduplicate logic between RunAgentsCardView and OrchestrationConfigBlockView - Both helpers take a fallback_base_model_id closure so model resets use the conversation's base model instead of first_filtered_model_id Co-Authored-By: Oz <oz-agent@warp.dev>
eafaae9 to
4ba7834
Compare
…TY-643) (warpdotdev#10491) ## Description Replace the hardcoded harness list and provider-filtered model picker in the orchestration config UI with server-provided harness availability data and harness-specific model catalogs. The selected model_id is delivered to harness processes (ANTHROPIC_MODEL for Claude Code, config.toml model key for Codex). Key changes across 12 files: - **Harness picker** reads from `HarnessAvailabilityModel` instead of hardcoded list; disabled harnesses shown but non-selectable - **Model picker** shows harness-specific models with "Default model" entry for non-Oz harnesses; local Codex shows only "Default model" - **Event subscriptions** on both card views for `HarnessAvailabilityEvent::Changed` to refresh pickers - **Model delivery**: Claude Code gets `ANTHROPIC_MODEL` env var; Codex gets `model` key in `~/.codex/config.toml` - **`build_runner` trait** accepts explicit `model_id` parameter (replaces env var smuggling) - **Streaming support**: `update_request()` re-syncs pickers when harness/model changes arrive via streaming Specs: `specs/QUALITY-643/PRODUCT.md`, `specs/QUALITY-643/TECH.md` [Agent conversation](https://staging.warp.dev/conversation/75d9ba5c-7a6f-4ea4-a15f-d1c57d44937f) ## Linked Issue https://linear.app/warpdotdev/issue/QUALITY-643 ## Testing - Added 6 new codex config.toml tests (model write/remove/preserve) - Added 2 new local_harness_launch tests (ANTHROPIC_MODEL env var) - All 115 relevant tests pass; existing orchestration_config tests unchanged - Renamed `preserves_unrelated_keys` → `preserves_non_model_keys` to reflect model is now a managed key - [x] I have manually tested my changes locally with `./script/run` ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode ## Demo https://www.loom.com/share/6dcff3f5f9d64b99880f678c999e7909 Co-authored-by: Oz <oz-agent@warp.dev>
Description
Replace the hardcoded harness list and provider-filtered model picker in the orchestration config UI with server-provided harness availability data and harness-specific model catalogs. The selected model_id is delivered to harness processes (ANTHROPIC_MODEL for Claude Code, config.toml model key for Codex).
Key changes across 12 files:
HarnessAvailabilityModelinstead of hardcoded list; disabled harnesses shown but non-selectableHarnessAvailabilityEvent::Changedto refresh pickersANTHROPIC_MODELenv var; Codex getsmodelkey in~/.codex/config.tomlbuild_runnertrait accepts explicitmodel_idparameter (replaces env var smuggling)update_request()re-syncs pickers when harness/model changes arrive via streamingSpecs:
specs/QUALITY-643/PRODUCT.md,specs/QUALITY-643/TECH.mdAgent conversation
Linked Issue
https://linear.app/warpdotdev/issue/QUALITY-643
Testing
Added 6 new codex config.toml tests (model write/remove/preserve)
Added 2 new local_harness_launch tests (ANTHROPIC_MODEL env var)
All 115 relevant tests pass; existing orchestration_config tests unchanged
Renamed
preserves_unrelated_keys→preserves_non_model_keysto reflect model is now a managed keyI have manually tested my changes locally with
./script/runAgent Mode
Demo
https://www.loom.com/share/6dcff3f5f9d64b99880f678c999e7909