fix(agent): default ACP agents to basic capability tier

crates/op-host-desktop/src/chat_design_request.rs

@@ -1,16 +1,24 @@
 use op_editor_core::EditorState;
 use op_orchestrator::{AppendContext, DesignRequest};
 
-/// Resolve the selected chat model's id for the orchestrator. Only
-/// built-in (API-key) agents expose a concrete model id; CLI/ACP agents
-/// pick their own model internally and yield `None` (the CLI-side
-/// selection rides `ChatProviderLlmClient::with_model` instead). The id
-/// feeds model-aware orchestrator policy — tier-gated skill filtering,
-/// the element-manifest routing gate, and the M3 thinking policy — and
-/// matches the configuration the ab-v9 benchmarks ran with (op-smoke
-/// has always passed `OPENPENCIL_ORCHESTRATOR_MODEL` through).
-fn selected_builtin_model(state: &EditorState) -> Option<String> {
+/// Resolve the selected chat model's capability id for the orchestrator.
+/// Built-in (API-key) agents expose their concrete model id. ACP entries
+/// preserve their `acp:<id>` catalog identity so the model-profile resolver
+/// can choose its conservative weak-agent default instead of treating a
+/// missing id as Full tier. The ACP marker is not a transport model override:
+/// `selected_cli_model_id` still yields `None` for ACP providers.
+///
+/// Fixed CLI agents keep choosing their own model internally and yield `None`
+/// here (the CLI-side selection rides `ChatProviderLlmClient::with_model`
+/// instead). The returned id feeds model-aware orchestrator policy —
+/// tier-gated skill filtering, the element-manifest routing gate, and the M3
+/// thinking policy — and matches the configuration the ab-v9 benchmarks ran
+/// with (op-smoke has always passed `OPENPENCIL_ORCHESTRATOR_MODEL` through).
+fn selected_orchestrator_model(state: &EditorState) -> Option<String> {
     let entry = state.chat.selected_model_entry()?;
+    if entry.acp_agent_id().is_some() {
+        return Some(entry.value.clone());
+    }
     let id = entry.builtin_provider_id.as_deref()?;
     state
         .editor_ui
@@ -29,7 +37,7 @@ pub(crate) fn build_design_request(
 ) -> DesignRequest {
     DesignRequest {
         prompt,
-        model: selected_builtin_model(state),
+        model: selected_orchestrator_model(state),
         provider: None,
         design_md: state.doc.design_md.clone(),
         // Detected by `chat_intent::detect_append_intent` when the
@@ -160,4 +168,19 @@ mod tests {
         // M3 thinking policy — must match the agent the session will call.
         assert_eq!(req.model.as_deref(), Some("MiniMax-M3"));
     }
+
+    #[test]
+    fn selected_acp_agent_reaches_the_orchestrator_as_basic_tier() {
+        let mut state = EditorState::new();
+        state.chat.available_models = vec![ModelEntry::acp("custom/vendor", "Custom ACP")];
+        state.chat.selected_model = 0;
+
+        let req = build_design_request("draw a dashboard".into(), &state, None);
+
+        assert_eq!(req.model.as_deref(), Some("acp:custom/vendor"));
+        assert_eq!(
+            op_orchestrator::resolve_model_profile(req.model.as_deref().unwrap()).tier,
+            op_orchestrator::ModelTier::Basic
+        );
+    }
 }

crates/op-host-desktop/src/chat_session_launch.rs

@@ -26,6 +26,23 @@ use super::ChatSession;
 mod chat_design_request;
 use chat_design_request::build_design_request;
 
+/// Build the model-aware request while the live chat selection is still
+/// attached, then take the narrowed worker snapshot.
+///
+/// `narrowed_snapshot` deliberately detaches `EditorState::chat`; reversing
+/// these two operations therefore erases the selected builtin/ACP identity
+/// and makes the orchestrator resolve `model=None` as Full tier.
+fn prepare_design_request_and_snapshot(
+    host: &mut WidgetHostNative,
+    prompt: String,
+    append_context: Option<op_orchestrator::AppendContext>,
+) -> (op_orchestrator::DesignRequest, EditorState) {
+    let request = build_design_request(prompt, host.editor_state(), append_context);
+    let initial_state =
+        op_editor_core::request_snapshot::narrowed_snapshot(host.editor_state_mut());
+    (request, initial_state)
+}
+
 // Design-agent-loop helpers (flag gate + provider builder + turn launcher)
 // split out at the 800-line cap; see module docs there.
 #[path = "chat_session_launch_design.rs"]
@@ -115,14 +132,17 @@ pub fn launch_if_pending(
                 host.editor_state(),
                 &effective_user_text,
             );
+            let (request, initial_state) = prepare_design_request_and_snapshot(
+                host,
+                effective_user_text.clone(),
+                append_context,
+            );
             // Narrowed clone — this becomes the design worker's
             // `RemoteDocSink` mirror, which is only ever read through
             // `DocSink::state()` (`active_children` / `doc` / `components`).
             // See `op_editor_core::request_snapshot` for the field audit.
-            let initial_state =
-                op_editor_core::request_snapshot::narrowed_snapshot(host.editor_state_mut());
-            let request =
-                build_design_request(effective_user_text.clone(), &initial_state, append_context);
+            // The request above must be built first because that snapshot
+            // intentionally detaches chat/model-selection state.
             // Persist the request onto the turn's assistant bubble (already
             // pushed by `begin_send`) BEFORE it moves into the worker — the
             // manual per-subtask "Retry" button needs it to re-run a failed
@@ -420,14 +440,13 @@ fn launch_cli_standard_turn(
     let system_prompt = build_chat_system_prompt(state, user_text);
     let modify_plan = op_host_services::chat_intent::build_modify_plan(state, user_text);
     let append_context = op_host_services::chat_intent::detect_append_intent(state, user_text);
+    let (design_request, initial_state) =
+        prepare_design_request_and_snapshot(host, user_text.to_string(), append_context);
     // Narrowed clone — `CliTurnPlan::initial_state` ends up as the design
     // worker's `RemoteDocSink` mirror, read only through `DocSink::state()`.
-    // See `op_editor_core::request_snapshot` for the field audit. Takes the
-    // mutable borrow, so it must come after the last read of `state`.
-    let initial_state =
-        op_editor_core::request_snapshot::narrowed_snapshot(host.editor_state_mut());
-    let design_request =
-        build_design_request(user_text.to_string(), &initial_state, append_context);
+    // See `op_editor_core::request_snapshot` for the field audit. Preparation
+    // takes the mutable borrow, so it must come after the last read of `state`;
+    // it builds the model-aware request before detaching chat for the snapshot.
     // Same stash as the builtin/design-intent path above — this turn may or
     // may not actually classify as `DesignIntent::New` on the worker (the
     // classifier runs async), but setting it unconditionally is harmless:

crates/op-host-desktop/src/chat_session_launch_tests.rs

@@ -2,7 +2,10 @@
 //! 800-line-per-file ceiling (mirrors `chat_session_launch_selection_tests.rs`).
 
 use super::*;
-use op_editor_core::pen_node_ext::PenNodeExt;
+use op_editor_core::{
+    pen_node_ext::PenNodeExt, AgentProvider, BuiltinAgentConfig, BuiltinAgentKind,
+    BuiltinAgentPresetKey, ModelEntry,
+};
 
 fn frame(
     id: &str,
@@ -97,6 +100,86 @@ fn stash_design_request_for_retry_writes_json_onto_the_last_message() {
     assert_eq!(restored.prompt, "design a login page");
 }
 
+#[test]
+fn design_launch_preparation_captures_acp_tier_before_detaching_chat() {
+    let mut host = WidgetHostNative::new();
+    host.editor_state_mut().chat.available_models =
+        vec![ModelEntry::acp("custom/vendor", "Custom ACP")];
+    host.editor_state_mut().chat.selected_model = 0;
+    let expected_revision = host.editor_state().document_revision();
+    let expected_root_count = host.editor_state().active_children().len();
+
+    let (request, initial_state) =
+        prepare_design_request_and_snapshot(&mut host, "draw a dashboard".into(), None);
+
+    assert_eq!(request.model.as_deref(), Some("acp:custom/vendor"));
+    assert_eq!(
+        op_orchestrator::resolve_model_profile(request.model.as_deref().unwrap()).tier,
+        op_orchestrator::ModelTier::Basic
+    );
+    assert!(
+        initial_state.chat.selected_model_entry().is_none(),
+        "the worker snapshot must stay narrowed"
+    );
+    assert_eq!(initial_state.document_revision(), expected_revision);
+    assert_eq!(initial_state.active_children().len(), expected_root_count);
+    assert_eq!(
+        host.editor_state()
+            .chat
+            .selected_model_entry()
+            .and_then(ModelEntry::acp_agent_id),
+        Some("custom/vendor"),
+        "snapshot preparation must restore the live chat selection"
+    );
+}
+
+#[test]
+fn design_launch_preparation_captures_builtin_model_before_detaching_chat() {
+    let mut host = WidgetHostNative::new();
+    host.editor_state_mut()
+        .editor_ui
+        .agent_settings
+        .builtin_agents
+        .push(BuiltinAgentConfig {
+            id: "builtin-1".into(),
+            preset: BuiltinAgentPresetKey::Custom,
+            display_name: "MiniMax".into(),
+            kind: BuiltinAgentKind::OpenAiCompat,
+            api_key: "sk-test".into(),
+            model: "MiniMax-M3".into(),
+            base_url: "http://localhost:9".into(),
+            enabled: true,
+        });
+    host.editor_state_mut().chat.available_models = vec![ModelEntry::builtin(
+        AgentProvider::ClaudeCode,
+        "builtin-1",
+        "builtin:builtin-1:MiniMax-M3",
+        "MiniMax M3",
+    )];
+    host.editor_state_mut().chat.selected_model = 0;
+
+    let (request, initial_state) =
+        prepare_design_request_and_snapshot(&mut host, "draw a dashboard".into(), None);
+
+    assert_eq!(request.model.as_deref(), Some("MiniMax-M3"));
+    assert_eq!(
+        op_orchestrator::resolve_model_profile(request.model.as_deref().unwrap()).tier,
+        op_orchestrator::ModelTier::Full
+    );
+    assert!(
+        initial_state.chat.selected_model_entry().is_none(),
+        "the worker snapshot must stay narrowed"
+    );
+    assert_eq!(
+        host.editor_state()
+            .chat
+            .selected_model_entry()
+            .and_then(|entry| entry.builtin_provider_id.as_deref()),
+        Some("builtin-1"),
+        "snapshot preparation must restore the live builtin selection"
+    );
+}
+
 /// Regression lock for a real bug a user hit: the CLI-standard route
 /// (`launch_cli_standard_turn`, reached whenever no builtin/ACP model is
 /// selected — the common case) never stashed `design_request_json_for_retry`

crates/op-host-services/src/chat_provider_llm.rs

@@ -98,6 +98,7 @@ impl LlmClient for ChatProviderLlmClient {
         let m3_keeps_thinking = req
             .model
             .as_deref()
+            .filter(|model| !op_orchestrator::is_acp_capability_marker(model))
             .map(|m| m.to_ascii_lowercase().contains("minimax-m3"))
             .unwrap_or(false);
         let chat_req = ChatRequest {
@@ -227,6 +228,14 @@ mod tests {
 
         let unknown = call_with_model(None);
         assert_eq!(unknown.thinking, ThinkingMode::Disabled);
+
+        let acp_marker = call_with_model(Some("acp:minimax-m3-wrapper"));
+        assert_eq!(acp_marker.thinking, ThinkingMode::Disabled);
+        assert_eq!(acp_marker.max_output_tokens, 16384);
+        assert_eq!(
+            acp_marker.model, None,
+            "ACP capability identity must not become a transport model"
+        );
     }
 
     // ── Provider-shape agnosticism ───────────────────────────────────────

crates/op-host-services/src/validation_providers.rs

@@ -153,7 +153,7 @@ impl ChatVisionLlmClient {
 
     /// Attach the vision model id to every request this client issues.
     pub fn with_model(mut self, model: Option<String>) -> Self {
-        self.model = model;
+        self.model = Self::transport_model(model);
         self
     }
 
@@ -171,6 +171,13 @@ impl ChatVisionLlmClient {
             data,
         })
     }
+
+    /// Keep ACP catalog identities inside orchestrator capability policy.
+    /// They name an agent, not a provider model, and therefore must collapse
+    /// to the provider default at this transport boundary.
+    fn transport_model(model: Option<String>) -> Option<String> {
+        model.filter(|id| !op_orchestrator::is_acp_capability_marker(id))
+    }
 }
 
 impl VisionLlmClient for ChatVisionLlmClient {
@@ -207,7 +214,8 @@ impl VisionLlmClient for ChatVisionLlmClient {
             thinking: ThinkingMode::Disabled,
             effort: EffortLevel::Low,
             attachments: vec![attachment],
-            model: self.model.clone().or_else(|| req.model.clone()),
+            model: Self::transport_model(self.model.clone())
+                .or_else(|| Self::transport_model(req.model.clone())),
         };
 
         // `provider.send` is a blocking delta iterator (the same shape the
@@ -401,6 +409,29 @@ mod tests {
         assert_eq!(r.model.as_deref(), Some("vision-model"));
     }
 
+    #[test]
+    fn chat_vision_client_does_not_forward_acp_capability_marker_as_model() {
+        let seen = Arc::new(Mutex::new(Vec::new()));
+        let provider = Arc::new(RecordingVisionProvider {
+            seen: seen.clone(),
+            reply: r#"{"issues":[],"fixes":[],"qualityScore":9}"#.into(),
+        });
+        let client =
+            ChatVisionLlmClient::new(provider).with_model(Some("acp:custom/vendor".to_string()));
+        let mut request = vision_req(&b64_png());
+        request.model = Some("acp:custom/vendor".to_string());
+
+        let response = client.validate(request);
+
+        assert!(matches!(response, VisionResponse::Text(_)));
+        let requests = seen.lock().unwrap();
+        assert_eq!(
+            requests.first().expect("provider was called").model,
+            None,
+            "ACP catalog identity is a capability marker, not a transport model"
+        );
+    }
+
     /// A non-base64 screenshot string can't drive a vision call → Skipped.
     #[test]
     fn chat_vision_client_skips_on_bad_base64() {

crates/op-orchestrator/src/lib.rs

@@ -127,7 +127,7 @@ pub use design_type::{detect_design_type, DesignType, DesignTypePreset};
 pub use intent::classify_intent;
 pub use loop_finalize::{apply_loop_finalize, apply_loop_finalize_counted};
 pub use mobile_reflow::repair_mobile_trailing_nav_reflow;
-pub use model_profile::{resolve_model_profile, ModelProfile, ModelTier};
+pub use model_profile::{is_acp_capability_marker, resolve_model_profile, ModelProfile, ModelTier};
 pub use prompt::build_orchestrator_prompt;
 pub use repair_summary::{CheckCategory, RepairSummary};
 pub use run::Orchestrator;

crates/op-orchestrator/src/model_profile.rs

@@ -48,6 +48,26 @@ const DEFAULT_PROFILE: ModelProfile = ModelProfile {
     label: "Unknown model",
 };
 
+/// ACP agents do not expose their backing model to OpenPencil. Treat their
+/// catalog identity conservatively instead of promoting a missing model id to
+/// the Full-tier default.
+const ACP_PROFILE: ModelProfile = ModelProfile {
+    tier: ModelTier::Basic,
+    thinking_disabled: true,
+    timeout_multiplier: 1.0,
+    label: "ACP agent",
+};
+
+/// Whether `model_id` is the catalog identity of an ACP agent rather than a
+/// concrete provider model. Hosts may carry this marker through
+/// [`crate::DesignRequest`] for capability policy, but must not forward it as a
+/// transport model override.
+pub fn is_acp_capability_marker(model_id: &str) -> bool {
+    model_id
+        .get(..4)
+        .is_some_and(|prefix| prefix.eq_ignore_ascii_case("acp:"))
+}
+
 /// 模型表 —— verbatim 移植自 `model-profiles.ts:22-95`,首个命中胜出。
 const MODEL_PROFILES: &[Entry] = &[
     // Full tier
@@ -215,8 +235,10 @@ const fn e(matcher: Match, tier: ModelTier, thinking_disabled: bool, label: &'st
     }
 }
 
-/// 解析模型 id → profile。strip `provider/` 前缀 → 小写 → 首个命中。
-/// 空 id → 强制 `Full`(TS 行为);无命中 → `DEFAULT_PROFILE`。
+/// Resolve a model id to its capability profile. ACP catalog ids use the
+/// conservative `Basic` default. Other ids strip a `provider/` prefix, then
+/// match the lower-cased model table. An empty id keeps the legacy forced
+/// `Full` behavior; an unmatched non-empty id uses [`DEFAULT_PROFILE`].
 pub fn resolve_model_profile(model_id: &str) -> ModelProfile {
     if model_id.is_empty() {
         return ModelProfile {
@@ -226,6 +248,11 @@ pub fn resolve_model_profile(model_id: &str) -> ModelProfile {
             label: "Default (no model)",
         };
     }
+    // Check before stripping a provider prefix: ACP ids are opaque and may
+    // themselves contain `/` (for example `acp:vendor/custom-agent`).
+    if is_acp_capability_marker(model_id) {
+        return ACP_PROFILE;
+    }
     let normalized = match model_id.find('/') {
         Some(i) => &model_id[i + 1..],
         None => model_id,
@@ -284,6 +311,10 @@ mod tests {
         assert_eq!(resolve_model_profile("minimax-01").tier, ModelTier::Basic);
         assert_eq!(resolve_model_profile("glm-4-plus").tier, ModelTier::Basic);
         assert_eq!(resolve_model_profile("qwen-max").tier, ModelTier::Basic);
+        assert_eq!(
+            resolve_model_profile("acp:vendor/custom-agent").tier,
+            ModelTier::Basic
+        );
     }
 
     #[test]