refactor(composio): extract derive_toolkit_slug helper (#3035)

app/src/components/settings/panels/ToolsPanel.test.tsx

@@ -0,0 +1,74 @@
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import ToolsPanel from './ToolsPanel';
+
+const mocks = vi.hoisted(() => ({ setOnboardingTasks: vi.fn(), useCoreStateMock: vi.fn() }));
+
+vi.mock('../../../providers/CoreStateProvider', () => ({
+  useCoreState: () => mocks.useCoreStateMock(),
+}));
+
+vi.mock('../../../lib/i18n/I18nContext', () => ({
+  useT: () => ({
+    t: (key: string) =>
+      ({
+        'settings.features.tools': 'Tools',
+        'settings.tools.chooseCapabilities': 'Choose capabilities',
+        'settings.tools.saveChanges': 'Save Changes',
+        'settings.tools.preferencesSaved': 'Preferences saved',
+        'settings.tools.saveFailed': 'Unable to save preferences',
+      })[key] ?? key,
+  }),
+}));
+
+vi.mock('../hooks/useSettingsNavigation', () => ({
+  useSettingsNavigation: () => ({ breadcrumbs: [], navigateBack: vi.fn() }),
+}));
+
+vi.mock('../components/SettingsHeader', () => ({
+  default: ({ title }: { title: string }) => <h1>{title}</h1>,
+}));
+
+function coreState(enabledTools: string[]) {
+  return {
+    snapshot: {
+      localState: {
+        onboardingTasks: {
+          accessibilityPermissionGranted: false,
+          localModelConsentGiven: false,
+          localModelDownloadStarted: false,
+          enabledTools,
+          connectedSources: [],
+        },
+      },
+    },
+    setOnboardingTasks: mocks.setOnboardingTasks,
+  };
+}
+
+describe('<ToolsPanel />', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.useCoreStateMock.mockReturnValue(coreState(['shell']));
+    mocks.setOnboardingTasks.mockResolvedValue(undefined);
+  });
+
+  it('exposes tool toggle state and saves the updated enabled tools list', async () => {
+    render(<ToolsPanel />);
+
+    const shellToggle = screen.getByRole('switch', { name: /Shell Commands/ });
+    await waitFor(() => expect(shellToggle).toHaveAttribute('aria-checked', 'true'));
+
+    fireEvent.click(shellToggle);
+
+    expect(shellToggle).toHaveAttribute('aria-checked', 'false');
+    fireEvent.click(screen.getByRole('button', { name: 'Save Changes' }));
+
+    await waitFor(() =>
+      expect(mocks.setOnboardingTasks).toHaveBeenCalledWith(
+        expect.objectContaining({ enabledTools: [] })
+      )
+    );
+  });
+});

app/src/components/settings/panels/ToolsPanel.tsx

@@ -129,6 +129,8 @@ const ToolsPanel = ({ embedded = false }: ToolsPanelProps = {}) => {
                     <button
                       key={tool.id}
                       type="button"
+                      role="switch"
+                      aria-checked={Boolean(enabled[tool.id])}
                       onClick={() => toggle(tool.id)}
                       className="w-full flex items-center justify-between p-2.5 rounded-xl border border-stone-200 dark:border-neutral-800 bg-white dark:bg-neutral-900 hover:border-stone-300 dark:border-neutral-700 dark:hover:border-neutral-700 transition-colors text-left">
                       <div className="min-w-0 flex-1">

app/test/playwright/specs/settings-feature-preferences.spec.ts

@@ -87,6 +87,19 @@ test.describe('Settings - Feature Preferences', () => {
   });
 
   test('persists tools preferences to the core app-state snapshot', async ({ page }) => {
+    await openAuthenticatedRoute(page, 'pw-settings-tools', '/settings/tools');
+
+    await callCoreRpc('openhuman.app_state_update_local_state', {
+      onboardingTasks: {
+        accessibilityPermissionGranted: false,
+        localModelConsentGiven: false,
+        localModelDownloadStarted: false,
+        enabledTools: ['shell'],
+        connectedSources: [],
+        updatedAtMs: Date.now(),
+      },
+    });
+
     const before = await callCoreRpc<{
       result?: {
         localState?: { onboardingTasks?: { enabledTools?: string[] | null } | null } | null;
@@ -94,13 +107,16 @@ test.describe('Settings - Feature Preferences', () => {
     }>('openhuman.app_state_snapshot', {});
     const enabledBefore = before.result?.localState?.onboardingTasks?.enabledTools ?? [];
 
-    await openAuthenticatedRoute(page, 'pw-settings-tools', '/settings/tools');
+    await reloadAndWait(page);
 
     await expect(page.getByText('Tools', { exact: true })).toBeVisible();
-    await page
+    const shellToggle = page
       .locator('button')
-      .filter({ has: page.getByText('Shell Commands', { exact: true }) })
-      .click();
+      .filter({ has: page.getByText('Shell Commands', { exact: true }) });
+    await expect(shellToggle).toHaveAttribute('aria-checked', 'true');
+    await shellToggle.click();
+    await expect(shellToggle).toHaveAttribute('aria-checked', 'false');
+
     await page.getByRole('button', { name: 'Save Changes', exact: true }).click();
     await expect(page.getByText('Preferences saved')).toBeVisible();
 
@@ -115,6 +131,13 @@ test.describe('Settings - Feature Preferences', () => {
         return JSON.stringify(enabledAfter) !== JSON.stringify(enabledBefore);
       })
       .toBe(true);
+
+    const after = await callCoreRpc<{
+      result?: {
+        localState?: { onboardingTasks?: { enabledTools?: string[] | null } | null } | null;
+      };
+    }>('openhuman.app_state_snapshot', {});
+    expect(after.result?.localState?.onboardingTasks?.enabledTools ?? []).not.toContain('shell');
   });
 
   test('persists notifications DND and category preferences', async ({ page }) => {

src/openhuman/composio/error_mapping.rs

@@ -121,12 +121,21 @@ fn prefix_class(class: ComposioErrorClass, body: &str) -> String {
     format!("[composio:error:{}] {}", class.as_str(), body)
 }
 
-fn format_insufficient_scope_message(tool: &str, detail: &str) -> String {
-    let toolkit = tool
-        .split('_')
+/// Derive the lowercase toolkit slug from a Composio tool/trigger identifier.
+///
+/// Identifiers are upper-snake-case (e.g. `GMAIL_NEW_GMAIL_MESSAGE`); the leading
+/// segment names the toolkit, so this returns e.g. `gmail`. `str::split('_').next()`
+/// always yields `Some(_)` for `&str` input (empty input yields `Some("")`), so
+/// `unwrap_or_default()` is a safe, equivalent terminator.
+fn derive_toolkit_slug(tool: &str) -> String {
+    tool.split('_')
         .next()
-        .unwrap_or("integration")
-        .to_ascii_lowercase();
+        .unwrap_or_default()
+        .to_ascii_lowercase()
+}
+
+fn format_insufficient_scope_message(tool: &str, detail: &str) -> String {
+    let toolkit = derive_toolkit_slug(tool);
     format!(
         "`{tool}` was rejected because the connected {toolkit} account is missing required \
          permissions ({detail}). Reconnect the integration in Settings → Connections → \
@@ -140,11 +149,7 @@ fn format_insufficient_scope_message(tool: &str, detail: &str) -> String {
 /// [`format_insufficient_scope_message`] does (e.g. `GMAIL_NEW_GMAIL_MESSAGE`
 /// → `gmail`), so the copy is branded and points the user at reconnecting.
 fn format_trigger_permission_message(tool: &str) -> String {
-    let toolkit = tool
-        .split('_')
-        .next()
-        .unwrap_or("integration")
-        .to_ascii_lowercase();
+    let toolkit = derive_toolkit_slug(tool);
     format!(
         "Couldn't enable this trigger: the connected {toolkit} account doesn't have \
          permission to manage triggers. Reconnect {toolkit} in Settings → Connections → \

src/openhuman/composio/error_mapping_tests.rs

@@ -1,7 +1,27 @@
 use super::{
-    classify_composio_error, format_provider_error, remap_transport_error, ComposioErrorClass,
+    classify_composio_error, derive_toolkit_slug, format_provider_error, remap_transport_error,
+    ComposioErrorClass,
 };
 
+// ── derive_toolkit_slug (issue #2913 nitpick — shared slug extraction) ──
+
+#[test]
+fn derive_toolkit_slug_extracts_leading_segment_lowercased() {
+    assert_eq!(derive_toolkit_slug("GMAIL_NEW_GMAIL_MESSAGE"), "gmail");
+}
+
+#[test]
+fn derive_toolkit_slug_single_segment_is_lowercased() {
+    assert_eq!(derive_toolkit_slug("SLACK"), "slack");
+}
+
+#[test]
+fn derive_toolkit_slug_empty_input_returns_empty_not_fallback() {
+    // Behavior-parity guard: `"".split('_').next()` yields `Some("")`, so the
+    // `unwrap_or("integration")` fallback does NOT apply — preserve that exactly.
+    assert_eq!(derive_toolkit_slug(""), "");
+}
+
 #[test]
 fn classifies_gmail_insufficient_scope() {
     let msg = "HTTP 403: Request had insufficient authentication scopes.";

src/openhuman/memory_sync/composio/providers/mod.rs

@@ -155,8 +155,7 @@ pub fn capability_matrix() -> Vec<ComposioCapability> {
 /// Lookup key is the lowercased prefix returned by
 /// [`toolkit_from_slug`] applied to the action slug — e.g.
 /// `GOOGLECALENDAR_CREATE_EVENT` → `"googlecalendar"`. Multi-segment
-/// prefixes like `MICROSOFT_TEAMS_*` are matched via their first
-/// segment with an extra arm.
+/// prefixes like `MICROSOFT_TEAMS_*` return their known toolkit slug.
 /// Synchronous visibility check for a Composio action slug given a
 /// pre-loaded user scope preference.
 ///
@@ -200,7 +199,8 @@ pub fn catalog_for_toolkit(toolkit: &str) -> Option<&'static [CuratedTool]> {
         "googledocs" | "google_docs" => Some(catalogs::GOOGLEDOCS_CURATED),
         "googlesheets" | "google_sheets" => Some(catalogs::GOOGLESHEETS_CURATED),
         "outlook" => Some(catalogs::OUTLOOK_CURATED),
-        // MICROSOFT_TEAMS_* slugs extract to "microsoft" via toolkit_from_slug.
+        // Keep the legacy "microsoft" alias while toolkit_from_slug now
+        // returns the precise "microsoft_teams" slug for Teams actions.
         "microsoft" | "microsoft_teams" => Some(catalogs::MICROSOFT_TEAMS_CURATED),
         "jira" => Some(catalogs::JIRA_CURATED),
         "trello" => Some(catalogs::TRELLO_CURATED),

src/openhuman/memory_tools/types.rs

@@ -125,7 +125,13 @@ impl ToolMemoryRule {
 
     /// Generate a fresh, opaque rule id.
     pub fn generate_id() -> String {
-        uuid::Uuid::new_v4().to_string()
+        let mut id = String::with_capacity(33);
+        id.push('r');
+        for byte in uuid::Uuid::new_v4().as_bytes() {
+            id.push((b'a' + (byte >> 4)) as char);
+            id.push((b'a' + (byte & 0x0f)) as char);
+        }
+        id
     }
 
     /// Storage key used inside the tool namespace.
@@ -207,6 +213,22 @@ mod tests {
         assert_ne!(a, b);
     }
 
+    #[test]
+    fn generated_rule_ids_are_safe_memory_document_keys() {
+        for _ in 0..128 {
+            let id = ToolMemoryRule::generate_id();
+            assert!(
+                id.chars().all(|ch| ch.is_ascii_lowercase()),
+                "generated id should avoid PII-shaped digits and separators: {id}"
+            );
+            let key = ToolMemoryRule::storage_key(&id);
+            assert!(
+                !crate::openhuman::memory_store::safety::pii::has_likely_pii(&key),
+                "generated storage key should not trip PII boundary: {key}"
+            );
+        }
+    }
+
     #[test]
     fn rule_storage_key_uses_rule_prefix() {
         assert_eq!(ToolMemoryRule::storage_key("abc"), "rule/abc");

tests/inference_local_admin_raw_coverage_e2e.rs

@@ -49,14 +49,14 @@ struct EnvVarGuard {
 impl EnvVarGuard {
     fn set(key: &'static str, value: impl AsRef<std::ffi::OsStr>) -> Self {
         let previous = std::env::var_os(key);
-        // SAFETY: validation runs this integration test with --test-threads=1.
+        // SAFETY: tests that mutate environment variables hold env_lock().
         unsafe { std::env::set_var(key, value) };
         Self { key, previous }
     }
 
     fn unset(key: &'static str) -> Self {
         let previous = std::env::var_os(key);
-        // SAFETY: validation runs this integration test with --test-threads=1.
+        // SAFETY: tests that mutate environment variables hold env_lock().
         unsafe { std::env::remove_var(key) };
         Self { key, previous }
     }
@@ -66,11 +66,11 @@ impl Drop for EnvVarGuard {
     fn drop(&mut self) {
         match &self.previous {
             Some(value) => {
-                // SAFETY: validation runs this integration test with --test-threads=1.
+                // SAFETY: tests that mutate environment variables hold env_lock().
                 unsafe { std::env::set_var(self.key, value) }
             }
             None => {
-                // SAFETY: validation runs this integration test with --test-threads=1.
+                // SAFETY: tests that mutate environment variables hold env_lock().
                 unsafe { std::env::remove_var(self.key) }
             }
         }