diff --git a/application/single_app/functions_databricks_operations.py b/application/single_app/functions_databricks_operations.py index 68458e695..c4cab805e 100644 --- a/application/single_app/functions_databricks_operations.py +++ b/application/single_app/functions_databricks_operations.py @@ -6,6 +6,7 @@ DATABRICKS_PLUGIN_TYPE = "databricks" DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE = "databricks_table" +DATABRICKS_DISCOVERY_PLUGIN_TYPES = {DATABRICKS_PLUGIN_TYPE, DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE} DATABRICKS_CLOUD_AZURE_COMMERCIAL = "azure_commercial" DATABRICKS_DEFAULT_CLOUD = DATABRICKS_CLOUD_AZURE_COMMERCIAL DATABRICKS_SQL_STATEMENTS_PATH = "/api/2.0/sql/statements" @@ -26,6 +27,10 @@ } +def is_builtin_databricks_discovery_type(plugin_type: Any) -> bool: + return str(plugin_type or "").strip().lower() in DATABRICKS_DISCOVERY_PLUGIN_TYPES + + def _as_bool(value: Any, default_value: bool = False) -> bool: if isinstance(value, bool): return value diff --git a/application/single_app/route_backend_plugins.py b/application/single_app/route_backend_plugins.py index c715bc3c2..bc5e736c1 100644 --- a/application/single_app/route_backend_plugins.py +++ b/application/single_app/route_backend_plugins.py @@ -68,6 +68,7 @@ from functions_databricks_operations import ( DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE, DATABRICKS_PLUGIN_TYPE, + is_builtin_databricks_discovery_type, normalize_databricks_additional_fields, ) from functions_snowflake_operations import ( @@ -297,6 +298,7 @@ def get_plugin_types(allowed_type_filter=None): for fname in os.listdir(plugintypes_dir): if fname.endswith('_plugin.py') and fname != 'base_plugin.py': module_name = fname[:-3] + module_type = module_name.replace('_plugin', '') file_path = os.path.join(plugintypes_dir, fname) debug_log.append(f"Checking plugin file: {fname}") try: @@ -322,7 +324,7 @@ def get_plugin_types(allowed_type_filter=None): display_name = "OpenAPI" description = "Plugin for integrating with external APIs using OpenAPI specifications. Supports file upload, URL download, and various authentication methods." types.append({ - 'type': module_name.replace('_plugin', ''), + 'type': module_type, 'class': attr, 'display': display_name, 'description': description @@ -340,7 +342,7 @@ def get_plugin_types(allowed_type_filter=None): # Only add minimal required fields based on plugin type #TODO: This can be improved by ensuring we have additional fields from the schemas we have not created if needed. - if 'databricks' in module_name.lower(): + if is_builtin_databricks_discovery_type(module_type): safe_manifest = { 'endpoint': 'https://adb-1234567890123456.7.azuredatabricks.net', 'auth': {'type': 'key', 'key': 'dummy'}, @@ -505,7 +507,7 @@ def get_plugin_types(allowed_type_filter=None): debug_log.append(f"Complete failure to instantiate {attr}: {e}. Using final fallback.") types.append({ - 'type': module_name.replace('_plugin', ''), + 'type': module_type, 'class': attr, 'display': display_name, 'description': description diff --git a/application/single_app/static/js/workspace/view-utils.js b/application/single_app/static/js/workspace/view-utils.js index 750aaf0fc..a415b1c7b 100644 --- a/application/single_app/static/js/workspace/view-utils.js +++ b/application/single_app/static/js/workspace/view-utils.js @@ -60,7 +60,7 @@ export function getTypeIcon(type) { if (t.includes("log_analytics")) return "bi-graph-up"; if (t.includes("msgraph")) return "bi-microsoft"; if (t.includes("azure_maps") || t.includes("openlayers")) return "bi-geo-alt"; - if (t.includes("databricks")) return "bi-bricks"; + if (t === "databricks" || t === "databricks_table") return "bi-bricks"; if (t.includes("snowflake")) return "bi-snow2"; if (t.includes("tableau")) return "bi-bar-chart"; if (t.includes("http") || t.includes("smart_http")) return "bi-cloud-arrow-up"; diff --git a/docs/explanation/fixes/CUSTOM_DATABRICKS_PLUGIN_DISCOVERY_FIX.md b/docs/explanation/fixes/CUSTOM_DATABRICKS_PLUGIN_DISCOVERY_FIX.md new file mode 100644 index 000000000..391cba8f7 --- /dev/null +++ b/docs/explanation/fixes/CUSTOM_DATABRICKS_PLUGIN_DISCOVERY_FIX.md @@ -0,0 +1,39 @@ +# Custom Databricks Plugin Discovery Fix + +Fixed/Implemented in version: **0.250.103** + +## Issue Description + +Custom plugin types with Databricks-prefixed names, such as `databricks_table_dscmo`, could inherit the built-in Databricks discovery defaults when action types were listed for the creation modal. These custom plugin types should remain on the standard plugin configuration path unless they are the exact built-in Databricks types. + +Associated issue: [microsoft/simplechat#1124](https://github.com/microsoft/simplechat/issues/1124) + +## Root Cause Analysis + +The action type discovery route selected the Databricks safe manifest whenever the plugin module name contained `databricks`. That substring check was too broad and could classify custom plugin modules as built-in Databricks plugins during metadata extraction. + +## Technical Details + +Files modified: + +- `application/single_app/functions_databricks_operations.py` +- `application/single_app/route_backend_plugins.py` +- `application/single_app/static/js/workspace/view-utils.js` +- `application/single_app/config.py` +- `functional_tests/test_plugin_type_discovery_custom_databricks.py` + +Code changes summary: + +- Added an exact built-in Databricks discovery classifier for `databricks` and `databricks_table`. +- Updated action type discovery to use the exact classifier instead of a broad `databricks` substring check. +- Updated shared action icon classification so custom Databricks-prefixed plugin types use the standard action icon. +- Added regression coverage for custom Databricks-prefixed plugin type discovery and visual classification. +- The regression test scaffolds a temporary `databricks_table_dscmo` plugin, definition file, and schema files, then confirms discovery and settings merge use the standard plugin path. + +## Validation + +- Added and ran `functional_tests/test_plugin_type_discovery_custom_databricks.py`. +- The test creates and removes its fake plugin scaffold at runtime, so no test-only plugin remains in the application plugin directory. +- Ran Python syntax checks for the changed Python files. + +Reference version update: `application/single_app/config.py` was updated to **0.250.103**. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 0fb0efb34..3e70c72bf 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -11,6 +11,14 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Ver * Cues play once for newly completed personal-chat responses outside the active visible conversation, with server-authoritative gating, cross-tab preference synchronization, and historical/duplicate suppression. * (Ref: Closes #1062, `completion-audio-cues.js`, notification polling, Profile and Admin Settings, `AI_RESPONSE_COMPLETION_AUDIO_CUES.md`) +#### Bug Fixes + +* **Custom Databricks-Prefixed Action Discovery** + * Fixed action type discovery so custom plugin types such as `databricks_table_dscmo` no longer inherit the built-in Databricks discovery defaults. + * Custom Databricks-prefixed plugin types now stay on the standard plugin configuration path and visual treatment unless their type is exactly `databricks` or `databricks_table`. + * Added a regression test that scaffolds a temporary fake custom Databricks-prefixed plugin, schema, and definition file to validate discovery and settings merge behavior. + * (Ref: microsoft/simplechat#1124, `functions_databricks_operations.py`, `route_backend_plugins.py`, `view-utils.js`, `test_plugin_type_discovery_custom_databricks.py`) + ### **(v0.250.102)** #### New Features diff --git a/functional_tests/test_plugin_type_discovery_custom_databricks.py b/functional_tests/test_plugin_type_discovery_custom_databricks.py new file mode 100644 index 000000000..bcbe87685 --- /dev/null +++ b/functional_tests/test_plugin_type_discovery_custom_databricks.py @@ -0,0 +1,254 @@ +# test_plugin_type_discovery_custom_databricks.py +#!/usr/bin/env python3 +""" +Functional test for custom Databricks-prefixed plugin type discovery. +Version: 0.250.103 +Implemented in: 0.250.103 + +This test ensures custom plugin types such as databricks_table_dscmo do not +receive the built-in Databricks discovery defaults or visual treatment that +drive the Databricks action creation experience. +""" + +import ast +import json +from pathlib import Path +import sys +import tempfile +import textwrap +import traceback + +from flask import Flask, jsonify + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "application" / "single_app")) +ROUTE_BACKEND_PLUGINS_FILE = REPO_ROOT / "application" / "single_app" / "route_backend_plugins.py" +VIEW_UTILS_FILE = REPO_ROOT / "application" / "single_app" / "static" / "js" / "workspace" / "view-utils.js" + +from functions_plugins import get_merged_plugin_settings # noqa: E402 +from functions_databricks_operations import is_builtin_databricks_discovery_type # noqa: E402 +from semantic_kernel_plugins.base_plugin import BasePlugin # noqa: E402 + + +def load_get_plugin_types_for_test(): + """Load get_plugin_types without importing the full route module and app config.""" + source = ROUTE_BACKEND_PLUGINS_FILE.read_text(encoding="utf-8") + tree = ast.parse(source, filename=str(ROUTE_BACKEND_PLUGINS_FILE)) + function_node = next( + node for node in tree.body + if isinstance(node, ast.FunctionDef) and node.name == "get_plugin_types" + ) + test_module = ast.Module(body=[function_node], type_ignores=[]) + ast.fix_missing_locations(test_module) + + namespace = { + "os": __import__("os"), + "importlib": __import__("importlib"), + "current_app": __import__("flask").current_app, + "jsonify": jsonify, + "BasePlugin": BasePlugin, + "debug_print": lambda *args, **kwargs: None, + "is_builtin_databricks_discovery_type": is_builtin_databricks_discovery_type, + } + exec(compile(test_module, str(ROUTE_BACKEND_PLUGINS_FILE), "exec"), namespace) + return namespace["get_plugin_types"] + + +def scaffold_fake_databricks_prefixed_plugin(root_path: Path) -> Path: + """Create a temporary plugin plus matching schema and definition files.""" + plugin_dir = root_path / "semantic_kernel_plugins" + schema_dir = root_path / "static" / "json" / "schemas" + plugin_dir.mkdir(parents=True) + schema_dir.mkdir(parents=True) + + plugin_file = plugin_dir / "databricks_table_dscmo_plugin.py" + plugin_file.write_text( + textwrap.dedent( + ''' + # databricks_table_dscmo_plugin.py + """Temporary test plugin for custom Databricks-prefixed discovery.""" + + from typing import Any, Dict, Optional + + from semantic_kernel_plugins.base_plugin import BasePlugin + + + class DatabricksTableDscmoPlugin(BasePlugin): + def __init__(self, manifest: Optional[Dict[str, Any]] = None): + super().__init__(manifest) + additional_fields = self.manifest.get("additionalFields", {}) + self.received_databricks_manifest = bool(additional_fields.get("warehouse_id")) + + @property + def display_name(self) -> str: + if self.received_databricks_manifest: + return "Databricks UI Fake DSCMO" + return "Standard Fake DSCMO" + + @property + def metadata(self) -> Dict[str, Any]: + description = ( + "Databricks-specific metadata" + if self.received_databricks_manifest + else "Standard DSCMO metadata" + ) + return { + "name": "databricks_table_dscmo", + "type": "databricks_table_dscmo", + "description": description, + "methods": [] + } + ''' + ).lstrip(), + encoding="utf-8", + ) + + (schema_dir / "databricks_table_dscmo.definition.json").write_text( + json.dumps( + { + "$schema": "./plugin.definition.schema.json", + "allowedAuthTypes": ["key", "identity"], + }, + indent=2, + ), + encoding="utf-8", + ) + (schema_dir / "databricks_table_dscmo_plugin.additional_settings.schema.json").write_text( + json.dumps( + { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "DSCMO Plugin Additional Settings", + "type": "object", + "properties": { + "dscmo_table": { + "type": "string", + "default": "customers", + "description": "Fake DSCMO table name." + } + }, + "required": ["dscmo_table"], + "additionalProperties": True, + }, + indent=2, + ), + encoding="utf-8", + ) + (schema_dir / "databricks_table_dscmo_plugin.metadata.schema.json").write_text( + json.dumps( + { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "DSCMO Plugin Metadata", + "type": "object", + "properties": { + "ui_variant": { + "type": "string", + "default": "standard", + "description": "Expected UI variant." + } + }, + "required": ["ui_variant"], + "additionalProperties": True, + }, + indent=2, + ), + encoding="utf-8", + ) + + return schema_dir + + +def test_custom_databricks_prefixed_type_uses_standard_discovery() -> bool: + """Verify only exact built-in Databricks types get Databricks discovery defaults.""" + print("Testing custom Databricks-prefixed plugin discovery classification...") + + built_in_types = ["databricks", "databricks_table", "DATABRICKS_TABLE"] + custom_types = ["databricks_table_dscmo", "databricks_custom", "custom_databricks_table"] + + for plugin_type in built_in_types: + if not is_builtin_databricks_discovery_type(plugin_type): + print(f"Expected built-in Databricks type classification for: {plugin_type}") + return False + + for plugin_type in custom_types: + if is_builtin_databricks_discovery_type(plugin_type): + print(f"Unexpected Databricks type classification for custom type: {plugin_type}") + return False + + print("Custom Databricks-prefixed plugin discovery classification test passed!") + return True + + +def test_databricks_icon_classification_uses_exact_types() -> bool: + """Verify visual type classification does not broad-match custom Databricks names.""" + print("Testing Databricks action icon classification...") + + content = VIEW_UTILS_FILE.read_text(encoding="utf-8") + exact_match_marker = 'if (t === "databricks" || t === "databricks_table") return "bi-bricks";' + broad_match_marker = 'if (t.includes("databricks")) return "bi-bricks";' + + if exact_match_marker not in content: + print("Missing exact Databricks icon classification marker.") + return False + if broad_match_marker in content: + print("Found broad Databricks icon classification marker.") + return False + + print("Databricks action icon classification test passed!") + return True + + +def test_fake_databricks_prefixed_plugin_uses_standard_discovery_path() -> bool: + """Scaffold a temporary plugin and verify discovery keeps it standard.""" + print("Testing fake Databricks-prefixed plugin scaffold through discovery...") + + get_plugin_types = load_get_plugin_types_for_test() + with tempfile.TemporaryDirectory() as temp_dir: + root_path = Path(temp_dir) + schema_dir = scaffold_fake_databricks_prefixed_plugin(root_path) + app = Flask("custom-databricks-plugin-test", root_path=str(root_path)) + + with app.app_context(): + response = get_plugin_types() + plugin_types = response.get_json() + + fake_plugin = next( + (plugin_type for plugin_type in plugin_types if plugin_type.get("type") == "databricks_table_dscmo"), + None, + ) + if fake_plugin is None: + print("Fake databricks_table_dscmo plugin was not discovered.") + return False + if fake_plugin.get("display") != "Standard Fake DSCMO": + print(f"Fake plugin used the wrong UI path: {fake_plugin}") + return False + if fake_plugin.get("description") != "Standard DSCMO metadata": + print(f"Fake plugin used the wrong metadata path: {fake_plugin}") + return False + + merged_settings = get_merged_plugin_settings("databricks_table_dscmo", {}, str(schema_dir)) + if merged_settings.get("additionalFields", {}).get("dscmo_table") != "customers": + print(f"Fake additional settings schema did not merge correctly: {merged_settings}") + return False + if merged_settings.get("metadata", {}).get("ui_variant") != "standard": + print(f"Fake metadata schema did not merge correctly: {merged_settings}") + return False + + print("Fake Databricks-prefixed plugin scaffold discovery test passed!") + return True + + +if __name__ == "__main__": + try: + tests = [ + test_custom_databricks_prefixed_type_uses_standard_discovery, + test_databricks_icon_classification_uses_exact_types, + test_fake_databricks_prefixed_plugin_uses_standard_discovery_path, + ] + results = [bool(test()) for test in tests] + success = all(results) + print(f"Results: {sum(results)}/{len(results)} tests passed") + except Exception as exc: + print(f"Test failed: {exc}") + traceback.print_exc() + success = False + sys.exit(0 if success else 1)