From 1c6e01f8cac9a550d5b020a151f1484862b30cd5 Mon Sep 17 00:00:00 2001 From: xy Date: Mon, 19 Oct 2026 18:06:30 +0800 Subject: [PATCH 1/6] fix(pi): honor enabledModels in the model selector --- src-tauri/src/acp/connection.rs | 277 ++++++++++++++++++++++++++++- src-tauri/src/acp/session_state.rs | 7 + src-tauri/src/commands/acp.rs | 77 ++++++++ 3 files changed, 354 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index 4ef98966ec..f9c99eed37 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -2564,6 +2564,8 @@ pub async fn spawn_agent_connection( terminal_shell_config: TerminalShellRuntimeConfig, cli_root: Option, ) -> Result, AcpError> { + // Use the same cwd for launch settings, the subprocess and session/new. + let launch_cwd = resolve_working_dir(working_dir.as_deref()); // Create the authoritative session state up front. Subsequent emit_with_state // calls write through this state and increment its seq counter so the first // event the frontend sees has seq=1, not the placeholder 0 from Phase 0. @@ -2593,6 +2595,10 @@ pub async fn spawn_agent_connection( initial_state.cli_run = Some(seed.tag); initial_state.cli_output_schema = seed.output_schema; } + if agent_type == AgentType::Pi { + initial_state.pi_enabled_model_patterns = + crate::commands::acp::pi_enabled_model_patterns_for_env(&runtime_env, &launch_cwd); + } let session_state = Arc::new(RwLock::new(initial_state)); @@ -2612,12 +2618,6 @@ pub async fn spawn_agent_connection( crate::commands::acp::reconcile_hermes_runtime_env(&runtime_env); } - // Resolve the launch cwd from the same `working_dir` (via the same helper) - // that run_connection uses for the session/new request, so the process - // cwd, the ACP session cwd, and any os.getcwd()-derived agent state all - // agree. Computed here because `working_dir` is moved into run_connection - // below. - let launch_cwd = resolve_working_dir(working_dir.as_deref()); // Shared cell that receives the agent process's OS pid the instant it // spawns (via `on_spawn` below). Stored on the `AgentConnection` so the // shutdown path can `kill_tree` the process tree synchronously as a @@ -3823,6 +3823,128 @@ fn visible_config_options( .collect() } +const PI_THINKING_LEVEL_SUFFIXES: &[&str] = + &["off", "minimal", "low", "medium", "high", "xhigh", "max"]; + +fn pi_model_pattern_without_thinking_level(pattern: &str) -> &str { + let Some((model, level)) = pattern.rsplit_once(':') else { + return pattern; + }; + if model.is_empty() || !PI_THINKING_LEVEL_SUFFIXES.contains(&level) { + pattern + } else { + model + } +} + +fn pi_model_glob_matches(value: &str, pattern: &str) -> bool { + let value = value.chars().collect::>(); + let pattern = pattern.chars().collect::>(); + let mut value_index = 0; + let mut pattern_index = 0; + let mut star_index = None; + let mut star_value_index = 0; + while value_index < value.len() { + if let Some(candidate) = pattern.get(pattern_index) { + if *candidate == '*' { + star_index = Some(pattern_index); + star_value_index = value_index; + pattern_index += 1; + } else if *candidate == '?' || candidate.eq_ignore_ascii_case(&value[value_index]) { + value_index += 1; + pattern_index += 1; + } else { + let Some(star) = star_index else { + return false; + }; + star_value_index += 1; + value_index = star_value_index; + pattern_index = star + 1; + } + } else { + let Some(star) = star_index else { + return false; + }; + star_value_index += 1; + value_index = star_value_index; + pattern_index = star + 1; + } + } + pattern[pattern_index..].iter().all(|ch| *ch == '*') +} + +fn pi_model_pattern_matches(option: &SessionConfigSelectOption, pattern: &str) -> bool { + let raw_pattern = pattern.trim(); + if raw_pattern.is_empty() { + return false; + } + let value = option.value.to_string(); + let bare_id = value + .split_once('/') + .map(|(_, id)| id) + .unwrap_or(value.as_str()); + if value.eq_ignore_ascii_case(raw_pattern) || bare_id.eq_ignore_ascii_case(raw_pattern) { + return true; + } + let pattern = pi_model_pattern_without_thinking_level(raw_pattern); + if pattern.contains('*') || pattern.contains('?') { + pi_model_glob_matches(&value, pattern) || pi_model_glob_matches(bare_id, pattern) + } else { + value.eq_ignore_ascii_case(pattern) || bare_id.eq_ignore_ascii_case(pattern) + } +} + +/// Apply Pi's native `enabledModels` scope to the model selector advertised by +/// pi-acp. Keep non-model options untouched and leave the list unrestricted +/// when no configured pattern matches an available option. +fn filter_pi_enabled_model_options( + patterns: &[String], + config_options: Vec, +) -> Vec { + if patterns.is_empty() { + return config_options; + } + config_options + .into_iter() + .map(|mut option| { + if !is_model_config_option(&option) { + return option; + } + let SessionConfigKind::Select(select) = &mut option.kind else { + return option; + }; + let matches = |option: &SessionConfigSelectOption| { + patterns + .iter() + .any(|pattern| pi_model_pattern_matches(option, pattern)) + }; + let has_match = match &select.options { + SessionConfigSelectOptions::Ungrouped(options) => options.iter().any(&matches), + SessionConfigSelectOptions::Grouped(groups) => { + groups + .iter() + .any(|group| group.options.iter().any(&matches)) + } + _ => false, + }; + if !has_match { + return option; + } + match &mut select.options { + SessionConfigSelectOptions::Ungrouped(options) => options.retain(&matches), + SessionConfigSelectOptions::Grouped(groups) => { + for group in groups.iter_mut() { + group.options.retain(&matches); + } + groups.retain(|group| !group.options.is_empty()); + } + _ => {} + } + option + }) + .collect() +} + async fn emit_session_config_options_values( state: &Arc>, emitter: &EventEmitter, @@ -3831,8 +3953,18 @@ async fn emit_session_config_options_values( // Drop what this launch froze BEFORE mapping, so every producer of this // event — establishment, the answer to a set, and the agent's own // `config_option_update` push — is filtered by one rule. - let pinned = state.read().await.env_pinned_config_option_ids.clone(); + let (pinned, pi_enabled_model_patterns) = { + let guard = state.read().await; + ( + guard.env_pinned_config_option_ids.clone(), + guard.pi_enabled_model_patterns.clone(), + ) + }; let visible = visible_config_options(&pinned, config_options); + let visible = match pi_enabled_model_patterns.as_deref() { + Some(patterns) => filter_pi_enabled_model_options(patterns, visible), + None => visible, + }; emit_with_state( state, emitter, @@ -35437,6 +35569,137 @@ mod tests { ); } + fn pi_model_selector_options() -> Vec { + serde_json::from_value(serde_json::json!([ + { + "type": "select", + "id": "model", + "name": "Model", + "category": "model", + "currentValue": "openai/gpt-5", + "options": [ + {"value": "openai/gpt-5", "name": "OpenAI/GPT-5"}, + {"value": "openai/gpt-4o", "name": "OpenAI/GPT-4o"}, + {"value": "anthropic/claude-opus", "name": "Anthropic/Claude Opus"} + ] + }, + { + "type": "select", + "id": "thought_level", + "name": "Thinking", + "category": "thought_level", + "currentValue": "high", + "options": [{"value": "high", "name": "High"}] + } + ])) + .expect("pi model selector fixture") + } + + fn pi_select_values(options: &[SessionConfigOption], id: &str) -> Vec { + options + .iter() + .find(|option| option.id.to_string() == id) + .and_then(|option| match &option.kind { + SessionConfigKind::Select(select) => match &select.options { + SessionConfigSelectOptions::Ungrouped(options) => Some( + options + .iter() + .map(|option| option.value.to_string()) + .collect(), + ), + _ => None, + }, + _ => None, + }) + .unwrap_or_default() + } + + #[test] + fn pi_enabled_models_filter_only_the_model_selector() { + let filtered = filter_pi_enabled_model_options( + &["openai/*".to_string()], + pi_model_selector_options(), + ); + + assert_eq!( + pi_select_values(&filtered, "model"), + vec!["openai/gpt-5", "openai/gpt-4o"] + ); + assert_eq!(pi_select_values(&filtered, "thought_level"), vec!["high"]); + } + + #[test] + fn pi_enabled_models_accept_exact_ids_and_thinking_suffixes() { + let filtered = filter_pi_enabled_model_options( + &["anthropic/claude-opus:high".to_string()], + pi_model_selector_options(), + ); + + assert_eq!( + pi_select_values(&filtered, "model"), + vec!["anthropic/claude-opus"] + ); + } + + #[test] + fn pi_enabled_models_leave_the_list_unrestricted_when_nothing_matches() { + let original = pi_model_selector_options(); + let filtered = + filter_pi_enabled_model_options(&["does-not-exist".to_string()], original); + + assert_eq!( + pi_select_values(&filtered, "model"), + vec!["openai/gpt-5", "openai/gpt-4o", "anthropic/claude-opus"] + ); + } + + #[test] + fn pi_enabled_models_match_bare_ids_and_wildcards() { + for pattern in ["gpt-5", "OPENAI/GPT-?", "*gpt-5*", "openai/*:high"] { + let filtered = filter_pi_enabled_model_options( + &[pattern.to_string()], + pi_model_selector_options(), + ); + let expected = if pattern == "openai/*:high" { + vec!["openai/gpt-5", "openai/gpt-4o"] + } else { + vec!["openai/gpt-5"] + }; + assert_eq!(pi_select_values(&filtered, "model"), expected, "{pattern}"); + } + } + + #[test] + fn pi_enabled_models_preserve_grouped_options_and_drop_empty_groups() { + let options = serde_json::from_value(serde_json::json!([{ + "type": "select", + "id": "model", + "name": "Model", + "category": "model", + "currentValue": "openai/gpt-5", + "options": [ + {"group": "OpenAI", "name": "OpenAI", "options": [ + {"value": "openai/gpt-5", "name": "GPT-5"}, + {"value": "openai/gpt-4o", "name": "GPT-4o"} + ]}, + {"group": "Anthropic", "name": "Anthropic", "options": [ + {"value": "anthropic/claude-opus", "name": "Claude Opus"} + ]} + ] + }])) + .expect("grouped model selector fixture"); + let filtered = filter_pi_enabled_model_options(&["openai/gpt-5".into()], options); + let SessionConfigKind::Select(select) = &filtered[0].kind else { + panic!("expected a model selector"); + }; + let SessionConfigSelectOptions::Grouped(groups) = &select.options else { + panic!("expected grouped model options"); + }; + assert_eq!(groups.len(), 1); + assert_eq!(groups[0].options.len(), 1); + assert_eq!(groups[0].options[0].value.to_string(), "openai/gpt-5"); + } + struct TestConversationDepthLookup; #[async_trait::async_trait] diff --git a/src-tauri/src/acp/session_state.rs b/src-tauri/src/acp/session_state.rs index 94813a08d4..f9b714535c 100644 --- a/src-tauri/src/acp/session_state.rs +++ b/src-tauri/src/acp/session_state.rs @@ -446,6 +446,12 @@ pub struct SessionState { /// /// Backend-internal — not serialized, not carried on `to_snapshot()`. pub trae_pat_sign_in: bool, + /// Pi's native `enabledModels` scope for this launch. The ACP adapter + /// advertises every authenticated model, so the connection layer applies + /// this Pi-only preference before publishing the model selector. + /// + /// Backend-internal — not serialized, not carried on `to_snapshot()`. + pub pi_enabled_model_patterns: Option>, pub prompt_capabilities: Option, pub fork_supported: bool, pub available_commands: Vec, @@ -846,6 +852,7 @@ impl SessionState { agent_chosen_config_values: BTreeMap::new(), env_pinned_config_option_ids: Vec::new(), trae_pat_sign_in: false, + pi_enabled_model_patterns: None, prompt_capabilities: None, fork_supported: false, available_commands: Vec::new(), diff --git a/src-tauri/src/commands/acp.rs b/src-tauri/src/commands/acp.rs index 1e7c39a4ff..21efe74527 100644 --- a/src-tauri/src/commands/acp.rs +++ b/src-tauri/src/commands/acp.rs @@ -6018,6 +6018,27 @@ fn account_home_dir() -> Option { dirs::home_dir() } +/// Read the launch's agent-directory `enabledModels` preference. Missing, +/// malformed or empty settings leave the model selector unrestricted. +pub(crate) fn pi_enabled_model_patterns_for_env( + runtime_env: &BTreeMap, + workspace: &Path, +) -> Option> { + let settings = read_json_object_or_empty( + &pi_agent_dir_in_workspace(runtime_env, workspace).join("settings.json"), + ); + let patterns = settings + .get("enabledModels") + .and_then(serde_json::Value::as_array)? + .iter() + .filter_map(serde_json::Value::as_str) + .map(str::trim) + .filter(|pattern| !pattern.is_empty()) + .map(str::to_string) + .collect::>(); + (!patterns.is_empty()).then_some(patterns) +} + /// Pi's agent dir as the pi CHILD of a launch with `runtime_env` resolves it — /// before a relative value is anchored (see [`pi_agent_dir_in_workspace`]). /// @@ -17175,6 +17196,62 @@ echo {"id":"codeg-models","type":"response","command":"get_available_models","su }); } + #[test] + fn pi_enabled_model_patterns_read_from_the_launch_agent_dir() { + let tmp = tempfile::tempdir().expect("tempdir"); + fs::write( + tmp.path().join("settings.json"), + r#"{"enabledModels":["openai/*"," anthropic/claude-opus:high ",42," "]}"#, + ) + .unwrap(); + let env = BTreeMap::from([( + String::from("PI_CODING_AGENT_DIR"), + tmp.path().to_string_lossy().into_owned(), + )]); + + assert_eq!( + pi_enabled_model_patterns_for_env(&env, tmp.path()), + Some(vec![ + "openai/*".to_string(), + "anthropic/claude-opus:high".to_string(), + ]) + ); + } + + #[test] + fn pi_enabled_model_patterns_treat_missing_or_empty_as_unscoped() { + let tmp = tempfile::tempdir().expect("tempdir"); + let env = BTreeMap::from([( + String::from("PI_CODING_AGENT_DIR"), + tmp.path().to_string_lossy().into_owned(), + )]); + + assert_eq!(pi_enabled_model_patterns_for_env(&env, tmp.path()), None); + + fs::write(tmp.path().join("settings.json"), r#"{"enabledModels":[]}"#).unwrap(); + assert_eq!(pi_enabled_model_patterns_for_env(&env, tmp.path()), None); + fs::write(tmp.path().join("settings.json"), "invalid json").unwrap(); + assert_eq!(pi_enabled_model_patterns_for_env(&env, tmp.path()), None); + } + + #[test] + fn pi_enabled_model_patterns_resolve_relative_agent_dir_in_workspace() { + let tmp = tempfile::tempdir().expect("tempdir"); + let agent_dir = tmp.path().join("agent"); + fs::create_dir(&agent_dir).unwrap(); + fs::write( + agent_dir.join("settings.json"), + r#"{"enabledModels":["openai/*"]}"#, + ) + .unwrap(); + let env = BTreeMap::from([("PI_CODING_AGENT_DIR".into(), "agent".into())]); + + assert_eq!( + pi_enabled_model_patterns_for_env(&env, tmp.path()), + Some(vec!["openai/*".into()]) + ); + } + #[test] fn opencode_auth_empty_payload_truncates_to_empty_object() { // Clearing the last credential sends "" — it must persist `{}` (clearing From 3955c434146aa44c0dc184652ec511ce4587ed3f Mon Sep 17 00:00:00 2001 From: xintaofei Date: Tue, 20 Oct 2026 01:13:54 +0800 Subject: [PATCH 2/6] fix(pi): keep the model list whole when enabledModels can't be mirrored pi resolves enabledModels with fuzzy id/name matching and minimatch, so an entry the filter read as "matches nothing" could still name a model in pi's scope, and filtering by the other entries hid that model with no other way to pick it in Codeg. Each entry is now classified the way pi 1.x resolves it: the rows it names exactly or by a `*`/`?` glob, an entry no model answers (skipped, as pi skips it), or an entry pi settles by a rule not mirrored here (fuzzy match, wider glob syntax), which keeps the whole list. A workspace whose own .pi/settings.json sets enabledModels also keeps the whole list, since pi uses that list once it trusts the project and only pi decides that at startup. The model in effect keeps its row when it falls outside the scope. --- src-tauri/src/acp/connection.rs | 390 +++++++++++++++++++++++++++++--- src-tauri/src/commands/acp.rs | 65 ++++++ 2 files changed, 419 insertions(+), 36 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index f9c99eed37..549818fbdc 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -3873,30 +3873,213 @@ fn pi_model_glob_matches(value: &str, pattern: &str) -> bool { pattern[pattern_index..].iter().all(|ch| *ch == '*') } -fn pi_model_pattern_matches(option: &SessionConfigSelectOption, pattern: &str) -> bool { - let raw_pattern = pattern.trim(); - if raw_pattern.is_empty() { - return false; +/// One advertised model row, split the way pi names the model behind it: +/// pi-acp publishes each of pi's models as value `provider/id` and label +/// `provider/name`. Lowercased once, because every comparison pi makes here is +/// case-insensitive. +struct PiModelRow { + value: String, + provider: String, + id: String, + name: String, +} + +impl PiModelRow { + fn new(option: &SessionConfigSelectOption) -> Self { + let value = option.value.to_string(); + let (provider, id) = value.split_once('/').unwrap_or(("", value.as_str())); + let name = option + .name + .strip_prefix(provider) + .and_then(|rest| rest.strip_prefix('/')) + .filter(|_| !provider.is_empty()) + .unwrap_or(&option.name); + Self { + provider: provider.to_lowercase(), + id: id.to_lowercase(), + name: name.to_lowercase(), + value, + } } - let value = option.value.to_string(); - let bare_id = value - .split_once('/') - .map(|(_, id)| id) - .unwrap_or(value.as_str()); - if value.eq_ignore_ascii_case(raw_pattern) || bare_id.eq_ignore_ascii_case(raw_pattern) { - return true; + + fn fuzzy_matches(&self, text: &str) -> bool { + self.id.contains(text) || self.name.contains(text) } - let pattern = pi_model_pattern_without_thinking_level(raw_pattern); - if pattern.contains('*') || pattern.contains('?') { - pi_model_glob_matches(&value, pattern) || pi_model_glob_matches(bare_id, pattern) - } else { - value.eq_ignore_ascii_case(pattern) || bare_id.eq_ignore_ascii_case(pattern) +} + +/// How one `enabledModels` entry resolves against the advertised rows, as far +/// as codeg can tell without running pi's own resolver. +#[derive(Debug, PartialEq, Eq)] +enum PiScopeEntry { + /// Indexes of the rows pi can pick for the entry: never fewer than pi + /// picks, at times more — codeg's `*` and `?` also cross `/`, and a + /// `provider/id` also takes another provider's model listed under that + /// whole text as its id. + Rows(Vec), + /// No row pi could pick: pi skips such an entry too (a retired id, a typo). + Dead, + /// pi decides this entry by a rule codeg does not mirror — a fuzzy id/name + /// match, or glob syntax beyond `*` and `?` — so its rows are unknown here. + Unknown, +} + +/// pi's exact reading of a model reference: `provider/id` (or that provider's +/// own literal id, like OpenRouter's `openrouter/free`) plus every model whose +/// bare id is the whole text. `lower` is the reference lowercased. +/// +/// Also answers whether pi would instead settle the named provider with a +/// fuzzy pick: when the text names a provider that lists neither, pi picks the +/// provider's closest id/name match before it looks at any other provider. +fn pi_exact_model_rows(lower: &str, rows: &[PiModelRow]) -> (Vec, bool) { + let mut exact = Vec::new(); + let mut named_fuzzily = false; + if let Some((provider, rest)) = lower.split_once('/') { + let (provider, rest) = (provider.trim(), rest.trim()); + let provider_rows = || { + rows.iter() + .enumerate() + .filter(|(_, row)| row.provider == provider) + }; + if !provider.is_empty() && !rest.is_empty() && provider_rows().next().is_some() { + exact.extend( + provider_rows() + .filter(|(_, row)| row.id == rest) + .map(|(i, _)| i), + ); + if exact.is_empty() { + exact.extend( + provider_rows() + .filter(|(_, row)| row.id == lower) + .map(|(i, _)| i), + ); + } + named_fuzzily = + exact.is_empty() && provider_rows().any(|(_, row)| row.fuzzy_matches(rest)); + } + } + for (index, row) in rows.iter().enumerate() { + if row.id == lower && !exact.contains(&index) { + exact.push(index); + } + } + (exact, named_fuzzily) +} + +/// Whether codeg's glob matcher reads `glob` the way pi's `minimatch` does, up +/// to its `*` and `?` also crossing `/` (so it only ever matches more rows). +/// Whatever minimatch reads more into is left out: character classes, brace +/// sets, extglobs, escapes, negation, comments, and its path rules — `**` +/// spanning zero or more segments, repeated slashes folding into one, `..` +/// undoing the segment before it. +fn pi_model_glob_is_supported(glob: &str) -> bool { + !glob.contains(['[', ']', '{', '}', '(', ')', '\\']) + && !glob.starts_with(['!', '#']) + && !glob.contains("**") + && !glob.contains("//") + && !glob + .split('/') + .any(|segment| segment == "." || segment == "..") +} + +/// Classify one `enabledModels` entry the way pi 1.x resolves a scope pattern +/// (`resolveModelScopeFromModels` in pi's `core/model-resolver.js`, checked +/// against 1.3.0): a pattern with `*`, `?` or `[` is a glob after an optional +/// thinking suffix, anything else a model reference. The one older rule left +/// out: pi 0.81 settled a bare id that several providers list with a single +/// fuzzy pick, where 1.x keeps all of them, as this does. +fn pi_scope_entry(pattern: &str, rows: &[PiModelRow]) -> PiScopeEntry { + if pattern.contains(['*', '?', '[']) { + let glob = pi_model_pattern_without_thinking_level(pattern).to_lowercase(); + // pi first takes a glob that is literally a model id (`model[1m]`). + let (exact, _) = pi_exact_model_rows(glob.trim(), rows); + if !exact.is_empty() { + return PiScopeEntry::Rows(exact); + } + if !pi_model_glob_is_supported(&glob) { + return PiScopeEntry::Unknown; + } + let matched: Vec = rows + .iter() + .enumerate() + .filter(|(_, row)| { + pi_model_glob_matches(&row.value.to_lowercase(), &glob) + || pi_model_glob_matches(&row.id, &glob) + }) + .map(|(index, _)| index) + .collect(); + return if matched.is_empty() { + PiScopeEntry::Dead + } else { + PiScopeEntry::Rows(matched) + }; + } + let text = pattern.trim(); + if text.is_empty() { + return PiScopeEntry::Dead; + } + pi_scope_reference(&text.to_lowercase(), rows) +} + +/// pi's reference rule, in pi's order: an exact model, else a fuzzy id/name +/// match, else the same text without its last `:suffix` (a thinking level, or +/// — scope parsing being lenient — anything after the colon). +fn pi_scope_reference(lower: &str, rows: &[PiModelRow]) -> PiScopeEntry { + let (exact, named_fuzzily) = pi_exact_model_rows(lower, rows); + if named_fuzzily || (exact.is_empty() && rows.iter().any(|row| row.fuzzy_matches(lower))) { + return PiScopeEntry::Unknown; + } + if !exact.is_empty() { + return PiScopeEntry::Rows(exact); + } + match lower.rsplit_once(':') { + Some((base, _)) => pi_scope_reference(base, rows), + None => PiScopeEntry::Dead, } } +/// The advertised values pi's scope covers, or `None` when the list must stay +/// whole. +/// +/// Mirroring pi's resolver has to fail safe, because a row dropped here is a +/// model the user cannot pick in codeg at all: an entry whose rows codeg +/// cannot know keeps the whole list, while an entry pi would skip anyway is +/// skipped. An empty result keeps the whole list too — pi itself runs unscoped +/// when no entry resolves. +fn pi_enabled_model_scope( + patterns: &[String], + options: &SessionConfigSelectOptions, +) -> Option> { + let rows: Vec = match options { + SessionConfigSelectOptions::Ungrouped(options) => { + options.iter().map(PiModelRow::new).collect() + } + SessionConfigSelectOptions::Grouped(groups) => groups + .iter() + .flat_map(|group| group.options.iter()) + .map(PiModelRow::new) + .collect(), + _ => return None, + }; + let mut scope = HashSet::new(); + for pattern in patterns { + match pi_scope_entry(pattern, &rows) { + PiScopeEntry::Rows(found) => { + scope.extend(found.into_iter().map(|index| rows[index].value.clone())); + } + PiScopeEntry::Dead => {} + PiScopeEntry::Unknown => return None, + } + } + (!scope.is_empty()).then_some(scope) +} + /// Apply Pi's native `enabledModels` scope to the model selector advertised by -/// pi-acp. Keep non-model options untouched and leave the list unrestricted -/// when no configured pattern matches an available option. +/// pi-acp. Non-model options are untouched, and the list stays whole whenever +/// [`pi_enabled_model_scope`] cannot name the scope. +/// +/// The model in effect keeps its row even outside the scope (a resumed +/// session, or a saved pick replayed on connect), so the selector still shows +/// what the session runs; once the user switches away, the row goes too. fn filter_pi_enabled_model_options( patterns: &[String], config_options: Vec, @@ -3913,28 +4096,19 @@ fn filter_pi_enabled_model_options( let SessionConfigKind::Select(select) = &mut option.kind else { return option; }; - let matches = |option: &SessionConfigSelectOption| { - patterns - .iter() - .any(|pattern| pi_model_pattern_matches(option, pattern)) + let Some(scope) = pi_enabled_model_scope(patterns, &select.options) else { + return option; }; - let has_match = match &select.options { - SessionConfigSelectOptions::Ungrouped(options) => options.iter().any(&matches), - SessionConfigSelectOptions::Grouped(groups) => { - groups - .iter() - .any(|group| group.options.iter().any(&matches)) - } - _ => false, + let current = select.current_value.to_string(); + let keep = |row: &SessionConfigSelectOption| { + let value = row.value.to_string(); + value == current || scope.contains(&value) }; - if !has_match { - return option; - } match &mut select.options { - SessionConfigSelectOptions::Ungrouped(options) => options.retain(&matches), + SessionConfigSelectOptions::Ungrouped(options) => options.retain(&keep), SessionConfigSelectOptions::Grouped(groups) => { for group in groups.iter_mut() { - group.options.retain(&matches); + group.options.retain(&keep); } groups.retain(|group| !group.options.is_empty()); } @@ -35635,10 +35809,19 @@ mod tests { pi_model_selector_options(), ); + // The fixture runs on `openai/gpt-5`, outside this scope: its row stays. assert_eq!( pi_select_values(&filtered, "model"), - vec!["anthropic/claude-opus"] + vec!["openai/gpt-5", "anthropic/claude-opus"] ); + // pi's scope parsing is lenient: an unknown suffix still names the model. + for pattern in ["anthropic/claude-opus:high", "anthropic/claude-opus:turbo"] { + assert_eq!( + pi_scope_of(&[pattern], PI_SCOPE_ROWS), + Some(vec!["anthropic/claude-opus".to_string()]), + "{pattern}" + ); + } } #[test] @@ -35700,6 +35883,141 @@ mod tests { assert_eq!(groups[0].options[0].value.to_string(), "openai/gpt-5"); } + /// Rows the way pi-acp publishes them: value `provider/id`, label + /// `provider/name`. + const PI_SCOPE_ROWS: &[(&str, &str)] = &[ + ("openai/gpt-5", "openai/GPT-5"), + ("openai/gpt-4o", "openai/GPT-4o"), + ("anthropic/claude-opus", "anthropic/Claude Opus"), + ("openrouter/openai/gpt-5", "openrouter/OpenAI: GPT-5"), + ("openrouter/openrouter/free", "openrouter/Free Router"), + ("custom/model[1m]", "custom/Model 1M"), + ("custom/Äther-1", "custom/Äther 1"), + ]; + + /// The sorted values `patterns` scope `rows` to; `None` keeps the list whole. + fn pi_scope_of(patterns: &[&str], rows: &[(&str, &str)]) -> Option> { + let options: SessionConfigSelectOptions = serde_json::from_value(serde_json::Value::Array( + rows.iter() + .map(|(value, name)| serde_json::json!({"value": value, "name": name})) + .collect(), + )) + .expect("model rows"); + let patterns: Vec = patterns.iter().map(|pattern| pattern.to_string()).collect(); + pi_enabled_model_scope(&patterns, &options).map(|scope| { + let mut values: Vec = scope.into_iter().collect(); + values.sort(); + values + }) + } + + #[test] + fn pi_enabled_models_keep_the_whole_list_for_entries_pi_settles_fuzzily() { + // pi resolves each second entry by a fuzzy id/name match, which codeg + // does not mirror: scoping by the first alone would hide the model pi + // enables for the second, with no other way to pick it in codeg. + for fuzzy in ["opus", "anthropic/opus", "Claude Opus", "opus:high", "gpt"] { + assert_eq!( + pi_scope_of(&["openai/gpt-4o", fuzzy], PI_SCOPE_ROWS), + None, + "{fuzzy}" + ); + } + let filtered = filter_pi_enabled_model_options( + &["openai/gpt-5".to_string(), "opus".to_string()], + pi_model_selector_options(), + ); + assert_eq!( + pi_select_values(&filtered, "model"), + vec!["openai/gpt-5", "openai/gpt-4o", "anthropic/claude-opus"] + ); + } + + #[test] + fn pi_enabled_models_skip_entries_no_model_answers() { + // A retired id or a typo is skipped by pi as well, so the rest still scope. + assert_eq!( + pi_scope_of( + &[ + "openai/gpt-4o", + "anthropic/claude-sonnet-9", + "retired-model:high" + ], + PI_SCOPE_ROWS + ), + Some(vec!["openai/gpt-4o".to_string()]) + ); + } + + #[test] + fn pi_enabled_models_keep_the_whole_list_for_glob_syntax_beyond_star_and_question_mark() { + for glob in [ + "anthropic/claude-[a-z]*", + "openai/{gpt-5,gpt-4o}*", + "!openai/*", + "openai/@(gpt-5)*", + // minimatch's path rules: each of these matches `openai/gpt-5`. + "openai/**/gpt-*", + "openai//gpt-*", + "x/../openai/gpt-*", + ] { + assert_eq!( + pi_scope_of(&["openai/gpt-4o", glob], PI_SCOPE_ROWS), + None, + "{glob}" + ); + } + // Except a glob that is literally a model id, which pi takes as that model. + assert_eq!( + pi_scope_of(&["model[1m]"], PI_SCOPE_ROWS), + Some(vec!["custom/model[1m]".to_string()]) + ); + } + + #[test] + fn pi_enabled_models_read_a_provider_id_the_way_pi_does() { + // `provider/id` names that provider's model; another provider listing + // the whole text as its own id comes along (pi keeps only the named one). + assert_eq!( + pi_scope_of(&["openai/gpt-5"], PI_SCOPE_ROWS), + Some(vec![ + "openai/gpt-5".to_string(), + "openrouter/openai/gpt-5".to_string(), + ]) + ); + // A provider's own literal id beats a fuzzy pick inside that provider. + assert_eq!( + pi_scope_of(&["openrouter/free"], PI_SCOPE_ROWS), + Some(vec!["openrouter/openrouter/free".to_string()]) + ); + // pi's `*` stops at `/`; codeg's crosses it, so it only ever adds rows. + assert_eq!( + pi_scope_of(&["openrouter/*"], PI_SCOPE_ROWS), + Some(vec![ + "openrouter/openai/gpt-5".to_string(), + "openrouter/openrouter/free".to_string(), + ]) + ); + // Case is ignored beyond ASCII, as minimatch's `nocase` does. + assert_eq!( + pi_scope_of(&["CUSTOM/ä*"], PI_SCOPE_ROWS), + Some(vec!["custom/Äther-1".to_string()]) + ); + } + + #[test] + fn pi_enabled_models_keep_the_row_of_the_model_in_effect() { + // The fixture runs on `openai/gpt-5`; the scope names only gpt-4o. + let filtered = filter_pi_enabled_model_options( + &["openai/gpt-4o".to_string()], + pi_model_selector_options(), + ); + assert_eq!( + pi_select_values(&filtered, "model"), + vec!["openai/gpt-5", "openai/gpt-4o"] + ); + } + struct TestConversationDepthLookup; #[async_trait::async_trait] diff --git a/src-tauri/src/commands/acp.rs b/src-tauri/src/commands/acp.rs index 21efe74527..19c7a2779e 100644 --- a/src-tauri/src/commands/acp.rs +++ b/src-tauri/src/commands/acp.rs @@ -6020,10 +6020,30 @@ fn account_home_dir() -> Option { /// Read the launch's agent-directory `enabledModels` preference. Missing, /// malformed or empty settings leave the model selector unrestricted. +/// +/// So does a workspace that sets its own: a trusted project's +/// `.pi/settings.json` replaces the agent dir's list outright, and whether pi +/// trusts the project is settled inside pi at startup (`trust.json`, an +/// extension, `defaultProjectTrust` or a prompt). codeg cannot tell which list +/// that pi runs with, and the wrong one would hide models it offers. A project +/// file pi loads nothing from — unreadable, or JSON other than an object — +/// leaves the agent dir's list in force, as in pi; one codeg cannot parse +/// counts as setting its own, since pi's parser may still read it. pub(crate) fn pi_enabled_model_patterns_for_env( runtime_env: &BTreeMap, workspace: &Path, ) -> Option> { + if let Ok(raw) = fs::read_to_string(workspace.join(".pi").join("settings.json")) { + // pi reads past a byte-order mark; serde_json does not. + let raw = raw.strip_prefix('\u{feff}').unwrap_or(&raw); + match serde_json::from_str::(raw) { + Ok(serde_json::Value::Object(project)) if project.contains_key("enabledModels") => { + return None + } + Ok(_) => {} + Err(_) => return None, + } + } let settings = read_json_object_or_empty( &pi_agent_dir_in_workspace(runtime_env, workspace).join("settings.json"), ); @@ -17252,6 +17272,51 @@ echo {"id":"codeg-models","type":"response","command":"get_available_models","su ); } + #[test] + fn pi_enabled_model_patterns_yield_to_a_project_that_sets_its_own() { + let agent_dir = tempfile::tempdir().expect("agent dir"); + fs::write( + agent_dir.path().join("settings.json"), + r#"{"enabledModels":["openai/*"]}"#, + ) + .unwrap(); + let env = BTreeMap::from([( + String::from("PI_CODING_AGENT_DIR"), + agent_dir.path().to_string_lossy().into_owned(), + )]); + let workspace = tempfile::tempdir().expect("workspace"); + let project = workspace.path().join(".pi"); + fs::create_dir(&project).unwrap(); + let global = Some(vec!["openai/*".to_string()]); + + // A trusted project's list replaces the agent dir's — and an empty one + // unscopes pi — so whether pi trusts the project decides; codeg can't. + // A file codeg cannot parse might be one of those for pi's parser. + for own in [ + r#"{"enabledModels":["anthropic/*"]}"#, + r#"{"enabledModels":[]}"#, + "\u{feff}{\"enabledModels\":[\"anthropic/*\"]}", + "not json", + ] { + fs::write(project.join("settings.json"), own).unwrap(); + assert_eq!( + pi_enabled_model_patterns_for_env(&env, workspace.path()), + None, + "{own}" + ); + } + // A project file without the key, or JSON pi loads nothing from, + // leaves the agent dir's list in force. + for other in [r#"{"theme":"dark"}"#, "[]", "null"] { + fs::write(project.join("settings.json"), other).unwrap(); + assert_eq!( + pi_enabled_model_patterns_for_env(&env, workspace.path()), + global, + "{other}" + ); + } + } + #[test] fn opencode_auth_empty_payload_truncates_to_empty_object() { // Clearing the last credential sends "" — it must persist `{}` (clearing From 6c7e9b341dfa30d1b17e204a22197db7f9e3dae6 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Tue, 20 Oct 2026 01:51:14 +0800 Subject: [PATCH 3/6] fix(pi): match enabledModels globs the way minimatch does The glob mirror let `*` and `?` cross `/`, so a glob pi matched to nothing could still match a row here, and filtering by it hid every other model of a list pi leaves unscoped. Globs are now matched segment by segment with minimatch's dot rule. A glob outside ASCII `*`/`?` syntax, or one with a leading drive letter, keeps the whole list. So does a row minimatch reads differently (non-ASCII, a backslash, an empty, `.` or `..` segment) that the glob may match, since minimatch counts `?` in UTF-16 units, folds case its own way, folds repeated slashes and lets a final `/` go. --- src-tauri/src/acp/connection.rs | 185 ++++++++++++++++++++++++++------ 1 file changed, 153 insertions(+), 32 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index 549818fbdc..88aa551d4b 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -3873,6 +3873,46 @@ fn pi_model_glob_matches(value: &str, pattern: &str) -> bool { pattern[pattern_index..].iter().all(|ch| *ch == '*') } +/// minimatch's verdict, with pi's `nocase` and its own default `dot: false`, +/// for a glob [`pi_model_glob_is_supported`] accepts on a path +/// [`PiModelRow::glob_plain`] calls plain: segment by segment, so `*` and `?` +/// stop at `/`, and a segment that starts with `.` only matches a literal `.`. +fn pi_minimatch(path: &str, glob: &str) -> bool { + let path: Vec<&str> = path.split('/').collect(); + let glob: Vec<&str> = glob.split('/').collect(); + path.len() == glob.len() + && path.iter().zip(&glob).all(|(segment, pattern)| { + !(segment.starts_with('.') && pattern.starts_with(['*', '?'])) + && pi_model_glob_matches(segment, pattern) + }) +} + +/// Whether minimatch could match a supported glob on a path that is not plain +/// — never `false` where minimatch matches. `?` stands for any run (minimatch +/// counts UTF-16 units), `*` crosses `/`, and the path is lowercased (as +/// minimatch's own shortcuts do) with runs of `/` folded into one and a final +/// `/` optional, as minimatch reads a path; a `\` is tried both as itself and +/// as the separator it is on Windows. +fn pi_minimatch_may_match(path: &str, glob: &str) -> bool { + let glob = glob.replace('?', "*"); + let lower = path.to_lowercase(); + [false, true].into_iter().any(|backslash_separates| { + let mut folded = String::with_capacity(lower.len()); + for ch in lower.chars() { + let ch = if backslash_separates && ch == '\\' { + '/' + } else { + ch + }; + if !(ch == '/' && folded.ends_with('/')) { + folded.push(ch); + } + } + pi_model_glob_matches(&folded, &glob) + || pi_model_glob_matches(folded.strip_suffix('/').unwrap_or(&folded), &glob) + }) +} + /// One advertised model row, split the way pi names the model behind it: /// pi-acp publishes each of pi's models as value `provider/id` and label /// `provider/name`. Lowercased once, because every comparison pi makes here is @@ -3882,6 +3922,12 @@ struct PiModelRow { provider: String, id: String, name: String, + /// Whether minimatch reads the value (and so the id) the way + /// [`pi_minimatch`] does: ASCII — beyond it minimatch counts `?` in UTF-16 + /// units and folds case its own way — with no `\` (a separator on + /// Windows), no empty segment (minimatch folds repeated slashes and lets a + /// final `/` go) and no `.` or `..` segment (which no glob matches). + glob_plain: bool, } impl PiModelRow { @@ -3894,10 +3940,16 @@ impl PiModelRow { .and_then(|rest| rest.strip_prefix('/')) .filter(|_| !provider.is_empty()) .unwrap_or(&option.name); + let glob_plain = value.is_ascii() + && !value.contains('\\') + && !value + .split('/') + .any(|segment| matches!(segment, "" | "." | "..")); Self { provider: provider.to_lowercase(), id: id.to_lowercase(), name: name.to_lowercase(), + glob_plain, value, } } @@ -3912,9 +3964,9 @@ impl PiModelRow { #[derive(Debug, PartialEq, Eq)] enum PiScopeEntry { /// Indexes of the rows pi can pick for the entry: never fewer than pi - /// picks, at times more — codeg's `*` and `?` also cross `/`, and a - /// `provider/id` also takes another provider's model listed under that - /// whole text as its id. + /// picks, at times more — a `provider/id` also takes another provider's + /// model listed under that whole text as its id — and never any when pi + /// picks none. Rows(Vec), /// No row pi could pick: pi skips such an entry too (a retired id, a typo). Dead, @@ -3965,14 +4017,17 @@ fn pi_exact_model_rows(lower: &str, rows: &[PiModelRow]) -> (Vec, bool) { (exact, named_fuzzily) } -/// Whether codeg's glob matcher reads `glob` the way pi's `minimatch` does, up -/// to its `*` and `?` also crossing `/` (so it only ever matches more rows). -/// Whatever minimatch reads more into is left out: character classes, brace -/// sets, extglobs, escapes, negation, comments, and its path rules — `**` -/// spanning zero or more segments, repeated slashes folding into one, `..` -/// undoing the segment before it. +/// Whether `glob` stays inside what [`pi_minimatch`] reads exactly as +/// minimatch does: ASCII, with `*` and `?` its only magic. Left out are +/// character classes, brace sets, extglobs, escapes, negation, comments, and +/// minimatch's path rules — `**` spanning zero or more segments, repeated +/// slashes folding into one, `..` undoing the segment before it, and a leading +/// drive letter (`c:`), whose segment Windows minimatch takes literally. fn pi_model_glob_is_supported(glob: &str) -> bool { - !glob.contains(['[', ']', '{', '}', '(', ')', '\\']) + let drive = matches!(glob.as_bytes(), [letter, b':', ..] if letter.is_ascii_alphabetic()); + !drive + && glob.is_ascii() + && !glob.contains(['[', ']', '{', '}', '(', ')', '\\']) && !glob.starts_with(['!', '#']) && !glob.contains("**") && !glob.contains("//") @@ -3989,24 +4044,28 @@ fn pi_model_glob_is_supported(glob: &str) -> bool { /// fuzzy pick, where 1.x keeps all of them, as this does. fn pi_scope_entry(pattern: &str, rows: &[PiModelRow]) -> PiScopeEntry { if pattern.contains(['*', '?', '[']) { - let glob = pi_model_pattern_without_thinking_level(pattern).to_lowercase(); + let glob = pi_model_pattern_without_thinking_level(pattern); // pi first takes a glob that is literally a model id (`model[1m]`). - let (exact, _) = pi_exact_model_rows(glob.trim(), rows); + let (exact, _) = pi_exact_model_rows(&glob.trim().to_lowercase(), rows); if !exact.is_empty() { return PiScopeEntry::Rows(exact); } - if !pi_model_glob_is_supported(&glob) { + if !pi_model_glob_is_supported(glob) { return PiScopeEntry::Unknown; } - let matched: Vec = rows - .iter() - .enumerate() - .filter(|(_, row)| { - pi_model_glob_matches(&row.value.to_lowercase(), &glob) - || pi_model_glob_matches(&row.id, &glob) - }) - .map(|(index, _)| index) - .collect(); + // pi matches the glob against `provider/id` and against the bare id. + let mut matched = Vec::new(); + for (index, row) in rows.iter().enumerate() { + if row.glob_plain { + if pi_minimatch(&row.value, glob) || pi_minimatch(&row.id, glob) { + matched.push(index); + } + } else if pi_minimatch_may_match(&row.value, glob) + || pi_minimatch_may_match(&row.id, glob) + { + return PiScopeEntry::Unknown; + } + } return if matched.is_empty() { PiScopeEntry::Dead } else { @@ -4041,10 +4100,11 @@ fn pi_scope_reference(lower: &str, rows: &[PiModelRow]) -> PiScopeEntry { /// whole. /// /// Mirroring pi's resolver has to fail safe, because a row dropped here is a -/// model the user cannot pick in codeg at all: an entry whose rows codeg -/// cannot know keeps the whole list, while an entry pi would skip anyway is -/// skipped. An empty result keeps the whole list too — pi itself runs unscoped -/// when no entry resolves. +/// model the user cannot pick in codeg at all. So the result never leaves out +/// a model pi's scope holds, and never scopes a list pi leaves unscoped (pi +/// runs unscoped when no entry resolves): an entry whose rows codeg cannot +/// know keeps the whole list, an entry pi would skip is skipped, and an empty +/// result keeps the whole list. fn pi_enabled_model_scope( patterns: &[String], options: &SessionConfigSelectOptions, @@ -35990,18 +36050,79 @@ mod tests { pi_scope_of(&["openrouter/free"], PI_SCOPE_ROWS), Some(vec!["openrouter/openrouter/free".to_string()]) ); - // pi's `*` stops at `/`; codeg's crosses it, so it only ever adds rows. + } + + #[test] + fn pi_enabled_models_match_globs_segment_by_segment_like_minimatch() { + // A glob tries `provider/id` and the bare id; `*` stops at `/`. assert_eq!( - pi_scope_of(&["openrouter/*"], PI_SCOPE_ROWS), + pi_scope_of(&["openai/*"], PI_SCOPE_ROWS), Some(vec![ + "openai/gpt-4o".to_string(), + "openai/gpt-5".to_string(), "openrouter/openai/gpt-5".to_string(), - "openrouter/openrouter/free".to_string(), ]) ); - // Case is ignored beyond ASCII, as minimatch's `nocase` does. assert_eq!( - pi_scope_of(&["CUSTOM/ä*"], PI_SCOPE_ROWS), - Some(vec!["custom/Äther-1".to_string()]) + pi_scope_of(&["OpenRouter/*"], PI_SCOPE_ROWS), + Some(vec!["openrouter/openrouter/free".to_string()]) + ); + // A glob pi matches to nothing must not scope the list pi leaves + // whole: `*` crossing `/` would have kept only OpenRouter's GPT-5. + assert_eq!( + pi_scope_of( + &["*gpt-5*"], + &[ + ("openrouter/openai/gpt-5", "openrouter/OpenAI: GPT-5"), + ("anthropic/claude-opus", "anthropic/Claude Opus"), + ] + ), + None + ); + } + + #[test] + fn pi_enabled_models_keep_the_whole_list_for_globs_past_ascii() { + // minimatch folds case the JavaScript way (σ ~ ς) and counts `?` in + // UTF-16 units (an emoji is two): neither is mirrored, so both keep + // the whole list instead of hiding the second row. + let rows = |second| [("openai/gpt-5", "openai/GPT-5"), second]; + assert_eq!( + pi_scope_of( + &["openai/gpt-5", "custom/σ*"], + &rows(("custom/ς-model", "custom/ς model")) + ), + None + ); + assert_eq!( + pi_scope_of( + &["openai/gpt-5", "custom/??"], + &rows(("custom/😀", "custom/smile")) + ), + None + ); + // minimatch lets a path's final `/` go: pi keeps `custom/trail/` here. + assert_eq!( + pi_scope_of( + &["openai/gpt-5", "custom/*l"], + &rows(("custom/trail/", "custom/Trail")) + ), + None + ); + // No glob matches a `.` segment, and Windows minimatch takes a leading + // drive letter literally: pi leaves both lists unscoped. + assert_eq!( + pi_scope_of(&[".*"], &rows(("custom/.", "custom/Dot"))), + None + ); + assert_eq!( + pi_scope_of(&["c:*"], &rows(("custom/c:x", "custom/C"))), + None + ); + // A row past ASCII no glob could reach leaves the rest to scope. + assert_eq!( + pi_scope_of(&["openai/*"], &rows(("custom/Äther-1", "custom/Äther 1"))), + Some(vec!["openai/gpt-5".to_string()]) ); } From 88d214cb20b76451f6c1669fe9196a24035408a0 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Tue, 20 Oct 2026 01:54:11 +0800 Subject: [PATCH 4/6] docs(pi): spell out what the enabledModels mirror leaves to pi --- src-tauri/src/acp/connection.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index 88aa551d4b..2f967051f7 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -3915,8 +3915,8 @@ fn pi_minimatch_may_match(path: &str, glob: &str) -> bool { /// One advertised model row, split the way pi names the model behind it: /// pi-acp publishes each of pi's models as value `provider/id` and label -/// `provider/name`. Lowercased once, because every comparison pi makes here is -/// case-insensitive. +/// `provider/name`. Provider, id and name are lowercased once, because every +/// comparison pi makes on them is case-insensitive. struct PiModelRow { value: String, provider: String, @@ -3971,7 +3971,8 @@ enum PiScopeEntry { /// No row pi could pick: pi skips such an entry too (a retired id, a typo). Dead, /// pi decides this entry by a rule codeg does not mirror — a fuzzy id/name - /// match, or glob syntax beyond `*` and `?` — so its rows are unknown here. + /// match, glob syntax beyond ASCII `*` and `?`, or a row minimatch reads + /// its own way — so its rows are unknown here. Unknown, } @@ -4039,9 +4040,10 @@ fn pi_model_glob_is_supported(glob: &str) -> bool { /// Classify one `enabledModels` entry the way pi 1.x resolves a scope pattern /// (`resolveModelScopeFromModels` in pi's `core/model-resolver.js`, checked /// against 1.3.0): a pattern with `*`, `?` or `[` is a glob after an optional -/// thinking suffix, anything else a model reference. The one older rule left -/// out: pi 0.81 settled a bare id that several providers list with a single -/// fuzzy pick, where 1.x keeps all of them, as this does. +/// thinking suffix, anything else a model reference. Two corners of older pi +/// are left out: 0.81 settled a bare id that several providers list with one +/// fuzzy pick (1.x keeps them all, as this does), and read a glob that spells +/// a model id (`model[1m]`) only as a glob. fn pi_scope_entry(pattern: &str, rows: &[PiModelRow]) -> PiScopeEntry { if pattern.contains(['*', '?', '[']) { let glob = pi_model_pattern_without_thinking_level(pattern); From 4baf39d59fce46d2f359567dd6e15ff0986de2ed Mon Sep 17 00:00:00 2001 From: xintaofei Date: Tue, 20 Oct 2026 02:09:59 +0800 Subject: [PATCH 5/6] fix(pi): read pi's files past a UTF-8 byte-order mark pi strips a byte-order mark before parsing settings.json, auth.json and trust.json, but codeg's reader took such a file for corrupt and returned an empty map. Saving Pi settings then rewrote settings.json with only codeg's three keys, dropping enabledModels and everything else; the launch gate read a trust grant behind a mark as no decision and let pi load the repository's extensions unconfirmed; and the trust writer refused to update the file. All three now read past the mark. --- src-tauri/src/commands/acp.rs | 70 ++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/commands/acp.rs b/src-tauri/src/commands/acp.rs index 19c7a2779e..dd7c69413f 100644 --- a/src-tauri/src/commands/acp.rs +++ b/src-tauri/src/commands/acp.rs @@ -6034,9 +6034,7 @@ pub(crate) fn pi_enabled_model_patterns_for_env( workspace: &Path, ) -> Option> { if let Ok(raw) = fs::read_to_string(workspace.join(".pi").join("settings.json")) { - // pi reads past a byte-order mark; serde_json does not. - let raw = raw.strip_prefix('\u{feff}').unwrap_or(&raw); - match serde_json::from_str::(raw) { + match serde_json::from_str::(without_bom(&raw)) { Ok(serde_json::Value::Object(project)) if project.contains_key("enabledModels") => { return None } @@ -6741,7 +6739,7 @@ fn pi_write_trust_decision_at( // and doesn't parse as a JSON object must NOT be rewritten from scratch. // Surface that rather than silently doing nothing, so the user can go fix it. let mut obj = match fs::read_to_string(path) { - Ok(text) => match serde_json::from_str::(&text) { + Ok(text) => match serde_json::from_str::(without_bom(&text)) { Ok(serde_json::Value::Object(map)) => map, _ => { return Err(AcpError::protocol(format!( @@ -6849,14 +6847,21 @@ pub struct PiModelReasoningSpec { /// no picker will ever offer. const PI_THINKING_LEVELS: [&str; 7] = ["off", "minimal", "low", "medium", "high", "xhigh", "max"]; +/// `raw` past a leading UTF-8 byte-order mark, which pi strips from its files +/// before parsing them and serde_json rejects. +fn without_bom(raw: &str) -> &str { + raw.strip_prefix('\u{feff}').unwrap_or(raw) +} + /// Read a JSON file into an owned object map, returning an empty map when the /// file is absent, unreadable, or does not parse to a JSON object. Pi's native /// files are small and codeg-owned; corruption shouldn't abort a save (we -/// re-author the managed keys and preserve whatever else parses). +/// re-author the managed keys and preserve whatever else parses). A leading +/// byte-order mark is not corruption: pi reads past it, and so does this. fn read_json_object_or_empty(path: &Path) -> serde_json::Map { fs::read_to_string(path) .ok() - .and_then(|raw| serde_json::from_str::(&raw).ok()) + .and_then(|raw| serde_json::from_str::(without_bom(&raw)).ok()) .and_then(|value| match value { serde_json::Value::Object(map) => Some(map), _ => None, @@ -16478,6 +16483,34 @@ base_url = \"https://example.test/v1\" /// `trust.json` holds decisions the user made inside pi. If codeg can't parse /// it, it must refuse loudly rather than rewrite the file from scratch. + /// pi reads `trust.json` past a byte-order mark, so a grant behind one is in + /// force for pi and must stop the launch here too. + #[test] + fn pi_launch_is_blocked_by_a_grant_behind_a_byte_order_mark() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (ws, agent_dir, home) = gated_workspace(tmp.path(), Some(true)); + let trust = agent_dir.join("trust.json"); + let text = fs::read_to_string(&trust).unwrap(); + fs::write(&trust, format!("\u{feff}{text}")).unwrap(); + + assert!(launch_block_for(&agent_dir, &home, &ws).is_some()); + } + + #[test] + fn pi_set_project_trust_reads_past_a_byte_order_mark() { + let tmp = tempfile::tempdir().expect("tempdir"); + let ws = tmp.path().join("ws"); + fs::create_dir_all(&ws).unwrap(); + let trust = tmp.path().join("trust.json"); + fs::write(&trust, "\u{feff}{\"/elsewhere\":false}").unwrap(); + + pi_write_trust_decision_at(&trust, &ws, Some(true)).unwrap(); + + let map = read_json_object_or_empty(&trust); + assert_eq!(map.get("/elsewhere"), Some(&serde_json::json!(false))); + assert_eq!(map.get(&canonical_key(&ws)), Some(&serde_json::json!(true))); + } + #[test] fn pi_set_project_trust_never_clobbers_an_unparseable_file() { let tmp = tempfile::tempdir().expect("tempdir"); @@ -17272,6 +17305,31 @@ echo {"id":"codeg-models","type":"response","command":"get_available_models","su ); } + #[test] + fn pi_files_are_read_past_a_byte_order_mark() { + // pi strips a UTF-8 byte-order mark before parsing its files. Read as + // corrupt instead, the settings merge-write would replace every key + // the user had with codeg's three. + let tmp = tempfile::tempdir().expect("tempdir"); + let settings = tmp.path().join("settings.json"); + fs::write( + &settings, + "\u{feff}{\"enabledModels\":[\"openai/*\"],\"sessionDir\":\"/s\"}", + ) + .unwrap(); + + let map = read_json_object_or_empty(&settings); + assert_eq!(map.get("sessionDir"), Some(&serde_json::json!("/s"))); + let env = BTreeMap::from([( + String::from("PI_CODING_AGENT_DIR"), + tmp.path().to_string_lossy().into_owned(), + )]); + assert_eq!( + pi_enabled_model_patterns_for_env(&env, tmp.path()), + Some(vec!["openai/*".to_string()]) + ); + } + #[test] fn pi_enabled_model_patterns_yield_to_a_project_that_sets_its_own() { let agent_dir = tempfile::tempdir().expect("agent dir"); From b837329a1429153607e5cd812cc5521e8223e62c Mon Sep 17 00:00:00 2001 From: xintaofei Date: Tue, 20 Oct 2026 02:15:07 +0800 Subject: [PATCH 6/6] fix(pi): gate a project's .pi/mcp.json behind project trust pi 1.x reads a project's .pi/mcp.json once the project is trusted and starts its stdio servers, which are commands. The trust mirror predated pi's native MCP, so a repository shipping only that file looked like it needed no approval: the launch gate let an unconfirmed inherited grant through and the banner listed nothing. List it, as code-executing, and correct the mirror's note that drift here was harmless. --- src-tauri/src/commands/acp.rs | 46 ++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/src-tauri/src/commands/acp.rs b/src-tauri/src/commands/acp.rs index dd7c69413f..9c449ca6a1 100644 --- a/src-tauri/src/commands/acp.rs +++ b/src-tauri/src/commands/acp.rs @@ -6107,15 +6107,17 @@ fn pi_agent_dir_in_workspace(runtime_env: &BTreeMap, workspace: /// The `/.pi/` entries whose mere presence makes a project "trust-requiring" /// for pi. Mirrors pi's own `TRUST_REQUIRING_PROJECT_CONFIG_RESOURCES` -/// (`core/trust-manager.js`, verified against pi 0.80.2). +/// (`core/trust-manager.js`, verified against pi 1.3.0, whose native MCP added +/// `mcp.json`). /// /// This list is a mirror of another project's constant, so it can drift if pi -/// adds a resource kind. It fails safe in both directions: under-detecting only -/// means codeg stays quiet about resources pi would ignore anyway (pi's own -/// non-interactive default is "don't load"), and over-detecting only costs one -/// extra prompt. Neither direction can load a resource without the user's word. -const PI_TRUST_REQUIRING_CONFIG_RESOURCES: [&str; 7] = [ +/// adds a resource kind, and the drift is not harmless. Over-detecting only +/// costs one extra prompt, but a kind missing here lets a repository that ships +/// only that kind past the launch gate under a grant nobody confirmed — and pi +/// loads it, since the grant is in force. +const PI_TRUST_REQUIRING_CONFIG_RESOURCES: [&str; 8] = [ "settings.json", + "mcp.json", "extensions", "skills", "prompts", @@ -6134,9 +6136,10 @@ pub struct PiProjectResource { pub kind: String, /// Whether loading this resource means executing repo-controlled code at pi /// startup, as opposed to feeding it repo-controlled text. `.pi/extensions` - /// are JS/TS modules whose top level runs immediately, and `.pi/settings.json` - /// can make pi install project packages — the approval UI must say so plainly - /// rather than describing all of these as "config". + /// are JS/TS modules whose top level runs immediately, `.pi/settings.json` + /// can make pi install project packages, and `.pi/mcp.json` names commands + /// pi starts as MCP servers — the approval UI must say so plainly rather + /// than describing all of these as "config". pub executes_code: bool, } @@ -6178,8 +6181,9 @@ fn pi_project_trust_resources_with_home(cwd: &Path, home: &Path) -> Vec