diff --git a/AppImage/scripts/ai_providers/openai_provider.py b/AppImage/scripts/ai_providers/openai_provider.py index 66f0304f..9e012dd7 100644 --- a/AppImage/scripts/ai_providers/openai_provider.py +++ b/AppImage/scripts/ai_providers/openai_provider.py @@ -183,6 +183,12 @@ class OpenAIProvider(AIProvider): ], } + # Custom OpenAI-compatible endpoints often expose opaque aliases whose + # upstream capabilities are known only to the proxy. Do not infer + # sampling or reasoning parameters from those aliases; let the proxy + # apply model-specific defaults. + if self.base_url: + payload['max_tokens'] = max_tokens # Reasoning models (o1/o3/o4/gpt-5*, excluding *-chat-latest) use a # different parameter contract: max_completion_tokens instead of # max_tokens, and no temperature field. Sending the classic chat @@ -196,7 +202,7 @@ class OpenAIProvider(AIProvider): # exactly what this pipeline wants. OpenAI documents 'minimal', # 'low', 'medium', 'high' — 'minimal' is the right setting for a # straightforward translate+explain task. - if self._is_reasoning_model(self.model): + elif self._is_reasoning_model(self.model): payload['max_completion_tokens'] = max_tokens payload['reasoning_effort'] = 'minimal' else: diff --git a/AppImage/scripts/notification_manager.py b/AppImage/scripts/notification_manager.py index c5ce1b49..23b2500e 100644 --- a/AppImage/scripts/notification_manager.py +++ b/AppImage/scripts/notification_manager.py @@ -895,6 +895,40 @@ class NotificationManager: self._config[key] = value except Exception as e: print(f"[NotificationManager] Failed to save setting {key}: {e}") + + def _active_ai_model(self, provider_name: str) -> str: + """Return the model selected for the active provider. + + `ai_model` is the legacy global key. Newer settings persist + provider-specific models as `ai_model_`, and those must win + whenever present so custom endpoints keep their opaque aliases. + """ + return ( + self._config.get(f'ai_model_{provider_name}', '') + or self._config.get('ai_model', '') + ) + + def _build_ai_config(self) -> Dict[str, Any]: + """Build the shared AI config passed to notification rewriters.""" + ai_provider = self._config.get('ai_provider', 'groq') + ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '') + return { + 'ai_enabled': self._config.get('ai_enabled', 'false'), + 'ai_provider': ai_provider, + 'ai_api_key': ai_api_key, + 'ai_model': self._active_ai_model(ai_provider), + 'ai_language': self._config.get('ai_language', 'en'), + 'ai_ollama_url': self._config.get('ai_ollama_url', ''), + # `ai_openai_base_url` was previously dropped from this dict and + # the downstream `notification_templates.AIRewriter` read it from + # the dict — meaning a user who configured LiteLLM / Azure as a + # custom base_url passed the "Test AI" check (which DOES pass it) + # but every real notification silently went to api.openai.com. + # Privacy + UX deception bug. Audit Tier 3.2 #1. + 'ai_openai_base_url': self._config.get('ai_openai_base_url', ''), + 'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'), + 'ai_custom_prompt': self._config.get('ai_custom_prompt', ''), + } def _rebuild_channels(self): """Rebuild channel instances from current config. @@ -1216,26 +1250,7 @@ class NotificationManager: default_event_enabled = 'true' if template.get('default_enabled', True) else 'false' # Build AI config once (shared across channels, detail_level varies) - # Use per-provider API key - ai_provider = self._config.get('ai_provider', 'groq') - ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '') - ai_config = { - 'ai_enabled': self._config.get('ai_enabled', 'false'), - 'ai_provider': ai_provider, - 'ai_api_key': ai_api_key, - 'ai_model': self._config.get('ai_model', ''), - 'ai_language': self._config.get('ai_language', 'en'), - 'ai_ollama_url': self._config.get('ai_ollama_url', ''), - # `ai_openai_base_url` was previously dropped from this dict and - # the downstream `notification_templates.AIRewriter` read it from - # the dict — meaning a user who configured LiteLLM / Azure as a - # custom base_url passed the "Test AI" check (which DOES pass it) - # but every real notification silently went to api.openai.com. - # Privacy + UX deception bug. Audit Tier 3.2 #1. - 'ai_openai_base_url': self._config.get('ai_openai_base_url', ''), - 'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'), - 'ai_custom_prompt': self._config.get('ai_custom_prompt', ''), - } + ai_config = self._build_ai_config() # Get journal context if available (will be enriched per-channel based on detail_level) raw_journal_context = data.get('_journal_context', '') @@ -2163,26 +2178,8 @@ class NotificationManager: message = rendered['body'] severity = severity or rendered['severity'] - # AI config for enhancement - use per-provider API key - ai_provider = self._config.get('ai_provider', 'groq') - ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '') - ai_config = { - 'ai_enabled': self._config.get('ai_enabled', 'false'), - 'ai_provider': ai_provider, - 'ai_api_key': ai_api_key, - 'ai_model': self._config.get('ai_model', ''), - 'ai_language': self._config.get('ai_language', 'en'), - 'ai_ollama_url': self._config.get('ai_ollama_url', ''), - # `ai_openai_base_url` was previously dropped from this dict and - # the downstream `notification_templates.AIRewriter` read it from - # the dict — meaning a user who configured LiteLLM / Azure as a - # custom base_url passed the "Test AI" check (which DOES pass it) - # but every real notification silently went to api.openai.com. - # Privacy + UX deception bug. Audit Tier 3.2 #1. - 'ai_openai_base_url': self._config.get('ai_openai_base_url', ''), - 'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'), - 'ai_custom_prompt': self._config.get('ai_custom_prompt', ''), - } + # AI config for enhancement + ai_config = self._build_ai_config() results = {} channels_sent = [] @@ -2268,26 +2265,9 @@ class NotificationManager: else: return {'success': False, 'error': f'Channel {channel_name} not configured'} - # AI config for enhancement - use per-provider API key - ai_provider = self._config.get('ai_provider', 'groq') - ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '') - ai_config = { - 'ai_enabled': self._config.get('ai_enabled', 'false'), - 'ai_provider': ai_provider, - 'ai_api_key': ai_api_key, - 'ai_model': self._config.get('ai_model', ''), - 'ai_language': self._config.get('ai_language', 'en'), - 'ai_ollama_url': self._config.get('ai_ollama_url', ''), - # `ai_openai_base_url` was previously dropped from this dict and - # the downstream `notification_templates.AIRewriter` read it from - # the dict — meaning a user who configured LiteLLM / Azure as a - # custom base_url passed the "Test AI" check (which DOES pass it) - # but every real notification silently went to api.openai.com. - # Privacy + UX deception bug. Audit Tier 3.2 #1. - 'ai_openai_base_url': self._config.get('ai_openai_base_url', ''), - 'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'), - 'ai_custom_prompt': self._config.get('ai_custom_prompt', ''), - } + # AI config for enhancement + ai_config = self._build_ai_config() + ai_provider = ai_config.get('ai_provider', 'groq') ai_enabled = self._config.get('ai_enabled', 'false') if isinstance(ai_enabled, str): @@ -2718,7 +2698,7 @@ class NotificationManager: 'ai_provider': current_provider, 'ai_api_keys': ai_api_keys, 'ai_models': ai_models, - 'ai_model': self._config.get('ai_model', ''), + 'ai_model': self._active_ai_model(current_provider), 'ai_language': self._config.get('ai_language', 'en'), 'ai_ollama_url': self._config.get('ai_ollama_url', 'http://localhost:11434'), 'ai_openai_base_url': self._config.get('ai_openai_base_url', ''), @@ -2893,7 +2873,7 @@ class NotificationManager: return {'checked': False, 'migrated': False, 'message': 'AI not enabled'} provider_name = self._config.get('ai_provider', 'groq') - current_model = self._config.get('ai_model', '') + current_model = self._active_ai_model(provider_name) # Skip Ollama - user manages their own models if provider_name == 'ollama': @@ -2927,7 +2907,13 @@ class NotificationManager: print(f"[NotificationManager] Failed to load verified models: {e}") from ai_providers import get_provider - provider = get_provider(provider_name, api_key=api_key, model=current_model) + provider_kwargs = { + 'api_key': api_key, + 'model': current_model, + } + if provider_name == 'openai': + provider_kwargs['base_url'] = self._config.get('ai_openai_base_url', '') + provider = get_provider(provider_name, **provider_kwargs) if not provider: return {'checked': False, 'migrated': False, 'message': f'Unknown provider: {provider_name}'} @@ -2935,8 +2921,24 @@ class NotificationManager: # Get available models from API api_models = provider.list_models() - # Combine: use verified models that are also in API (or all verified if API fails) - if api_models and verified_models: + # Combine: official providers intersect the API list with the + # verified catalogue. Custom OpenAI-compatible endpoints are + # authoritative for their own opaque aliases, so do not intersect + # them with ProxMenux's bundled official OpenAI IDs. + openai_custom_endpoint = ( + provider_name == 'openai' + and bool(self._config.get('ai_openai_base_url', '').strip()) + ) + if openai_custom_endpoint: + if not api_models: + return { + 'checked': True, + 'migrated': False, + 'new_model': current_model, + 'message': 'Could not retrieve custom endpoint model list' + } + available_models = api_models + elif api_models and verified_models: available_models = [m for m in verified_models if m in api_models] elif verified_models: available_models = verified_models @@ -2970,13 +2972,16 @@ class NotificationManager: try: conn = sqlite3.connect(str(DB_PATH), timeout=10) cursor = conn.cursor() - cursor.execute(''' - INSERT OR REPLACE INTO user_settings (setting_key, setting_value, updated_at) - VALUES (?, ?, ?) - ''', (f'{SETTINGS_PREFIX}ai_model', recommended, datetime.now().isoformat())) + now_iso = datetime.now().isoformat() + for model_key in ('ai_model', f'ai_model_{provider_name}'): + cursor.execute(''' + INSERT OR REPLACE INTO user_settings (setting_key, setting_value, updated_at) + VALUES (?, ?, ?) + ''', (f'{SETTINGS_PREFIX}{model_key}', recommended, now_iso)) conn.commit() conn.close() self._config['ai_model'] = recommended + self._config[f'ai_model_{provider_name}'] = recommended print(f"[NotificationManager] AI model migrated: {old_model} -> {recommended}") diff --git a/AppImage/scripts/tests/test_openai_compatible_ai_models.py b/AppImage/scripts/tests/test_openai_compatible_ai_models.py new file mode 100644 index 00000000..0b7febaf --- /dev/null +++ b/AppImage/scripts/tests/test_openai_compatible_ai_models.py @@ -0,0 +1,185 @@ +import sqlite3 +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + + +SCRIPTS_DIR = Path(__file__).resolve().parents[1] +if str(SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(SCRIPTS_DIR)) + +import ai_providers +import notification_manager +from ai_providers.openai_provider import OpenAIProvider + + +class CapturingOpenAIProvider(OpenAIProvider): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.captured_payload = None + + def _make_request(self, url, payload, headers): + self.captured_payload = payload + return {"choices": [{"message": {"content": "ok"}}]} + + +class FakeProvider: + last_kwargs = None + models = [] + + def __init__(self, **kwargs): + FakeProvider.last_kwargs = kwargs + + def list_models(self): + return list(FakeProvider.models) + + +class OpenAICompatibleModelTests(unittest.TestCase): + def setUp(self): + FakeProvider.last_kwargs = None + FakeProvider.models = [] + self.provider_patch = mock.patch.dict( + ai_providers.PROVIDERS, + {"openai": FakeProvider}, + ) + self.provider_patch.start() + self.addCleanup(self.provider_patch.stop) + + def _manager(self, config): + manager = notification_manager.NotificationManager() + manager._config = dict(config) + manager._enabled = manager._config.get("enabled", "false") == "true" + return manager + + def _temp_db_patch(self): + temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(temp_dir.cleanup) + db_path = Path(temp_dir.name) / "health_monitor.db" + conn = sqlite3.connect(str(db_path)) + conn.execute( + "CREATE TABLE user_settings (setting_key TEXT PRIMARY KEY, " + "setting_value TEXT, updated_at TEXT)" + ) + conn.commit() + conn.close() + patcher = mock.patch.object(notification_manager, "DB_PATH", db_path) + patcher.start() + self.addCleanup(patcher.stop) + return db_path + + def test_custom_openai_endpoint_omits_temperature_for_opaque_alias(self): + provider = CapturingOpenAIProvider( + api_key="token", + model="opaque-gpt5-alias", + base_url="https://litellm.example", + ) + + self.assertEqual(provider.generate("system", "user", max_tokens=123), "ok") + + self.assertEqual(provider.captured_payload["model"], "opaque-gpt5-alias") + self.assertEqual(provider.captured_payload["max_tokens"], 123) + self.assertNotIn("temperature", provider.captured_payload) + self.assertNotIn("reasoning_effort", provider.captured_payload) + self.assertNotIn("max_completion_tokens", provider.captured_payload) + + def test_official_openai_reasoning_model_still_uses_reasoning_contract(self): + provider = CapturingOpenAIProvider( + api_key="token", + model="gpt-5-mini", + ) + + self.assertEqual(provider.generate("system", "user", max_tokens=123), "ok") + + self.assertEqual(provider.captured_payload["model"], "gpt-5-mini") + self.assertEqual(provider.captured_payload["max_completion_tokens"], 123) + self.assertEqual(provider.captured_payload["reasoning_effort"], "minimal") + self.assertNotIn("temperature", provider.captured_payload) + self.assertNotIn("max_tokens", provider.captured_payload) + + def test_runtime_ai_config_prefers_provider_specific_model(self): + manager = self._manager({ + "ai_enabled": "true", + "ai_provider": "openai", + "ai_api_key_openai": "token", + "ai_model": "gpt-4.1-nano", + "ai_model_openai": "proxy-alias", + "ai_openai_base_url": "https://litellm.example", + }) + + ai_config = manager._build_ai_config() + + self.assertEqual(ai_config["ai_model"], "proxy-alias") + self.assertEqual(ai_config["ai_openai_base_url"], "https://litellm.example") + + def test_model_verifier_uses_custom_endpoint_alias_without_migration(self): + manager = self._manager({ + "ai_enabled": "true", + "ai_provider": "openai", + "ai_api_key_openai": "token", + "ai_model": "gpt-4.1-nano", + "ai_model_openai": "proxy-alias", + "ai_openai_base_url": "https://litellm.example", + }) + FakeProvider.models = ["proxy-alias"] + + result = manager.verify_and_update_ai_model() + + self.assertTrue(result["checked"]) + self.assertFalse(result["migrated"]) + self.assertEqual(result["new_model"], "proxy-alias") + self.assertEqual(FakeProvider.last_kwargs["model"], "proxy-alias") + self.assertEqual(FakeProvider.last_kwargs["base_url"], "https://litellm.example") + + def test_custom_endpoint_does_not_fallback_to_official_model_catalogue(self): + manager = self._manager({ + "ai_enabled": "true", + "ai_provider": "openai", + "ai_api_key_openai": "token", + "ai_model": "gpt-4.1-nano", + "ai_model_openai": "proxy-alias", + "ai_openai_base_url": "https://litellm.example", + }) + FakeProvider.models = [] + + result = manager.verify_and_update_ai_model() + + self.assertTrue(result["checked"]) + self.assertFalse(result["migrated"]) + self.assertEqual(result["new_model"], "proxy-alias") + self.assertEqual(result["message"], "Could not retrieve custom endpoint model list") + + def test_model_migration_updates_legacy_and_provider_specific_keys(self): + db_path = self._temp_db_patch() + manager = self._manager({ + "ai_enabled": "true", + "ai_provider": "openai", + "ai_api_key_openai": "token", + "ai_model": "old-generic", + "ai_model_openai": "old-alias", + "ai_openai_base_url": "https://litellm.example", + }) + FakeProvider.models = ["new-alias"] + + result = manager.verify_and_update_ai_model() + + self.assertTrue(result["checked"]) + self.assertTrue(result["migrated"]) + self.assertEqual(result["old_model"], "old-alias") + self.assertEqual(result["new_model"], "new-alias") + self.assertEqual(manager._config["ai_model"], "new-alias") + self.assertEqual(manager._config["ai_model_openai"], "new-alias") + + conn = sqlite3.connect(str(db_path)) + rows = dict(conn.execute( + "SELECT setting_key, setting_value FROM user_settings" + ).fetchall()) + conn.close() + + self.assertEqual(rows["notification.ai_model"], "new-alias") + self.assertEqual(rows["notification.ai_model_openai"], "new-alias") + + +if __name__ == "__main__": + unittest.main()