From b3127a7d914c228b65e0c81d0a77a96107fca625 Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Wed, 16 Sep 2026 00:33:16 -0700 Subject: [PATCH] fix(tui): gate recommended_plugins to once per engine, suppressed by loaded skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-turn fragment fired on every turn for any conversation that merely mentioned a plugin's domain keywords, and it could not tell that a loaded local skill already covered the domain — a takeover session talking about handoffs was nudged to install the handoff plugin four times in one evening. A plugin id is now suggested at most once per engine lifetime, and a plugin whose name (or alias) matches a skill in the session's catalogue is never suggested. Dismissals still apply. The suppression snapshot is taken at engine construction from the same catalogue the system prompt indexes (workspace roots + configured skills dir + plugin-sourced skills); a skill installed mid-session is not suppressed until the next engine starts (known limitation, documented on the gate). Closes #6274 Signed-off-by: CodeWhale Bot --- CHANGELOG.md | 5 + crates/tui/CHANGELOG.md | 5 + crates/tui/src/core/engine.rs | 54 ++++++++-- crates/tui/src/plugins/recommend.rs | 122 ++++++++++++++++++++++- crates/tui/src/tui/plugin_suggestions.rs | 1 + web/lib/changelog.generated.ts | 3 +- 6 files changed, 178 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b27bace9a..e1c33dc02e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `` suggestions stop nagging: a plugin id is now + injected at most once per engine lifetime, and a plugin whose name a + loaded skill already covers is never suggested — the local skill owns + the domain, so the nudge was noise. Dismissals still apply, and the + fragment stays append-only on the user turn (#6274). - A canceled automation run now settles with a transcript receipt that names the cancellation (by request, cancel timeout, or shutdown) instead of vanishing from the live band silently. The receipt wears attention ink and diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index abf942a085..a4faa75de8 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -114,6 +114,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `` suggestions stop nagging: a plugin id is now + injected at most once per engine lifetime, and a plugin whose name a + loaded skill already covers is never suggested — the local skill owns + the domain, so the nudge was noise. Dismissals still apply, and the + fragment stays append-only on the user turn (#6274). - A canceled automation run now settles with a transcript receipt that names the cancellation (by request, cancel timeout, or shutdown) instead of vanishing from the live band silently. The receipt wears attention ink and diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 99895ae3b5..51ac1137d7 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -873,6 +873,12 @@ pub struct Engine { mcp_event_generation: u64, /// Workspace-scoped immutable plugin catalogue and authority receipts. plugin_registry: Arc, + /// Keeps the append-only `` fragment once-per- + /// Engine-lifetime per plugin id, and suppresses plugins whose name a + /// catalogue skill already covers (#6274). The skill-name snapshot is + /// taken at construction from the same catalogue the system prompt + /// indexes (see the gate's known-limitations note). + recommended_plugin_gate: StdMutex, api_provider: ApiProvider, /// Exact configured route key. Named custom providers share the `Custom` /// enum, so the enum alone cannot prove that the active client is current. @@ -1674,6 +1680,28 @@ impl Engine { // `run_turn` restarts it per turn; this initial value only matters // for hosts that inspect the engine before the first turn. let turn_wall_clock_budget = config.turn_wall_clock; + // Skill-name snapshot for the plugin-suggestion gate (#6274): the + // SAME catalogue the system prompt indexes (prompts.rs skills block — + // workspace roots + configured skills_dir + plugin-sourced skills), + // so suppression sees everything the session actually has. + let gate_skill_names: std::collections::BTreeSet = + crate::skills::discover_for_workspace_and_dir_with_mode_and_plugins( + &config.workspace, + &config.skills_dir, + crate::skills::SkillDiscoveryMode::from_codewhale_only( + config.skills_scan_codewhale_only, + ), + Some(plugin_registry.as_ref()), + ) + .list() + .iter() + .flat_map(|skill| { + std::iter::once(skill.name.clone()).chain(skill.aliases.iter().cloned()) + }) + .map(|name| name.trim().to_ascii_lowercase()) + .filter(|name| !name.is_empty()) + .collect(); + let engine = Engine { config, api_config: api_config.clone(), @@ -1698,6 +1726,11 @@ impl Engine { mcp_boot_generation: None, mcp_event_generation: 0, plugin_registry, + recommended_plugin_gate: StdMutex::new( + crate::plugins::recommend::RecommendedPluginGate::with_skill_names( + gate_skill_names, + ), + ), api_provider, api_provider_identity, api_provider_id, @@ -3708,13 +3741,20 @@ impl Engine { cache_control: None, }]; } - let recommended_plugins = crate::plugins::recommend::recommended_plugins_user_fragment( - &text, - self.plugin_registry.as_ref(), - &crate::plugins::recommend::load_marketplace_candidates( - self.plugin_registry.state_path(), - ), - ); + let recommended_plugins = { + let mut recommended_plugin_gate = self + .recommended_plugin_gate + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + crate::plugins::recommend::recommended_plugins_user_fragment( + &text, + self.plugin_registry.as_ref(), + &crate::plugins::recommend::load_marketplace_candidates( + self.plugin_registry.state_path(), + ), + &mut recommended_plugin_gate, + ) + }; let expanded = crate::image_attach::expand_attachment_blocks(&text); let mut content = Vec::with_capacity(3 + expanded.blocks.len()); content.push(ContentBlock::Text { diff --git a/crates/tui/src/plugins/recommend.rs b/crates/tui/src/plugins/recommend.rs index e55058cc99..ffe3aa6ee4 100644 --- a/crates/tui/src/plugins/recommend.rs +++ b/crates/tui/src/plugins/recommend.rs @@ -259,6 +259,51 @@ pub fn match_plugin_for_draft_among( Some(matched) } +/// Per-Engine gate for the append-only `` fragment. +/// +/// A plugin id is suggested at most once per Engine lifetime, and a plugin +/// whose name (or alias) matches a skill in the session's catalogue is never +/// suggested — the skill already covers the domain, so the nudge is noise +/// (#6274). Dismissals continue to be honored through `Settings`. +/// +/// Known limitation: the skill-name set is snapshotted once at Engine +/// construction (from the same catalogue the system prompt indexes), so a +/// skill installed mid-session does not suppress its plugin twin until the +/// next Engine starts. +#[derive(Debug, Default)] +pub struct RecommendedPluginGate { + shown: BTreeSet, + skill_names: BTreeSet, +} + +impl RecommendedPluginGate { + /// Engine constructor input and test seam: suppress exactly these + /// skill names and aliases (case is normalized here, so callers may + /// pass them in any form). + #[must_use] + pub fn with_skill_names(skill_names: BTreeSet) -> Self { + Self { + shown: BTreeSet::new(), + skill_names: skill_names + .into_iter() + .map(|name| name.trim().to_ascii_lowercase()) + .filter(|name| !name.is_empty()) + .collect(), + } + } + + /// True when this plugin may be suggested now: not covered by a + /// catalogue skill and not already suggested in this Engine's lifetime. + /// First admission records the plugin id. + fn admits(&mut self, id: &str, name: &str) -> bool { + let name_key = name.trim().to_ascii_lowercase(); + if !name_key.is_empty() && self.skill_names.contains(&name_key) { + return false; + } + self.shown.insert(id.to_string()) + } +} + /// Append-only user-turn fragment. Never part of the pinned system prefix. /// Bounded, omitted when nothing matches. #[must_use] @@ -266,6 +311,7 @@ pub fn recommended_plugins_user_fragment( draft: &str, registry: &PluginRegistry, marketplace: &[MarketplaceCandidate], + gate: &mut RecommendedPluginGate, ) -> Option { // Called once when composing a user turn, never from the render loop. // Read the shared preference so headless and long-lived Engines also @@ -277,6 +323,11 @@ pub fn recommended_plugins_user_fragment( marketplace, &settings.dismissed_plugin_suggestions, )?; + // Once per Engine lifetime per plugin id, and never when a local skill + // already covers the plugin's domain (#6274). + if !gate.admits(&matched.id, &matched.name) { + return None; + } let mut listed = vec![matched]; listed.truncate(MAX_RECOMMENDED_PLUGINS); let body = listed @@ -650,14 +701,77 @@ mod tests { let registry = crate::plugins::PluginDiscoveryContext::capture_pre_dotenv() .registry_for_workspace(root.path()); - let fragment = - recommended_plugins_user_fragment("add supabase auth to login", ®istry, &[]) - .expect("idle plugin should produce a fragment"); + let fragment = recommended_plugins_user_fragment( + "add supabase auth to login", + ®istry, + &[], + &mut RecommendedPluginGate::default(), + ) + .expect("idle plugin should produce a fragment"); assert!(fragment.starts_with("")); assert!(fragment.contains("- supabase (")); assert!(fragment.contains("")); assert!( - recommended_plugins_user_fragment("fix the failing test", ®istry, &[]).is_none() + recommended_plugins_user_fragment( + "fix the failing test", + ®istry, + &[], + &mut RecommendedPluginGate::default(), + ) + .is_none() + ); + } + + #[test] + fn recommended_plugins_fragment_suggests_a_plugin_once_per_gate() { + let _lock = lock_test_env(); + let root = TempDir::new().unwrap(); + let _home = EnvVarGuard::set("CODEWHALE_HOME", root.path().join("home")); + write_keyword_bundle(root.path(), "supabase", "Hosted Postgres", &["supabase"]); + let registry = crate::plugins::PluginDiscoveryContext::capture_pre_dotenv() + .registry_for_workspace(root.path()); + + let mut gate = RecommendedPluginGate::default(); + let first = recommended_plugins_user_fragment( + "add supabase auth to login", + ®istry, + &[], + &mut gate, + ) + .expect("first matching turn suggests the plugin"); + assert!(first.contains("- supabase (")); + assert!( + recommended_plugins_user_fragment( + "add supabase auth to the signup flow", + ®istry, + &[], + &mut gate, + ) + .is_none(), + "a plugin id is suggested at most once per Engine lifetime (#6274)" + ); + } + + #[test] + fn recommended_plugins_fragment_suppressed_by_local_skill_name() { + let _lock = lock_test_env(); + let root = TempDir::new().unwrap(); + let _home = EnvVarGuard::set("CODEWHALE_HOME", root.path().join("home")); + write_keyword_bundle(root.path(), "supabase", "Hosted Postgres", &["supabase"]); + let registry = crate::plugins::PluginDiscoveryContext::capture_pre_dotenv() + .registry_for_workspace(root.path()); + + let skills: BTreeSet = ["Supabase".to_string()].into_iter().collect(); + let mut gate = RecommendedPluginGate::with_skill_names(skills); + assert!( + recommended_plugins_user_fragment( + "add supabase auth to login", + ®istry, + &[], + &mut gate, + ) + .is_none(), + "a loaded local skill covering the plugin name must suppress the suggestion (#6274)" ); } diff --git a/crates/tui/src/tui/plugin_suggestions.rs b/crates/tui/src/tui/plugin_suggestions.rs index 7e1e5f59cd..4dc6c5c2a8 100644 --- a/crates/tui/src/tui/plugin_suggestions.rs +++ b/crates/tui/src/tui/plugin_suggestions.rs @@ -510,6 +510,7 @@ mod tests { &app.input, restarted.plugin_registry.as_ref(), &[], + &mut crate::plugins::recommend::RecommendedPluginGate::default(), ) .is_none() ); diff --git a/web/lib/changelog.generated.ts b/web/lib/changelog.generated.ts index 01d6cc98b2..cd62937497 100644 --- a/web/lib/changelog.generated.ts +++ b/web/lib/changelog.generated.ts @@ -62,6 +62,7 @@ export const CHANGELOG: ChangelogRelease[] = [ { "heading": "Fixed", "items": [ + " suggestions stop nagging: a plugin id is now injected at most once per engine lifetime, and a plugin whose name a loaded skill already covers is never suggested — the local skill owns the domain, so the nudge was noise. Dismissals still apply, and the fragment stays append-only on the user turn (#6274).", "A canceled automation run now settles with a transcript receipt that names the cancellation (by request, cancel timeout, or shutdown) instead of vanishing from the live band silently. The receipt wears attention ink and never lights the failure demand; the run record keeps the cancellation reason as its error detail. (#6162)", "A failed workflow run no longer settles silently: its terminal failure raises a sticky error toast naming the cause (dispatch, schema, or script errors), alongside the existing panel state (#5528).", "MCP OAuth re-login now forces the provider's consent screen: logout only clears the local token, so without a prompt the provider silently re-granted the same account/workspace and a re-login could never change it. /mcp logout and codewhale mcp logout also say plainly that they clear local credentials only (#6040).", @@ -73,7 +74,7 @@ export const CHANGELOG: ChangelogRelease[] = [ "/mcp no longer freezes the console while a turn is running: the panel opens immediately from the last known MCP snapshot with a receipt naming the wait, and live-pool mutations say their refresh is deferred instead of parking the UI event loop behind the running turn (#6159).", "MCP OAuth login no longer fails with \"Authorization server response missing required issuer\" against servers that implement RFC 9207, such as Cloudflare's mcp.cloudflare.com. The local callback listener now keeps the iss parameter from the redirect and hands it to the token exchange so the callback binds to the discovered issuer; servers that do not send iss keep working unchanged. (#6157)" ], - "itemCount": 10 + "itemCount": 11 } ] },