From 58a177030e25a3d5f6cf8ec120eb9ab8d0e3f703 Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 22 Jul 2026 10:15:29 +0530 Subject: [PATCH 01/13] multitenancy for google-drive v1 Signed-off-by: gokul-aot --- playground/app.js | 146 ++++++- playground/index.html | 50 +++ playground/scenarios.py | 120 +++-- playground/style.css | 69 ++- src/bindings/factory.py | 295 ++++++++++--- src/bindings/grpc_server/connector.proto | 1 + src/bindings/grpc_server/connector_pb2.py | 12 +- .../grpc_server/connector_pb2_grpc.py | 2 +- src/bindings/grpc_server/server.py | 33 +- src/bindings/mcp_server/server.py | 41 +- src/bindings/rest_api/app.py | 167 ++++++- src/node_wire_runtime/__init__.py | 28 +- src/node_wire_runtime/base_connector.py | 11 +- src/node_wire_runtime/config_store.py | 413 ++++++++++++++++++ src/node_wire_runtime/identity.py | 76 ++++ src/node_wire_runtime/policy.py | 23 +- src/node_wire_runtime/secrets/__init__.py | 4 + src/node_wire_runtime/secrets/base.py | 47 ++ tests/test_factory_and_rest.py | 51 +-- tests/test_mcp_auth.py | 12 +- tests/test_multitenancy.py | 357 +++++++++++++++ tests/test_rest_body_limit.py | 3 +- tests/test_rest_connector_dispatch.py | 11 +- tests/test_rest_rate_limit_enforcement.py | 9 +- 24 files changed, 1804 insertions(+), 177 deletions(-) create mode 100644 src/node_wire_runtime/config_store.py create mode 100644 src/node_wire_runtime/identity.py create mode 100644 tests/test_multitenancy.py diff --git a/playground/app.js b/playground/app.js index b2c49525..e0c3af34 100644 --- a/playground/app.js +++ b/playground/app.js @@ -779,6 +779,32 @@ document.addEventListener('DOMContentLoaded', () => { gdriveActionSelect.addEventListener('change', scheduleGdriveDriveActionSyncFromUser); } + function getPlaygroundTenantId() { + const el = document.getElementById('playground-tenant-id'); + return (el && el.value ? el.value : '').trim(); + } + + function getPlaygroundConfigName() { + const el = document.getElementById('playground-config-name'); + return (el && el.value ? el.value : '').trim(); + } + + function playgroundRequestHeaders(extra = {}) { + const headers = { 'Content-Type': 'application/json', ...extra }; + const tenantId = getPlaygroundTenantId(); + if (tenantId) { + headers['X-Tenant-ID'] = tenantId; + } + return headers; + } + + function withTenancyQuery(endpoint) { + const configName = getPlaygroundConfigName(); + if (!configName) return endpoint; + const sep = endpoint.includes('?') ? '&' : '?'; + return `${endpoint}${sep}config_name=${encodeURIComponent(configName)}`; + } + async function handleSubmission(payload, endpoint, btn, btnLbl, spinner, resetText, pipelineLabelOverride = null) { resetUI(pipelineLabelOverride); @@ -787,22 +813,38 @@ document.addEventListener('DOMContentLoaded', () => { btnLbl.textContent = 'Orchestrating...'; log(`Initiating intelligent ${currentMode.toUpperCase()} orchestration...`, 'system'); + const tenantId = getPlaygroundTenantId(); + const configName = getPlaygroundConfigName(); + if (tenantId) { + log(`Tenant: ${tenantId}`, 'system'); + } else { + log('Tenant: __default__ (no X-Tenant-ID)', 'system'); + } + if (configName) { + log(`Config name: ${configName}`, 'system'); + payload = { ...payload, config_name: configName }; + } + const firstNode = nodes.find((n) => n && !n.classList.contains('hidden')); if (firstNode) firstNode.classList.add('active'); try { - const response = await fetch(endpoint, { + const response = await fetch(withTenancyQuery(endpoint), { method: 'POST', - headers: { 'Content-Type': 'application/json' }, + headers: playgroundRequestHeaders(), body: JSON.stringify(payload) }); - if (!response.ok) throw new Error(`Server returned ${response.status}`); + const data = await response.json().catch(() => ({})); - const data = await response.json(); - traceDisplay.textContent = data.trace_id.toUpperCase(); + if (!response.ok) { + const detail = data.detail || data.message || `Server returned ${response.status}`; + throw new Error(typeof detail === 'string' ? detail : JSON.stringify(detail)); + } - for (let i = 0; i < data.steps.length; i++) { + traceDisplay.textContent = (data.trace_id || '').toUpperCase() || '—'; + + for (let i = 0; i < (data.steps || []).length; i++) { const step = data.steps[i]; const node = nodes[i]; if (!node) continue; @@ -1250,6 +1292,98 @@ document.addEventListener('DOMContentLoaded', () => { } }); + // --- Google Drive runtime config CRUD (tenant-scoped via Tenancy bar) --- + const gdriveCfgResult = document.getElementById('gdrive-cfg-result'); + const GDRIVE_CFG_BASE = '/v1/connectors/google_drive/configs'; + + function showGdriveCfgResult(obj) { + if (!gdriveCfgResult) return; + gdriveCfgResult.classList.remove('hidden'); + gdriveCfgResult.textContent = typeof obj === 'string' ? obj : JSON.stringify(obj, null, 2); + } + + async function gdriveCfgFetch(method, path = '', body = null) { + const tenantId = getPlaygroundTenantId(); + if (!tenantId) { + throw new Error('Set Tenant ID in the Tenancy bar before managing configs (e.g. acme).'); + } + const opts = { method, headers: playgroundRequestHeaders() }; + if (body != null) { + opts.body = JSON.stringify(body); + } + const response = await fetch(`${GDRIVE_CFG_BASE}${path}`, opts); + const data = await response.json().catch(() => ({})); + if (!response.ok) { + const detail = data.detail || `HTTP ${response.status}`; + throw new Error(typeof detail === 'string' ? detail : JSON.stringify(detail)); + } + return data; + } + + document.getElementById('gdrive-cfg-create')?.addEventListener('click', async () => { + try { + const name = (document.getElementById('gdrive-cfg-name')?.value || 'default').trim(); + const isDefault = document.getElementById('gdrive-cfg-default')?.value === 'true'; + const data = await gdriveCfgFetch('POST', '', { + name, + default: isDefault, + config: {}, + // Same auth shape as connectors.yaml; tenant secrets resolve via + // NW_{TENANT}_GOOGLE_DRIVE_GOOGLE_DRIVE_SA_JSON (or plain GOOGLE_DRIVE_SA_JSON for __default__). + auth: { + provider: 'service_account', + sa_json_secret: 'GOOGLE_DRIVE_SA_JSON', + scopes: ['https://www.googleapis.com/auth/drive'], + }, + }); + showGdriveCfgResult(data); + log(`Created google_drive config '${name}' for tenant '${getPlaygroundTenantId()}'`, 'success'); + } catch (err) { + showGdriveCfgResult({ error: err.message }); + log(`Config create failed: ${err.message}`, 'error'); + } + }); + + document.getElementById('gdrive-cfg-list')?.addEventListener('click', async () => { + try { + const data = await gdriveCfgFetch('GET'); + showGdriveCfgResult(data); + log(`Listed google_drive configs for tenant '${getPlaygroundTenantId()}'`, 'success'); + } catch (err) { + showGdriveCfgResult({ error: err.message }); + log(`Config list failed: ${err.message}`, 'error'); + } + }); + + document.getElementById('gdrive-cfg-set-default')?.addEventListener('click', async () => { + try { + const name = (document.getElementById('gdrive-cfg-name')?.value || '').trim(); + if (!name) throw new Error('Config name is required'); + const data = await gdriveCfgFetch('PUT', `/${encodeURIComponent(name)}/default`); + showGdriveCfgResult(data); + log(`Set '${name}' as default for tenant '${getPlaygroundTenantId()}'`, 'success'); + } catch (err) { + showGdriveCfgResult({ error: err.message }); + log(`Set default failed: ${err.message}`, 'error'); + } + }); + + document.getElementById('gdrive-cfg-delete')?.addEventListener('click', async () => { + try { + const name = (document.getElementById('gdrive-cfg-name')?.value || '').trim(); + if (!name) throw new Error('Config name is required'); + // If deleting the default while siblings exist, nominate "default" or leave blank for last-config case. + const newDefault = name === 'default' ? '' : 'default'; + const q = newDefault ? `?new_default=${encodeURIComponent(newDefault)}` : ''; + const data = await gdriveCfgFetch('DELETE', `/${encodeURIComponent(name)}${q}`); + showGdriveCfgResult(data); + log(`Deleted google_drive config '${name}' for tenant '${getPlaygroundTenantId()}'`, 'success'); + } catch (err) { + showGdriveCfgResult({ error: err.message }); + log(`Config delete failed: ${err.message}`, 'error'); + } + }); + if (slackActionSelect) { slackActionSelect.addEventListener('change', () => { const action = slackActionSelect.value; diff --git a/playground/index.html b/playground/index.html index 0882d126..f75268ab 100644 --- a/playground/index.html +++ b/playground/index.html @@ -297,6 +297,28 @@

Salesforce

Back to All Connectors + +
+
+

Tenancy

+ Header-based tenant & named config +
+
+
+ + + Sent as X-Tenant-ID. Missing → __default__ (YAML bootstrap). +
+
+ + + Optional named config for this tenant. Unknown name → same 403 as unconfigured tenant. +
+
+
+
- +
+ + +
@@ -183,6 +200,45 @@

node-wire MCP via ToolHive

+ + + + diff --git a/playground/scenarios.py b/playground/scenarios.py index 61a947e9..5973e74f 100644 --- a/playground/scenarios.py +++ b/playground/scenarios.py @@ -22,6 +22,7 @@ from node_wire_runtime.config_store import ConfigNotFoundError from node_wire_runtime.identity import ( MissingTenantError, + is_multitenancy_enabled, resolve_config_name, resolve_tenant_id, ) @@ -366,6 +367,13 @@ async def resolve_connector( except MissingTenantError as exc: raise HTTPException(status_code=400, detail=str(exc)) from exc name = resolve_config_name((config_name or "").strip() or None) + if is_multitenancy_enabled(): + logger.info( + "Playground action | connector=%s | tenant_id=%s | config_name=%s", + connector_id, + tenant_id, + name or "(default)", + ) try: return await factory.get( connector_id, @@ -1890,15 +1898,47 @@ def _current_agent_transport() -> str: return transport if transport in {"stdio", "streamable-http"} else "stdio" +def _resolve_agent_mcp_tenant(request: Request) -> str: + """Resolve tenant for playground agent → MCP (stdio pin / HTTP header).""" + from bindings.rest_api.auth import get_rest_caller_identity + + try: + return resolve_tenant_id( + headers=request.headers, + jwt_identity=get_rest_caller_identity(request), + ) + except MissingTenantError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + + +def _agent_mcp_extra_headers(tenant_id: str) -> Dict[str, str]: + from node_wire_runtime.identity import TENANT_HEADER + + # TENANT_HEADER is lowercased; HTTP clients accept any casing. + return {TENANT_HEADER: tenant_id} + + +def _playground_inprocess_mcp_client(tenant_id: str, request: Request): + """Share playground factory/store with agent tool calls (named tenants).""" + from agents.toolhive import InProcessMcpClient + from bindings.mcp_server.server import McpServer + + config_name = resolve_config_name( + (request.query_params.get("config_name") or "").strip() or None + ) + factory = get_playground_factory() + server = McpServer(server_name="node-wire-playground-agent", factory=factory) + return InProcessMcpClient(server, tenant_id=tenant_id, config_name=config_name) + + @router.post("/agent-chat", response_model=AgentChatResponse) -async def agent_chat(payload: AgentChatInput) -> AgentChatResponse: +async def agent_chat(request: Request, payload: AgentChatInput) -> AgentChatResponse: """ AI Agent chatbot endpoint. Accepts a user message + conversation history, runs through the ToolHiveAgent, and returns the agent's reply with any tool steps executed. """ import os - import sys trace_id = str(uuid.uuid4()) logger.info( @@ -1915,12 +1955,23 @@ async def agent_chat(payload: AgentChatInput) -> AgentChatResponse: success=False, ) + tenant_id = _resolve_agent_mcp_tenant(request) + mcp_headers = _agent_mcp_extra_headers(tenant_id) + config_name = resolve_config_name( + (request.query_params.get("config_name") or "").strip() or None + ) + if is_multitenancy_enabled(): + logger.info( + "Agent Chat | mcp_tenant_id=%s | config_name=%s", + tenant_id, + config_name or "(default)", + ) + try: from agents.llm_factory import LLMProviderFactory from agents.toolhive import ( MultiMcpClient, ToolHiveAgent, - StdioMcpClient, resolve_mcp_urls, resolve_max_tool_failures, ) @@ -1932,21 +1983,28 @@ async def agent_chat(payload: AgentChatInput) -> AgentChatResponse: task = _build_agent_chat_task(payload) - # Determine MCP transport — try proxy first, fallback to local stdio + # Determine MCP transport — try proxy first, fallback to local in-process transport = _current_agent_transport() urls = resolve_mcp_urls() if transport == "streamable-http" else [] run_result = None fallback_to_stdio = ( - os.environ.get("PLAYGROUND_AGENT_PROXY_FALLBACK_TO_STDIO", "false").lower() == "true" + os.environ.get("PLAYGROUND_AGENT_PROXY_FALLBACK_TO_STDIO", "true").lower() == "true" ) if urls: logger.info("Agent Chat | trying ToolHive proxy URL(s): %s", ",".join(urls)) try: if len(urls) == 1: - mcp_client = create_http_mcp_client(urls[0]) + mcp_client = create_http_mcp_client( + urls[0], extra_headers=mcp_headers + ) else: - mcp_client = MultiMcpClient([create_http_mcp_client(u) for u in urls]) + mcp_client = MultiMcpClient( + [ + create_http_mcp_client(u, extra_headers=mcp_headers) + for u in urls + ] + ) agent = ToolHiveAgent( mcp_client, llm_provider, @@ -1996,10 +2054,9 @@ async def agent_chat(payload: AgentChatInput) -> AgentChatResponse: ) if run_result is None: - # Use local stdio transport - logger.info("Agent Chat | using local stdio MCP transport") - cmd = [sys.executable, "-m", "agents.mcp_entrypoint"] - async with StdioMcpClient(cmd) as mcp_client: + # In-process MCP shares the playground config store (named tenants). + logger.info("Agent Chat | using in-process MCP (shared factory)") + async with _playground_inprocess_mcp_client(tenant_id, request) as mcp_client: agent = ToolHiveAgent( mcp_client, llm_provider, @@ -2044,22 +2101,30 @@ async def agent_chat(payload: AgentChatInput) -> AgentChatResponse: @router.post("/agent-chat-stream") -async def agent_chat_stream(payload: AgentChatInput) -> Any: +async def agent_chat_stream(request: Request, payload: AgentChatInput) -> Any: """ Stream agent progress and final-answer chunks to web clients. The terminal ``done`` event includes ``trace_id`` and ``message``. Clients should stop their streaming loader only when that event arrives. """ + tenant_id = _resolve_agent_mcp_tenant(request) + mcp_headers = _agent_mcp_extra_headers(tenant_id) + config_name = resolve_config_name( + (request.query_params.get("config_name") or "").strip() or None + ) + if is_multitenancy_enabled(): + logger.info( + "Agent Chat stream | mcp_tenant_id=%s | config_name=%s", + tenant_id, + config_name or "(default)", + ) async def stream_events(): try: - import sys - from agents.llm_factory import LLMProviderFactory from agents.toolhive import ( MultiMcpClient, - StdioMcpClient, ToolHiveAgent, resolve_mcp_urls, resolve_max_tool_failures, @@ -2094,25 +2159,61 @@ async def stream_events(): task = _build_agent_chat_task(payload) transport = _current_agent_transport() urls = resolve_mcp_urls() if transport == "streamable-http" else [] + # Default true: playground should work without a separate MCP on :8081. + fallback_to_local = ( + os.environ.get("PLAYGROUND_AGENT_PROXY_FALLBACK_TO_STDIO", "true").lower() + == "true" + ) if urls: - if len(urls) == 1: - mcp_client = create_http_mcp_client(urls[0]) - else: - mcp_client = MultiMcpClient([create_http_mcp_client(u) for u in urls]) - agent = ToolHiveAgent( - mcp_client, - llm_provider, - max_steps=10, - max_tool_failures=resolve_max_tool_failures(None), - ) - agent._system_prompt = AGENT_GUARDRAIL_PROMPT - async for event in agent.run_events(task): - yield json.dumps(event) + "\n" - return + proxy_ok = False + try: + if len(urls) == 1: + mcp_client = create_http_mcp_client( + urls[0], extra_headers=mcp_headers + ) + else: + mcp_client = MultiMcpClient( + [ + create_http_mcp_client(u, extra_headers=mcp_headers) + for u in urls + ] + ) + agent = ToolHiveAgent( + mcp_client, + llm_provider, + max_steps=10, + max_tool_failures=resolve_max_tool_failures(None), + ) + agent._system_prompt = AGENT_GUARDRAIL_PROMPT + async for event in agent.run_events(task): + if ( + fallback_to_local + and event.get("type") == "error" + and "Failed to list MCP tools" in str(event.get("message") or "") + ): + logger.warning( + "Agent Chat stream | proxy incomplete (%s) — " + "falling back to in-process MCP", + event.get("message"), + ) + break + proxy_ok = True + yield json.dumps(event) + "\n" + else: + # Exhausted generator without break → proxy finished normally. + return + if proxy_ok: + return + except Exception as proxy_err: + if not fallback_to_local: + raise + logger.warning( + "Agent Chat stream | proxy error: %s — falling back to in-process MCP", + proxy_err, + ) - cmd = [sys.executable, "-m", "agents.mcp_entrypoint"] - async with StdioMcpClient(cmd) as mcp_client: + async with _playground_inprocess_mcp_client(tenant_id, request) as mcp_client: agent = ToolHiveAgent( mcp_client, llm_provider, diff --git a/playground/style.css b/playground/style.css index a217e43d..1863ad1a 100644 --- a/playground/style.css +++ b/playground/style.css @@ -141,7 +141,7 @@ body { display: flex; gap: 0.75rem; align-items: center; - justify-content: space-between; + justify-content: flex-start; margin-bottom: 1.5rem; flex-wrap: wrap; } @@ -154,18 +154,20 @@ body { display: inline-flex; align-items: center; justify-content: center; - gap: 0.45rem; - min-height: 2.75rem; - padding: 0.7rem 1.4rem; - border-radius: 12px; + gap: 0.35rem; + align-self: flex-end; + min-height: 2.05rem; + padding: 0.35rem 0.85rem; + border-radius: 8px; border: 1px solid color-mix(in srgb, var(--brand-accent) 55%, var(--border)); background: color-mix(in srgb, var(--brand-accent) 18%, transparent); color: var(--text-main); font-family: inherit; - font-size: 0.9rem; + font-size: 0.8rem; font-weight: 650; letter-spacing: 0.01em; cursor: pointer; + white-space: nowrap; box-shadow: 0 1px 0 rgba(255, 255, 255, 0.06) inset; transition: background 0.2s, border-color 0.2s, transform 0.2s, box-shadow 0.2s; } @@ -182,8 +184,68 @@ body { outline-offset: 2px; } +.config-admin-modal { + position: fixed; + inset: 0; + z-index: 1200; + display: flex; + align-items: center; + justify-content: center; + padding: 1.25rem; +} + +.config-admin-modal.hidden { + display: none; +} + +.config-admin-modal-backdrop { + position: absolute; + inset: 0; + background: rgba(15, 23, 42, 0.55); + backdrop-filter: blur(2px); +} + +.config-admin-modal-card { + position: relative; + z-index: 1; + width: min(40rem, 100%); + max-height: min(90vh, 52rem); + overflow: auto; + margin: 0; + padding: 1.35rem 1.6rem 1.5rem; + box-shadow: 0 24px 48px rgba(0, 0, 0, 0.35); +} + +.config-admin-modal-title { + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 1rem; + margin-bottom: 0.65rem; +} + +.config-admin-close { + display: inline-flex; + align-items: center; + justify-content: center; + flex-shrink: 0; + width: 2rem; + height: 2rem; + border: 1px solid var(--border); + border-radius: 8px; + background: transparent; + color: var(--text-muted); + cursor: pointer; + transition: background 0.15s, color 0.15s, border-color 0.15s; +} + +.config-admin-close:hover { + color: var(--text-main); + border-color: var(--brand-accent); + background: color-mix(in srgb, var(--brand-accent) 12%, transparent); +} + .config-admin-panel { - margin-bottom: 1.5rem; padding: 1.35rem 1.6rem 1.5rem; } diff --git a/sample.env b/sample.env index 3dce90d3..b880d8ca 100644 --- a/sample.env +++ b/sample.env @@ -7,6 +7,13 @@ # named configs, and tenancy controls in the playground. NW_MULTITENANCY_ENABLED=false # +# MCP stdio / ToolHive: pin one process to one tenant (ignored on streamable-http; +# HTTP clients must send X-Tenant-ID instead). Required when multitenancy is enabled +# for stdio launches. +# NW_TENANT_ID=acme +# Optional header name override (REST / MCP HTTP / gRPC metadata): +# NW_TENANT_ID_HEADER=X-Tenant-ID +# # Named tenants (anything other than __default__) resolve secrets strictly via # TenantSecretProvider as: NW_{TENANT}_{CONNECTOR}_{KEY} # (segments uppercased; non-alphanumeric → _). No fallback to shared env vars. diff --git a/src/agents/llm_factory.py b/src/agents/llm_factory.py index 0d293b2f..53191fed 100644 --- a/src/agents/llm_factory.py +++ b/src/agents/llm_factory.py @@ -67,6 +67,34 @@ def wants_tool_call(self) -> bool: return bool(self.tool_calls) +def openai_compatible_tool_parameters(input_schema: Optional[Dict[str, Any]]) -> Dict[str, Any]: + """Copy a JSON Schema so optional properties also accept ``null``. + + Providers such as Groq validate model tool calls against the schema and reject + ``"field": null`` when the type is only ``"string"``. Models often emit null + for unused optional keys instead of omitting them. + """ + import copy + + schema = copy.deepcopy(input_schema) if input_schema else {"type": "object", "properties": {}} + props = schema.get("properties") + if not isinstance(props, dict): + return schema + required = set(schema.get("required") or []) + for key, prop in props.items(): + if key in required or not isinstance(prop, dict): + continue + t = prop.get("type") + if t is None: + continue + if isinstance(t, list): + if "null" not in t: + prop["type"] = [*t, "null"] + elif t != "null": + prop["type"] = [t, "null"] + return schema + + # --------------------------------------------------------------------------- # Abstract base # --------------------------------------------------------------------------- diff --git a/src/agents/providers/groq_provider.py b/src/agents/providers/groq_provider.py index 8105efb4..1b55e769 100644 --- a/src/agents/providers/groq_provider.py +++ b/src/agents/providers/groq_provider.py @@ -18,7 +18,8 @@ import logging from typing import Any, Dict, List, cast -from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall +from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall, openai_compatible_tool_parameters + logger = logging.getLogger("agents.providers.groq") @@ -30,7 +31,7 @@ def _mcp_tool_to_groq(tool: Dict[str, Any]) -> Dict[str, Any]: "function": { "name": tool["name"], "description": tool.get("description", ""), - "parameters": tool.get("input_schema", {"type": "object", "properties": {}}), + "parameters": openai_compatible_tool_parameters(tool.get("input_schema")), }, } diff --git a/src/agents/providers/openai_provider.py b/src/agents/providers/openai_provider.py index 29d1b500..e7654569 100644 --- a/src/agents/providers/openai_provider.py +++ b/src/agents/providers/openai_provider.py @@ -17,7 +17,7 @@ import logging from typing import Any, Dict, List, cast -from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall +from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall, openai_compatible_tool_parameters logger = logging.getLogger("agents.providers.openai") @@ -28,7 +28,7 @@ def _mcp_tool_to_openai(tool: Dict[str, Any]) -> Dict[str, Any]: "function": { "name": tool["name"], "description": tool.get("description", ""), - "parameters": tool.get("input_schema", {"type": "object", "properties": {}}), + "parameters": openai_compatible_tool_parameters(tool.get("input_schema")), }, } diff --git a/src/agents/toolhive.py b/src/agents/toolhive.py index de8cb035..6a511206 100644 --- a/src/agents/toolhive.py +++ b/src/agents/toolhive.py @@ -48,7 +48,7 @@ import uuid from contextlib import AsyncExitStack from dataclasses import dataclass, field -from typing import Any, AsyncIterator, Dict, List, Optional, Protocol, Union, runtime_checkable +from typing import Any, AsyncIterator, Dict, List, Mapping, Optional, Protocol, Union, runtime_checkable import re from dotenv import load_dotenv @@ -90,6 +90,11 @@ def _redact_tool_args_for_log(tool_name: str, args: Dict[str, Any]) -> Dict[str, return scrubbed +def omit_null_tool_args(arguments: Optional[Dict[str, Any]]) -> Dict[str, Any]: + """Drop keys whose value is ``None`` (LLM 'omit optional' often arrives as null).""" + return {k: v for k, v in dict(arguments or {}).items() if v is not None} + + def truncate_tool_result_for_llm(text: str) -> str: """ Cap tool output size sent to the LLM so providers with strict limits (e.g. Groq @@ -241,19 +246,26 @@ class ToolHiveMcpClient: response header must be forwarded in all subsequent requests. """ - def __init__(self, base_url: str) -> None: + def __init__( + self, + base_url: str, + *, + extra_headers: Optional[Mapping[str, str]] = None, + ) -> None: self._base_url = base_url.rstrip("/") self._session_id: Optional[str] = None self._initialized: bool = False self._auth_token: Optional[str] = os.environ.get( "TOOLHIVE_MCP_BEARER_TOKEN" ) or os.environ.get("TOOLHIVE_MCP_API_KEY") + self._extra_headers: Dict[str, str] = dict(extra_headers or {}) def _build_request_headers(self) -> Dict[str, str]: headers: Dict[str, str] = { "Content-Type": "application/json", "Accept": "application/json, text/event-stream", } + headers.update(self._extra_headers) if self._session_id: headers["Mcp-Session-Id"] = self._session_id # For MCP auth-gated servers, send both forms for compatibility. @@ -425,8 +437,14 @@ class StdioMcpClient: Useful for local manual testing without ToolHive. """ - def __init__(self, command: List[str]) -> None: + def __init__( + self, + command: List[str], + *, + env: Optional[Mapping[str, str]] = None, + ) -> None: self._command = command + self._env = dict(env) if env is not None else None self._exit_stack = AsyncExitStack() self._session: Any = None @@ -437,10 +455,13 @@ async def __aenter__(self) -> StdioMcpClient: except ImportError as exc: raise ImportError("mcp SDK not installed.") from exc + child_env = os.environ.copy() + if self._env: + child_env.update({k: str(v) for k, v in self._env.items() if v is not None}) params = StdioServerParameters( command=self._command[0], args=self._command[1:], - env=os.environ.copy(), + env=child_env, ) stdio_transport = await self._exit_stack.enter_async_context(stdio_client(params)) self._read, self._write = stdio_transport @@ -467,10 +488,52 @@ async def call_tool(self, name: str, arguments: Dict[str, Any]) -> str: if not self._session: raise RuntimeError("Client not initialised. Use 'async with'") resp = await self._session.call_tool(name, arguments) - parts = [c.text for c in resp.content if hasattr(c, "text")] + # Extract text content + parts = [] + for block in resp.content: + if hasattr(block, "text"): + parts.append(block.text) + else: + parts.append(str(block)) return "\n".join(parts) +class InProcessMcpClient: + """MCP client backed by an in-process :class:`McpServer` (shared factory/store). + + Used by the playground agent so named-tenant configs created via REST are + visible to tool calls without spawning a separate stdio process. + """ + + def __init__( + self, + server: Any, + *, + tenant_id: str, + config_name: Optional[str] = None, + ) -> None: + self._server = server + self._tenant_id = tenant_id + self._config_name = config_name + + async def __aenter__(self) -> InProcessMcpClient: + self._server._stdio_env_tenant_pin = self._tenant_id + return self + + async def __aexit__(self, exc_type, exc_val, exc_tb) -> None: + return None + + async def list_tools(self) -> List[Dict[str, Any]]: + return self._server.list_tools() + + async def call_tool(self, name: str, arguments: Dict[str, Any]) -> str: + args = omit_null_tool_args(arguments) + if self._config_name and not args.get("config_name"): + args["config_name"] = self._config_name + result = await self._server.invoke_tool(name, args) + return json.dumps(result, default=str) + + # --------------------------------------------------------------------------- # The Agent # --------------------------------------------------------------------------- @@ -617,7 +680,9 @@ async def run(self, task: str) -> AgentRunResult: ) try: - tool_result_str = await self._mcp.call_tool(tc.name, tc.arguments) + tool_result_str = await self._mcp.call_tool( + tc.name, omit_null_tool_args(tc.arguments) + ) logger.info( "Tool %s returned response of length: %d chars", tc.name, @@ -786,7 +851,9 @@ async def _run_events_inner(self, task: str, trace_id: str) -> AsyncIterator[Dic logger.info("Calling tool: %s | args=%s", tc.name, scrubbed_args) try: - tool_result_str = await self._mcp.call_tool(tc.name, tc.arguments) + tool_result_str = await self._mcp.call_tool( + tc.name, omit_null_tool_args(tc.arguments) + ) logger.info("Tool %s returned: %.200s", tc.name, tool_result_str) except Exception as exc: tool_result_str = f"ERROR: {exc}" diff --git a/src/bindings/mcp_server/server.py b/src/bindings/mcp_server/server.py index 0b717d6b..26c7a73d 100644 --- a/src/bindings/mcp_server/server.py +++ b/src/bindings/mcp_server/server.py @@ -23,6 +23,7 @@ from node_wire_runtime.config_store import ConfigNotFoundError from node_wire_runtime.identity import ( MissingTenantError, + is_multitenancy_enabled, resolve_config_name, resolve_tenant_id, ) @@ -67,6 +68,13 @@ def is_public_bind_host(host: str) -> bool: default=None, ) +# Pinned factory tenant for the current streamable-http request (§6.3). +# Set with env_pin=None so process NW_TENANT_ID cannot override X-Tenant-ID. +_session_tenant_ctx: ContextVar[str | None] = ContextVar( + "mcp_session_tenant", + default=None, +) + def _process_response_payload(data: Any, max_items: int) -> Tuple[Any, bool, int, Optional[str]]: """ @@ -176,14 +184,18 @@ def __init__( *, server_name: str = "node-wire", connector_ids: Optional[List[str]] = None, + factory: ConnectorFactory | None = None, ) -> None: self._server_name = server_name self._connector_ids: Optional[frozenset[str]] = ( None if connector_ids is None else frozenset(connector_ids) ) auto_register() - self._factory = ConnectorFactory() - self._factory.load() + if factory is not None: + self._factory = factory + else: + self._factory = ConnectorFactory() + self._factory.load() self._upstream_passthrough = _resolve_upstream_passthrough( self._factory, self._connector_ids ) @@ -192,6 +204,8 @@ def __init__( if self._upstream_passthrough else () ) + # §6.4: set in run_stdio from NW_TENANT_ID; unused on HTTP (session pin). + self._stdio_env_tenant_pin: str | None = None try: from importlib.metadata import version as pkg_version @@ -258,15 +272,22 @@ def _list_tools_impl(self, *, identity: CallerIdentity | None = None) -> List[Di ) # config_name is an optional resolution-time argument (§6.3): an agent # may target a named config; omitting it uses the tenant's default. + # Only advertise when multitenancy is enabled (ignored when off anyway). input_schema = entry["input_schema"] - props = input_schema.setdefault("properties", {}) - props.setdefault( - "config_name", - { - "type": "string", - "description": "Optional named connector configuration; omit for the default.", - }, - ) + if is_multitenancy_enabled(): + props = input_schema.setdefault("properties", {}) + # Allow null: Groq/OpenAI strict tool validation rejects + # ``config_name: null`` when type is only "string". + props.setdefault( + "config_name", + { + "type": ["string", "null"], + "description": ( + "Optional named connector configuration; omit or null " + "for the tenant default." + ), + }, + ) tools.append( { @@ -344,18 +365,24 @@ async def invoke_tool( if not self._factory.is_exposed(connector_id, "mcp"): raise ValueError(f"Connector {connector_id!r} is not available via MCP.") - # Tenant pinned from the session (SSE headers) / stdio env; config_name is a + # Tenant: HTTP session pin (§6.3) or stdio env pin (§6.4). config_name is a # resolution-time argument, never a connector input. arguments = dict(arguments or {}) config_name = resolve_config_name(arguments.pop("config_name", None)) - try: - tenant_id = resolve_tenant_id( - headers=_http_request_headers.get(), - jwt_identity=identity, - env_pin=os.getenv("NW_TENANT_ID"), - ) - except MissingTenantError as exc: - raise ValueError(str(exc)) from exc + # LLMs often fill optional schema keys with null; treat as omitted. + arguments = {k: v for k, v in arguments.items() if v is not None} + session_tenant = _session_tenant_ctx.get() + if session_tenant is not None: + tenant_id = session_tenant + else: + try: + tenant_id = resolve_tenant_id( + headers=_http_request_headers.get(), + jwt_identity=identity, + env_pin=self._stdio_env_tenant_pin, + ) + except MissingTenantError as exc: + raise ValueError(str(exc)) from exc try: connector = await self._factory.get( @@ -367,6 +394,22 @@ async def invoke_tool( except ConfigNotFoundError: raise ValueError(f"Connector {connector_id!r} is not available via MCP.") + resolved_config_name = getattr(connector, "_config_name", config_name) + if is_multitenancy_enabled(): + logger.info( + "MCP tool resolved | tool=%s | tenant_id=%s | config_name=%s", + name, + tenant_id, + resolved_config_name or "(default)", + extra={ + "tool_name": name, + "connector_id": connector_id, + "action": action, + "tenant_id": tenant_id, + "config_name": resolved_config_name or "", + }, + ) + run_args = normalize_mcp_tool_arguments(connector, action, arguments) enforce_authoritative_action(run_args, action) run_args["action"] = action @@ -561,6 +604,10 @@ async def _run_stdio_async(self) -> None: from mcp.server.stdio import stdio_server from mcp.server import NotificationOptions + # §6.4: one process, one tenant — pin from env at stdio start (not on HTTP). + raw = os.getenv("NW_TENANT_ID") + self._stdio_env_tenant_pin = raw.strip() if raw and raw.strip() else None + log_effective_mcp_auth_state() low = self._setup_lowlevel_server() @@ -615,10 +662,25 @@ async def dispatch(self, request: Request, call_next): # type: ignore[override] ) setattr(request.state, "nw_mcp_identity", identity) + # §6.3: pin tenant for this HTTP request/session context; never use + # process NW_TENANT_ID (stdio-only) so it cannot override X-Tenant-ID. + try: + session_tenant = resolve_tenant_id( + headers=request.headers, + jwt_identity=identity, + env_pin=None, + ) + except MissingTenantError as exc: + return JSONResponse( + status_code=400, + content={"detail": str(exc), "error_code": "MISSING_TENANT"}, + ) token = _streamable_http_identity_ctx.set(identity) + tenant_token = _session_tenant_ctx.set(session_tenant) try: return await call_next(request) finally: + _session_tenant_ctx.reset(tenant_token) _streamable_http_identity_ctx.reset(token) reset_upstream_passthrough_context() diff --git a/src/bindings/rest_api/app.py b/src/bindings/rest_api/app.py index 4dd41743..84e786a5 100644 --- a/src/bindings/rest_api/app.py +++ b/src/bindings/rest_api/app.py @@ -33,6 +33,7 @@ ) from node_wire_runtime.identity import ( MissingTenantError, + is_multitenancy_enabled, resolve_config_name, resolve_tenant_id, ) @@ -162,6 +163,23 @@ def _config_tenant(request: Request) -> str: raise HTTPException(status_code=400, detail=str(exc)) from exc +def _log_playground_tenant( + *, + action: str, + tenant_id: str, + connector_id: str | None = None, + config_name: str | None = None, +) -> None: + """Emit tenant context when multitenancy is on (visible in default formatters).""" + if not is_multitenancy_enabled(): + return + parts = [f"Playground action | op={action}", f"tenant_id={tenant_id}"] + if connector_id: + parts.append(f"connector={connector_id}") + parts.append(f"config_name={config_name or '(default)'}") + logger.info(" | ".join(parts)) + + def _map_config_error(exc: Exception) -> HTTPException: if isinstance(exc, MissingTenantError): return HTTPException(status_code=400, detail=str(exc)) @@ -203,7 +221,14 @@ async def config_create( factory_dep: ConnectorFactory = Depends(get_factory), ) -> JSONResponse: try: - record = factory_dep.store.create(_config_tenant(request), cid, doc) + tenant_id = _config_tenant(request) + _log_playground_tenant( + action="config.create", + tenant_id=tenant_id, + connector_id=cid, + config_name=str(doc.get("name") or "") or None, + ) + record = factory_dep.store.create(tenant_id, cid, doc) except Exception as exc: # noqa: BLE001 raise _map_config_error(exc) return JSONResponse(status_code=201, content={"name": record.name, "default": record.default}) @@ -215,8 +240,10 @@ async def config_list( request: Request, factory_dep: ConnectorFactory = Depends(get_factory), ) -> JSONResponse: + # No per-list INFO: playground dropdown refresh would flood the terminal. + tenant_id = _config_tenant(request) return JSONResponse( - status_code=200, content=factory_dep.store.list(_config_tenant(request), cid) + status_code=200, content=factory_dep.store.list(tenant_id, cid) ) @@ -227,7 +254,8 @@ async def config_get( request: Request, factory_dep: ConnectorFactory = Depends(get_factory), ) -> JSONResponse: - doc = factory_dep.store.get(_config_tenant(request), cid, name) + tenant_id = _config_tenant(request) + doc = factory_dep.store.get(tenant_id, cid, name) if doc is None: raise HTTPException(status_code=404, detail="Config not found") return JSONResponse(status_code=200, content=doc) @@ -242,7 +270,14 @@ async def config_update( factory_dep: ConnectorFactory = Depends(get_factory), ) -> JSONResponse: try: - record = factory_dep.store.update(_config_tenant(request), cid, name, doc) + tenant_id = _config_tenant(request) + _log_playground_tenant( + action="config.update", + tenant_id=tenant_id, + connector_id=cid, + config_name=name, + ) + record = factory_dep.store.update(tenant_id, cid, name, doc) except Exception as exc: # noqa: BLE001 raise _map_config_error(exc) return JSONResponse(status_code=200, content={"name": record.name, "default": record.default}) @@ -257,7 +292,14 @@ async def config_delete( factory_dep: ConnectorFactory = Depends(get_factory), ) -> JSONResponse: try: - factory_dep.store.delete(_config_tenant(request), cid, name, new_default=new_default) + tenant_id = _config_tenant(request) + _log_playground_tenant( + action="config.delete", + tenant_id=tenant_id, + connector_id=cid, + config_name=name, + ) + factory_dep.store.delete(tenant_id, cid, name, new_default=new_default) except Exception as exc: # noqa: BLE001 raise _map_config_error(exc) return JSONResponse(status_code=200, content={"status": "ok"}) @@ -271,7 +313,14 @@ async def config_set_default( factory_dep: ConnectorFactory = Depends(get_factory), ) -> JSONResponse: try: - factory_dep.store.set_default(_config_tenant(request), cid, name) + tenant_id = _config_tenant(request) + _log_playground_tenant( + action="config.set_default", + tenant_id=tenant_id, + connector_id=cid, + config_name=name, + ) + factory_dep.store.set_default(tenant_id, cid, name) except Exception as exc: # noqa: BLE001 raise _map_config_error(exc) return JSONResponse(status_code=200, content={"status": "ok"}) @@ -338,6 +387,17 @@ async def endpoint( except MissingTenantError as exc: raise HTTPException(status_code=400, detail=str(exc)) from exc + run_payload = dict(payload) + # config_name is a resolution-time argument, never a connector input. + # Suppressed when multitenancy is disabled so legacy path is always used. + config_name = resolve_config_name(run_payload.pop("config_name", None)) + _log_playground_tenant( + action=f"rest.{act}", + tenant_id=tenant_id, + connector_id=cid, + config_name=config_name, + ) + if _rate_limit_enabled(): limiter = _get_rate_limiter() identity_key = get_request_identity_key(request) @@ -353,11 +413,6 @@ async def endpoint( if not factory_dep.is_exposed(cid, "rest"): raise HTTPException(status_code=404, detail="Connector not available for REST") - run_payload = dict(payload) - # config_name is a resolution-time argument, never a connector input. - # Suppressed when multitenancy is disabled so legacy path is always used. - config_name = resolve_config_name(run_payload.pop("config_name", None)) - try: connector = await factory_dep.get( cid, tenant_id=tenant_id, config_name=config_name, action=act diff --git a/src/node_wire_runtime/base_connector.py b/src/node_wire_runtime/base_connector.py index 9fe63663..11185352 100644 --- a/src/node_wire_runtime/base_connector.py +++ b/src/node_wire_runtime/base_connector.py @@ -475,17 +475,21 @@ async def run( }, ): logger.info( - "Starting connector execution", + "Starting connector execution | connector=%s | action=%s | tenant_id=%s | config_name=%s", + self.connector_id, + self.action, + tenant_id or "(none)", + config_name or "(default)", extra={ "trace_id": trace_id, "connector_id": self.connector_id, "config_name": config_name, "action": self.action, - "principal": principal, - "tenant_id": tenant_id, + "principal": principal, "scopes": list(scopes) if scopes else [], "audit": True, "audit_event": "invocation_start", + "tenant_id": tenant_id or "", }, ) diff --git a/src/node_wire_runtime/identity.py b/src/node_wire_runtime/identity.py index 2f3e6224..23b84985 100644 --- a/src/node_wire_runtime/identity.py +++ b/src/node_wire_runtime/identity.py @@ -112,11 +112,18 @@ def resolve_tenant_id( def resolve_config_name(config_name: Optional[str]) -> Optional[str]: - """Return config_name unchanged when multitenancy is enabled, else None. + """Return a non-empty config name when multitenancy is enabled, else None. Ensures user-supplied named configs are silently ignored in single-tenant mode so the factory falls back to the YAML-bootstrapped default. + + ``None``, non-strings, and blank strings are treated as omit (tenant default). + LLMs often emit ``config_name: null`` for optional fields; that must not + fail closed as an unknown name. """ if not is_multitenancy_enabled(): return None - return config_name + if not isinstance(config_name, str): + return None + stripped = config_name.strip() + return stripped or None diff --git a/src/node_wire_runtime/mcp_client/client.py b/src/node_wire_runtime/mcp_client/client.py index 44a88aa3..da43f024 100644 --- a/src/node_wire_runtime/mcp_client/client.py +++ b/src/node_wire_runtime/mcp_client/client.py @@ -43,6 +43,7 @@ def __init__( token_manager: Optional[TokenManager] = None, http_client: Optional[httpx.AsyncClient] = None, reauthorize: Optional[Callable[[], Awaitable[OAuthTokenSet]]] = None, + extra_headers: Optional[Dict[str, str]] = None, ) -> None: self._base_url = base_url.rstrip("/") self._config = config or config_from_env(server_url=base_url) @@ -51,6 +52,7 @@ def __init__( self._initialized = False self._http = http_client self._owns_http = http_client is None + self._extra_headers = dict(extra_headers or {}) self._token_manager = token_manager or TokenManager( self._config, user_id=self._user_id, @@ -85,6 +87,7 @@ async def _auth_headers(self) -> Dict[str, str]: def _merge_headers(self, extra: Dict[str, str]) -> Dict[str, str]: out = dict(extra) + out.update(self._extra_headers) if self._session_id: out["Mcp-Session-Id"] = self._session_id return out @@ -204,6 +207,7 @@ def create_http_mcp_client( user_id: Optional[str] = None, force_oauth: bool = False, reauthorize: Optional[Callable[[], Awaitable[OAuthTokenSet]]] = None, + extra_headers: Optional[Dict[str, str]] = None, ): """ Factory: OAuth client when enabled, else legacy static-token HTTP client. @@ -216,12 +220,17 @@ def create_http_mcp_client( from agents.toolhive import ToolHiveMcpClient if legacy_static_mcp_token() and not force_oauth: - return ToolHiveMcpClient(base_url) + return ToolHiveMcpClient(base_url, extra_headers=extra_headers) if mcp_oauth_enabled() or force_oauth: - return McpOAuthClient(base_url, user_id=user_id, reauthorize=reauthorize) + return McpOAuthClient( + base_url, + user_id=user_id, + reauthorize=reauthorize, + extra_headers=extra_headers, + ) - return ToolHiveMcpClient(base_url) + return ToolHiveMcpClient(base_url, extra_headers=extra_headers) def create_http_mcp_clients_for_urls( @@ -229,5 +238,14 @@ def create_http_mcp_clients_for_urls( *, user_id: Optional[str] = None, reauthorize: Optional[Callable[[], Awaitable[OAuthTokenSet]]] = None, + extra_headers: Optional[Dict[str, str]] = None, ) -> list: - return [create_http_mcp_client(u, user_id=user_id, reauthorize=reauthorize) for u in urls] + return [ + create_http_mcp_client( + u, + user_id=user_id, + reauthorize=reauthorize, + extra_headers=extra_headers, + ) + for u in urls + ] diff --git a/tests/test_mcp_auth.py b/tests/test_mcp_auth.py index ce1a4341..bcf84478 100644 --- a/tests/test_mcp_auth.py +++ b/tests/test_mcp_auth.py @@ -154,10 +154,12 @@ async def test_mcp_authz_denies_tool_without_scope(monkeypatch: pytest.MonkeyPat async def test_mcp_execution_passes_principal_and_tenant( monkeypatch: pytest.MonkeyPatch, ) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") monkeypatch.delenv("NW_MCP_AUTH_DISABLED", raising=False) monkeypatch.delenv("NW_MCP_API_KEY", raising=False) monkeypatch.setenv("NW_MCP_JWT_SECRET", "jwt-secret") monkeypatch.delenv("NW_MCP_ACTION_SCOPE_MAP_JSON", raising=False) + monkeypatch.delenv("NW_TENANT_ID", raising=False) token = mint_test_jwt( {"sub": "service-account", "tenant_id": "tenant-42", "scopes": ["*"]}, diff --git a/tests/test_mcp_multitenant.py b/tests/test_mcp_multitenant.py new file mode 100644 index 00000000..ec050647 --- /dev/null +++ b/tests/test_mcp_multitenant.py @@ -0,0 +1,237 @@ +# +# SPDX-FileCopyrightText: 2026 AOT Technologies +# SPDX-License-Identifier: Apache-2.0 +# +"""MCP binding multitenancy: session pin, stdio env pin, config_name (§6.3 / §6.4).""" + +from __future__ import annotations + +import os + +import pytest + +from bindings.mcp_server.server import ( + McpServer, + _http_request_headers, + _session_tenant_ctx, +) +from node_wire_runtime.models import ConnectorResponse + + +@pytest.fixture(autouse=True) +def _mcp_mt_env(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv( + "NW_ALLOWED_CONNECTORS", + "http_generic,smtp,stripe,google_drive,fhir_epic,fhir_cerner", + ) + monkeypatch.setenv("NW_MCP_SCOPE_POLICY_DEFAULT", "allow") + monkeypatch.setenv("NW_MCP_AUTH_DISABLED", "true") + monkeypatch.delenv("NW_MCP_ACTION_SCOPE_MAP_JSON", raising=False) + monkeypatch.delenv("NW_MCP_API_KEY_SCOPES", raising=False) + monkeypatch.delenv("NW_TENANT_ID", raising=False) + monkeypatch.setenv("NW_RATE_LIMIT_DISABLED", "true") + + +async def _capture_invoke( + server: McpServer, + *, + tenant_for_instance: str, + arguments: dict | None = None, +) -> dict[str, object]: + """Provision tenant config, patch run(), invoke tool, return captured kwargs.""" + server._factory.store.create( + tenant_for_instance, + "http_generic", + {"name": "default", "default": True, "config": {}, "auth": {}}, + ) + connector = await server._factory.get( + "http_generic", tenant_id=tenant_for_instance, config_name="default" + ) + captured: dict[str, object] = {} + + async def fake_run(raw_input, *, principal=None, tenant_id=None, scopes=None): + captured["payload"] = dict(raw_input) + captured["tenant_id"] = tenant_id + return ConnectorResponse(success=True, data={"ok": True}, trace_id="t") + + orig = connector.run + connector.run = fake_run # type: ignore[method-assign] + try: + await server.invoke_tool( + "http_generic.request", + arguments + or { + "method": "GET", + "url": "https://example.com", + }, + ) + finally: + connector.run = orig # type: ignore[method-assign] + return captured + + +@pytest.mark.asyncio +async def test_mt_off_uses_default_tenant(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "false") + server = McpServer(connector_ids=["http_generic"]) + tools = server.list_tools() + assert all("config_name" not in (t["input_schema"].get("properties") or {}) for t in tools) + + connector = await server._factory.get("http_generic", tenant_id="__default__") + captured: dict[str, object] = {} + + async def fake_run(raw_input, *, principal=None, tenant_id=None, scopes=None): + captured["tenant_id"] = tenant_id + captured["payload"] = dict(raw_input) + return ConnectorResponse(success=True, data={"ok": True}, trace_id="t") + + orig = connector.run + connector.run = fake_run # type: ignore[method-assign] + hdr_tok = _http_request_headers.set({"x-tenant-id": "acme"}) + try: + await server.invoke_tool( + "http_generic.request", + { + "method": "GET", + "url": "https://example.com", + "config_name": "should-be-ignored", + }, + ) + finally: + connector.run = orig # type: ignore[method-assign] + _http_request_headers.reset(hdr_tok) + + assert captured["tenant_id"] == "__default__" + assert "config_name" not in captured["payload"] # type: ignore[operator] + + +@pytest.mark.asyncio +async def test_mt_on_stdio_env_pin_selects_tenant(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + server = McpServer(connector_ids=["http_generic"]) + server._stdio_env_tenant_pin = "acme" + + captured = await _capture_invoke(server, tenant_for_instance="acme") + assert captured["tenant_id"] == "acme" + assert "config_name" not in captured["payload"] # type: ignore[operator] + + +@pytest.mark.asyncio +async def test_mt_on_session_pin_ignores_process_nw_tenant_id( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + monkeypatch.setenv("NW_TENANT_ID", "from-env-must-not-win") + server = McpServer(connector_ids=["http_generic"]) + server._stdio_env_tenant_pin = "from-env-must-not-win" + + sess = _session_tenant_ctx.set("acme") + try: + captured = await _capture_invoke(server, tenant_for_instance="acme") + finally: + _session_tenant_ctx.reset(sess) + + assert captured["tenant_id"] == "acme" + + +@pytest.mark.asyncio +async def test_mt_on_missing_tenant_raises(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + server = McpServer(connector_ids=["http_generic"]) + server._stdio_env_tenant_pin = None + + with pytest.raises(ValueError, match="X-Tenant-ID is required"): + await server.invoke_tool( + "http_generic.request", + {"method": "GET", "url": "https://example.com"}, + ) + + +@pytest.mark.asyncio +async def test_mt_on_config_name_stripped_and_unknown_fail_closed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + server = McpServer(connector_ids=["http_generic"]) + server._stdio_env_tenant_pin = "acme" + server._factory.store.create( + "acme", + "http_generic", + {"name": "primary", "default": True, "config": {}, "auth": {}}, + ) + + connector = await server._factory.get( + "http_generic", tenant_id="acme", config_name="primary" + ) + captured: dict[str, object] = {} + + async def fake_run(raw_input, *, principal=None, tenant_id=None, scopes=None): + captured["payload"] = dict(raw_input) + return ConnectorResponse(success=True, data={"ok": True}, trace_id="t") + + connector.run = fake_run # type: ignore[method-assign] + await server.invoke_tool( + "http_generic.request", + { + "method": "GET", + "url": "https://example.com", + "config_name": "primary", + }, + ) + assert "config_name" not in captured["payload"] # type: ignore[operator] + + with pytest.raises(ValueError, match="not available via MCP"): + await server.invoke_tool( + "http_generic.request", + { + "method": "GET", + "url": "https://example.com", + "config_name": "does-not-exist", + }, + ) + + +@pytest.mark.asyncio +async def test_mt_on_config_name_in_tool_schema(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + server = McpServer(connector_ids=["http_generic"]) + tools = server.list_tools() + cfg_schemas = [ + (t["input_schema"].get("properties") or {}).get("config_name") + for t in tools + if "config_name" in (t["input_schema"].get("properties") or {}) + ] + assert cfg_schemas + assert cfg_schemas[0]["type"] == ["string", "null"] + + +@pytest.mark.asyncio +async def test_mt_on_null_config_name_uses_tenant_default( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """LLMs emit config_name: null; treat as omit (tenant default).""" + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + server = McpServer(connector_ids=["http_generic"]) + server._stdio_env_tenant_pin = "acme" + captured = await _capture_invoke( + server, + tenant_for_instance="acme", + arguments={ + "method": "GET", + "url": "https://example.com", + "config_name": None, + "headers": None, + }, + ) + assert captured["tenant_id"] == "acme" + assert "config_name" not in captured["payload"] # type: ignore[operator] + assert "headers" not in captured["payload"] # type: ignore[operator] + + +def test_stdio_pin_assignment_from_nw_tenant_id(monkeypatch: pytest.MonkeyPatch) -> None: + """Same assignment used at the start of _run_stdio_async.""" + monkeypatch.setenv("NW_TENANT_ID", " acme ") + server = McpServer(connector_ids=["http_generic"]) + raw = os.getenv("NW_TENANT_ID") + server._stdio_env_tenant_pin = raw.strip() if raw and raw.strip() else None + assert server._stdio_env_tenant_pin == "acme" diff --git a/tests/test_multitenancy.py b/tests/test_multitenancy.py index e2dbdf1e..0c93dea5 100644 --- a/tests/test_multitenancy.py +++ b/tests/test_multitenancy.py @@ -463,3 +463,11 @@ def test_resolve_config_name_enabled_passthrough(monkeypatch: pytest.MonkeyPatch monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") assert resolve_config_name("my-config") == "my-config" assert resolve_config_name(None) is None + assert resolve_config_name("") is None + assert resolve_config_name(" ") is None + + +def test_resolve_config_name_enabled_rejects_non_string(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + # JSON null and LLM quirks must map to omit (tenant default), not fail closed. + assert resolve_config_name(None) is None # type: ignore[arg-type] diff --git a/tests/test_playground_agent_tenant.py b/tests/test_playground_agent_tenant.py new file mode 100644 index 00000000..6a78ed38 --- /dev/null +++ b/tests/test_playground_agent_tenant.py @@ -0,0 +1,113 @@ +# +# SPDX-FileCopyrightText: 2026 AOT Technologies +# SPDX-License-Identifier: Apache-2.0 +# +"""Playground agent chat forwards tenant to MCP clients.""" + +from __future__ import annotations + +import json +from unittest.mock import MagicMock, patch + +import httpx +import pytest +from fastapi.testclient import TestClient + +from agents.toolhive import InProcessMcpClient, ToolHiveMcpClient + + +@pytest.fixture(autouse=True) +def _agent_tenant_env(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv( + "NW_ALLOWED_CONNECTORS", + "http_generic,smtp,stripe,google_drive,fhir_epic,fhir_cerner", + ) + monkeypatch.setenv("NW_REST_AUTH_DISABLED", "true") + monkeypatch.setenv("NW_MCP_SCOPE_POLICY_DEFAULT", "allow") + monkeypatch.setenv("NW_RATE_LIMIT_DISABLED", "true") + monkeypatch.delenv("TOOLHIVE_MCP_URL", raising=False) + monkeypatch.delenv("TOOLHIVE_MCP_URLS", raising=False) + monkeypatch.setenv("NW_MCP_TRANSPORT", "stdio") + + +@pytest.mark.asyncio +async def test_toolhive_client_sends_tenant_header() -> None: + seen: list[dict] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append({k.lower(): v for k, v in request.headers.items()}) + body = json.loads(request.content.decode()) + method = body.get("method") + req_id = body.get("id") + if method == "initialize": + return httpx.Response( + 200, + json={"jsonrpc": "2.0", "id": req_id, "result": {"protocolVersion": "2024-11-05"}}, + headers={"Mcp-Session-Id": "s1"}, + ) + if method == "notifications/initialized": + return httpx.Response(200, json={}) + if method == "tools/list": + return httpx.Response( + 200, + json={"jsonrpc": "2.0", "id": req_id, "result": {"tools": []}}, + ) + return httpx.Response(404) + + transport = httpx.MockTransport(handler) + _Real = httpx.AsyncClient + + def make_client(**kwargs: object) -> httpx.AsyncClient: + return _Real(transport=transport, timeout=60.0) + + with patch("httpx.AsyncClient", side_effect=make_client): + client = ToolHiveMcpClient( + "http://127.0.0.1:9/mcp", + extra_headers={"x-tenant-id": "acme"}, + ) + await client.list_tools() + + assert any(h.get("x-tenant-id") == "acme" for h in seen) + + +@pytest.mark.asyncio +async def test_inprocess_mcp_client_pins_tenant_and_config_name() -> None: + server = MagicMock() + server.list_tools.return_value = [{"name": "http_generic.request", "input_schema": {}}] + + async def fake_invoke(name, arguments, identity=None): + return {"ok": True, "args": arguments} + + server.invoke_tool = fake_invoke + + client = InProcessMcpClient(server, tenant_id="acme", config_name="primary") + async with client: + assert server._stdio_env_tenant_pin == "acme" + tools = await client.list_tools() + assert tools[0]["name"] == "http_generic.request" + out = await client.call_tool("http_generic.request", {"url": "https://example.com"}) + data = json.loads(out) + assert data["args"]["config_name"] == "primary" + assert data["args"]["url"] == "https://example.com" + + # Null from LLM must not block host-injected config_name. + out2 = await client.call_tool( + "http_generic.request", + {"url": "https://example.com", "config_name": None, "query": None}, + ) + data2 = json.loads(out2) + assert data2["args"]["config_name"] == "primary" + assert "query" not in data2["args"] + + +def test_agent_chat_requires_tenant_when_mt_on(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + from bindings.rest_api.app import app + + client = TestClient(app) + resp = client.post( + "/scenarios/agent-chat", + json={"message": "hello", "history": []}, + ) + assert resp.status_code == 400 + assert "X-Tenant-ID" in resp.json().get("detail", "") diff --git a/tests/test_toolhive_agent.py b/tests/test_toolhive_agent.py index 7321146c..118bcb98 100644 --- a/tests/test_toolhive_agent.py +++ b/tests/test_toolhive_agent.py @@ -25,16 +25,40 @@ LLMProviderFactory, LLMResponse, ToolCall, + openai_compatible_tool_parameters, ) from agents.toolhive import ( ToolHiveAgent, ToolHiveMcpClient, _is_tool_failure, + omit_null_tool_args, resolve_max_tool_failures, truncate_tool_result_for_llm, ) +def test_omit_null_tool_args() -> None: + assert omit_null_tool_args({"a": 1, "b": None, "c": ""}) == {"a": 1, "c": ""} + + +def test_openai_compatible_tool_parameters_allows_null_on_optional() -> None: + schema = { + "type": "object", + "properties": { + "required_str": {"type": "string"}, + "optional_str": {"type": "string"}, + "already": {"type": ["string", "null"]}, + }, + "required": ["required_str"], + } + out = openai_compatible_tool_parameters(schema) + assert out["properties"]["required_str"]["type"] == "string" + assert out["properties"]["optional_str"]["type"] == ["string", "null"] + assert out["properties"]["already"]["type"] == ["string", "null"] + # Input schema must not be mutated. + assert schema["properties"]["optional_str"]["type"] == "string" + + def test_truncate_tool_result_for_llm_respects_limit(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("TOOLHIVE_MAX_TOOL_RESULT_CHARS", "20") long = "x" * 100 From 0c9c5b622c6e9f20626ac2331296f9eaacfdfaee Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 29 Jul 2026 13:02:27 +0530 Subject: [PATCH 04/13] lint and ruff format done Signed-off-by: gokul-aot --- playground/scenarios.py | 29 +++++++------------------- src/bindings/factory.py | 16 +++++++------- src/bindings/rest_api/app.py | 4 +--- src/node_wire_runtime/config_store.py | 30 ++++++++------------------- tests/conftest.py | 3 +++ tests/test_factory_and_rest.py | 2 ++ 6 files changed, 30 insertions(+), 54 deletions(-) diff --git a/playground/scenarios.py b/playground/scenarios.py index 5973e74f..edf4b20a 100644 --- a/playground/scenarios.py +++ b/playground/scenarios.py @@ -410,13 +410,9 @@ async def get_cerner_connector(request: Request, config_name: Optional[str] = No return connector -async def get_google_drive_connector( - request: Request, config_name: Optional[str] = None -) -> Any: +async def get_google_drive_connector(request: Request, config_name: Optional[str] = None) -> Any: """``config_name`` query param is optional; GDrive body may also carry it.""" - return await resolve_connector( - request, "google_drive", config_name=config_name - ) + return await resolve_connector(request, "google_drive", config_name=config_name) async def get_slack_connector(request: Request, config_name: Optional[str] = None): @@ -1995,15 +1991,10 @@ async def agent_chat(request: Request, payload: AgentChatInput) -> AgentChatResp logger.info("Agent Chat | trying ToolHive proxy URL(s): %s", ",".join(urls)) try: if len(urls) == 1: - mcp_client = create_http_mcp_client( - urls[0], extra_headers=mcp_headers - ) + mcp_client = create_http_mcp_client(urls[0], extra_headers=mcp_headers) else: mcp_client = MultiMcpClient( - [ - create_http_mcp_client(u, extra_headers=mcp_headers) - for u in urls - ] + [create_http_mcp_client(u, extra_headers=mcp_headers) for u in urls] ) agent = ToolHiveAgent( mcp_client, @@ -2161,23 +2152,17 @@ async def stream_events(): urls = resolve_mcp_urls() if transport == "streamable-http" else [] # Default true: playground should work without a separate MCP on :8081. fallback_to_local = ( - os.environ.get("PLAYGROUND_AGENT_PROXY_FALLBACK_TO_STDIO", "true").lower() - == "true" + os.environ.get("PLAYGROUND_AGENT_PROXY_FALLBACK_TO_STDIO", "true").lower() == "true" ) if urls: proxy_ok = False try: if len(urls) == 1: - mcp_client = create_http_mcp_client( - urls[0], extra_headers=mcp_headers - ) + mcp_client = create_http_mcp_client(urls[0], extra_headers=mcp_headers) else: mcp_client = MultiMcpClient( - [ - create_http_mcp_client(u, extra_headers=mcp_headers) - for u in urls - ] + [create_http_mcp_client(u, extra_headers=mcp_headers) for u in urls] ) agent = ToolHiveAgent( mcp_client, diff --git a/src/bindings/factory.py b/src/bindings/factory.py index 649862a9..791e8299 100644 --- a/src/bindings/factory.py +++ b/src/bindings/factory.py @@ -19,6 +19,7 @@ from node_wire_runtime import BaseConnector, SecretProvider, get_connector_registry from node_wire_runtime.config_store import ( DEFAULT_TENANT, + ConfigNotFoundError, ConfigRecord, ConnectorConfigStore, ) @@ -188,9 +189,7 @@ def __init__(self, config_path: str | Path | None = None) -> None: self._store.attach_factory(self) # Scope policy takes precedence when configured; otherwise fall back to the # config-existence hook (defense in depth for configured = entitled). - self._policy_hook: PolicyHook | None = _build_policy_hook() or TenantConfigHook( - self._store - ) + self._policy_hook: PolicyHook | None = _build_policy_hook() or TenantConfigHook(self._store) # YAML-derived metadata (enabled, exposed_via, raw) for enumeration, # protocol gating, and the MCP upstream-passthrough check. self._configs: Dict[str, ConnectorConfig] = {} @@ -224,7 +223,9 @@ def load(self) -> None: raw = _resolve_env_vars(raw) connectors_cfg: Dict[str, Any] = raw.get("connectors", {}) - bootstrap_enabled = os.environ.get("NW_CONFIG_BOOTSTRAP_YAML", "true").strip().lower() not in ( + bootstrap_enabled = os.environ.get( + "NW_CONFIG_BOOTSTRAP_YAML", "true" + ).strip().lower() not in ( "0", "false", "no", @@ -488,9 +489,7 @@ def _evict_if_needed(self) -> None: _, old = self._instances.popitem(last=False) self._schedule_aclose(old) - def invalidate_configs( - self, tenant_id: str, connector_id: str, names: List[str] - ) -> None: + def invalidate_configs(self, tenant_id: str, connector_id: str, names: List[str]) -> None: """Drop cached instances for the given config names and schedule teardown. Called synchronously by the store on every mutating write; may run on a @@ -519,7 +518,8 @@ async def _safe_aclose() -> None: if loop is not None: loop.create_task(_safe_aclose()) elif self._loop is not None and self._loop.is_running(): - self._loop.call_soon_threadsafe(lambda: self._loop.create_task(_safe_aclose())) + running_loop = self._loop + running_loop.call_soon_threadsafe(lambda: running_loop.create_task(_safe_aclose())) # else: no running loop known; the instance is dropped without aclose. def _default_instance(self, connector_id: str) -> Optional[BaseConnector]: diff --git a/src/bindings/rest_api/app.py b/src/bindings/rest_api/app.py index 84e786a5..938d7ee9 100644 --- a/src/bindings/rest_api/app.py +++ b/src/bindings/rest_api/app.py @@ -242,9 +242,7 @@ async def config_list( ) -> JSONResponse: # No per-list INFO: playground dropdown refresh would flood the terminal. tenant_id = _config_tenant(request) - return JSONResponse( - status_code=200, content=factory_dep.store.list(tenant_id, cid) - ) + return JSONResponse(status_code=200, content=factory_dep.store.list(tenant_id, cid)) @app.get("/v1/connectors/{cid}/configs/{name}", tags=["config"]) diff --git a/src/node_wire_runtime/config_store.py b/src/node_wire_runtime/config_store.py index 4a07aeb6..91036865 100644 --- a/src/node_wire_runtime/config_store.py +++ b/src/node_wire_runtime/config_store.py @@ -263,9 +263,7 @@ def set_default(self, tenant_id: str, connector_id: str, name: str) -> None: with self._lock: scope = self._scope(tenant_id, connector_id) if scope is None or name not in scope: - raise ConfigNotFoundError( - f"no config {name!r} for {tenant_id!r}/{connector_id!r}" - ) + raise ConfigNotFoundError(f"no config {name!r} for {tenant_id!r}/{connector_id!r}") changed: List[str] = [] for other_name, other in scope.items(): should_be = other_name == name @@ -293,9 +291,7 @@ def delete( with self._lock: scope = self._scope(tenant_id, connector_id) if scope is None or name not in scope: - raise ConfigNotFoundError( - f"no config {name!r} for {tenant_id!r}/{connector_id!r}" - ) + raise ConfigNotFoundError(f"no config {name!r} for {tenant_id!r}/{connector_id!r}") record = scope[name] is_last = len(scope) == 1 @@ -333,9 +329,7 @@ def get(self, tenant_id: str, connector_id: str, name: str) -> Optional[Dict[str return None return self._public_view(scope[name]) - def list( - self, tenant_id: str, connector_id: Optional[str] = None - ) -> List[Dict[str, Any]]: + def list(self, tenant_id: str, connector_id: Optional[str] = None) -> List[Dict[str, Any]]: """Redacted list. ``connector_id=None`` lists all configs for the tenant.""" with self._lock: tenant_map = self._data.get(tenant_id) @@ -359,9 +353,7 @@ def has_config(self, tenant_id: str, connector_id: str) -> bool: # ---- internal (unredacted) ----------------------------------------- - def resolve( - self, tenant_id: str, connector_id: str, name: Optional[str] - ) -> ConfigRecord: + def resolve(self, tenant_id: str, connector_id: str, name: Optional[str]) -> ConfigRecord: """INTERNAL (factory use). ``name=None`` resolves the default. Missing scope or name raises :class:`ConfigNotFoundError`. Returns the UNREDACTED record.""" with self._lock: @@ -376,18 +368,16 @@ def resolve( return record # Defensive: a non-empty scope always has a default by construction. return next(iter(scope.values())) - record = scope.get(name) - if record is None: + found = scope.get(name) + if found is None: raise ConfigNotFoundError( f"no config for tenant {tenant_id!r} / connector {connector_id!r}" ) - return record + return found # ---- helpers -------------------------------------------------------- - def _scope( - self, tenant_id: str, connector_id: str - ) -> Optional[Dict[str, ConfigRecord]]: + def _scope(self, tenant_id: str, connector_id: str) -> Optional[Dict[str, ConfigRecord]]: tenant_map = self._data.get(tenant_id) if tenant_map is None: return None @@ -396,9 +386,7 @@ def _scope( def _require(self, tenant_id: str, connector_id: str, name: str) -> ConfigRecord: scope = self._scope(tenant_id, connector_id) if scope is None or name not in scope: - raise ConfigNotFoundError( - f"no config {name!r} for {tenant_id!r}/{connector_id!r}" - ) + raise ConfigNotFoundError(f"no config {name!r} for {tenant_id!r}/{connector_id!r}") return scope[name] @staticmethod diff --git a/tests/conftest.py b/tests/conftest.py index 9d97f679..f087ec6d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -69,6 +69,9 @@ def _preload_connector_logic_modules() -> None: def _rest_auth_disabled_for_tests(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("NW_REST_AUTH_DISABLED", "true") monkeypatch.setenv("NW_MCP_AUTH_DISABLED", "true") + # Isolate from developer .env: legacy REST tests expect single-tenant unless + # a test explicitly enables multitenancy. + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "false") monkeypatch.delenv("GOOGLE_DRIVE_AUTH_PROVIDER", raising=False) monkeypatch.setenv("NW_MCP_SCOPE_POLICY_DEFAULT", "allow") monkeypatch.setenv("NW_JWT_AUDIENCE", "node-wire-test") diff --git a/tests/test_factory_and_rest.py b/tests/test_factory_and_rest.py index f0358355..c3d9e2b4 100644 --- a/tests/test_factory_and_rest.py +++ b/tests/test_factory_and_rest.py @@ -159,6 +159,8 @@ def test_rest_post_propagates_api_key_identity_to_connector_run( def test_rest_post_propagates_jwt_claims_to_connector_run(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("NW_REST_AUTH_DISABLED", raising=False) monkeypatch.delenv("NW_REST_API_KEY", raising=False) + # JWT tenant claim is only applied when multitenancy is enabled. + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") secret = "rest-jwt-test-secret-at-least-32bytes!!" monkeypatch.setenv("NW_REST_JWT_SECRET", secret) From 8e956e8e7d5441feb55481f49dab8e0faee1015b Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 29 Jul 2026 13:39:10 +0530 Subject: [PATCH 05/13] reviewed issues resolved Signed-off-by: gokul-aot --- tests/test_multitenancy.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_multitenancy.py b/tests/test_multitenancy.py index 0c93dea5..f22dfd2c 100644 --- a/tests/test_multitenancy.py +++ b/tests/test_multitenancy.py @@ -36,8 +36,6 @@ TenantSecretNotFoundError, TenantSecretProvider, ) -import node_wire_runtime.identity as identity_mod - # --------------------------------------------------------------------------- # # Config store lifecycle @@ -163,7 +161,7 @@ def test_missing_header_resolves_default(monkeypatch: pytest.MonkeyPatch): def test_header_override_env(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(identity_mod, "TENANT_HEADER", "x-org-id") + monkeypatch.setattr("node_wire_runtime.identity.TENANT_HEADER", "x-org-id") assert tenant_from_headers({"X-Org-ID": "globex"}) == "globex" From ee7300f983b4d27eda86d0c52217f5029ba40b77 Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 29 Jul 2026 14:14:22 +0530 Subject: [PATCH 06/13] codeQL listed issues Signed-off-by: gokul-aot --- playground/scenarios.py | 11 +++++--- src/agents/llm_factory.py | 32 +++------------------- src/agents/providers/groq_provider.py | 3 ++- src/agents/providers/openai_provider.py | 3 ++- src/agents/schema_utils.py | 36 +++++++++++++++++++++++++ src/bindings/rest_api/app.py | 13 ++++++--- src/node_wire_runtime/base_connector.py | 1 + 7 files changed, 63 insertions(+), 36 deletions(-) create mode 100644 src/agents/schema_utils.py diff --git a/playground/scenarios.py b/playground/scenarios.py index edf4b20a..ab805f43 100644 --- a/playground/scenarios.py +++ b/playground/scenarios.py @@ -80,6 +80,11 @@ logger = logging.getLogger("playground.scenarios") + + +def _sanitize_log(value: str) -> str: + """Strip newlines/carriage-returns from user-supplied log values (log injection).""" + return value.replace("\n", " ").replace("\r", " ") if value else value router = APIRouter(prefix="/scenarios", tags=["scenarios"]) @@ -370,9 +375,9 @@ async def resolve_connector( if is_multitenancy_enabled(): logger.info( "Playground action | connector=%s | tenant_id=%s | config_name=%s", - connector_id, - tenant_id, - name or "(default)", + _sanitize_log(connector_id), + _sanitize_log(tenant_id), + _sanitize_log(name or "(default)"), ) try: return await factory.get( diff --git a/src/agents/llm_factory.py b/src/agents/llm_factory.py index 53191fed..ff41470d 100644 --- a/src/agents/llm_factory.py +++ b/src/agents/llm_factory.py @@ -28,6 +28,10 @@ from dataclasses import dataclass, field from typing import Any, Dict, List, Optional, Type +# Re-export so existing callers can still do: +# from agents.llm_factory import openai_compatible_tool_parameters +from agents.schema_utils import openai_compatible_tool_parameters as openai_compatible_tool_parameters # noqa: F401 + # --------------------------------------------------------------------------- # Data models (provider-agnostic) @@ -67,34 +71,6 @@ def wants_tool_call(self) -> bool: return bool(self.tool_calls) -def openai_compatible_tool_parameters(input_schema: Optional[Dict[str, Any]]) -> Dict[str, Any]: - """Copy a JSON Schema so optional properties also accept ``null``. - - Providers such as Groq validate model tool calls against the schema and reject - ``"field": null`` when the type is only ``"string"``. Models often emit null - for unused optional keys instead of omitting them. - """ - import copy - - schema = copy.deepcopy(input_schema) if input_schema else {"type": "object", "properties": {}} - props = schema.get("properties") - if not isinstance(props, dict): - return schema - required = set(schema.get("required") or []) - for key, prop in props.items(): - if key in required or not isinstance(prop, dict): - continue - t = prop.get("type") - if t is None: - continue - if isinstance(t, list): - if "null" not in t: - prop["type"] = [*t, "null"] - elif t != "null": - prop["type"] = [t, "null"] - return schema - - # --------------------------------------------------------------------------- # Abstract base # --------------------------------------------------------------------------- diff --git a/src/agents/providers/groq_provider.py b/src/agents/providers/groq_provider.py index 1b55e769..3d084407 100644 --- a/src/agents/providers/groq_provider.py +++ b/src/agents/providers/groq_provider.py @@ -18,7 +18,8 @@ import logging from typing import Any, Dict, List, cast -from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall, openai_compatible_tool_parameters +from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall +from agents.schema_utils import openai_compatible_tool_parameters logger = logging.getLogger("agents.providers.groq") diff --git a/src/agents/providers/openai_provider.py b/src/agents/providers/openai_provider.py index e7654569..95797eec 100644 --- a/src/agents/providers/openai_provider.py +++ b/src/agents/providers/openai_provider.py @@ -17,7 +17,8 @@ import logging from typing import Any, Dict, List, cast -from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall, openai_compatible_tool_parameters +from agents.llm_factory import BaseLLMProvider, LLMMessage, LLMResponse, ToolCall +from agents.schema_utils import openai_compatible_tool_parameters logger = logging.getLogger("agents.providers.openai") diff --git a/src/agents/schema_utils.py b/src/agents/schema_utils.py new file mode 100644 index 00000000..9ebf77e3 --- /dev/null +++ b/src/agents/schema_utils.py @@ -0,0 +1,36 @@ +# +# SPDX-FileCopyrightText: 2026 AOT Technologies +# SPDX-License-Identifier: Apache-2.0 +# +"""Shared JSON-Schema helpers for LLM provider tool definitions.""" + +from __future__ import annotations + +import copy +from typing import Any, Dict, Optional + + +def openai_compatible_tool_parameters(input_schema: Optional[Dict[str, Any]]) -> Dict[str, Any]: + """Copy a JSON Schema so optional properties also accept ``null``. + + Providers such as Groq validate model tool calls against the schema and reject + ``"field": null`` when the type is only ``"string"``. Models often emit null + for unused optional keys instead of omitting them. + """ + schema = copy.deepcopy(input_schema) if input_schema else {"type": "object", "properties": {}} + props = schema.get("properties") + if not isinstance(props, dict): + return schema + required = set(schema.get("required") or []) + for key, prop in props.items(): + if key in required or not isinstance(prop, dict): + continue + t = prop.get("type") + if t is None: + continue + if isinstance(t, list): + if "null" not in t: + prop["type"] = [*t, "null"] + elif t != "null": + prop["type"] = [t, "null"] + return schema diff --git a/src/bindings/rest_api/app.py b/src/bindings/rest_api/app.py index 938d7ee9..ddee2d93 100644 --- a/src/bindings/rest_api/app.py +++ b/src/bindings/rest_api/app.py @@ -163,6 +163,11 @@ def _config_tenant(request: Request) -> str: raise HTTPException(status_code=400, detail=str(exc)) from exc +def _sanitize_log(value: str) -> str: + """Strip newlines/carriage-returns from user-supplied log values (log injection).""" + return value.replace("\n", " ").replace("\r", " ") if value else value + + def _log_playground_tenant( *, action: str, @@ -173,10 +178,12 @@ def _log_playground_tenant( """Emit tenant context when multitenancy is on (visible in default formatters).""" if not is_multitenancy_enabled(): return - parts = [f"Playground action | op={action}", f"tenant_id={tenant_id}"] + safe_tenant = _sanitize_log(tenant_id) + safe_config = _sanitize_log(config_name or "(default)") + parts = [f"Playground action | op={action}", f"tenant_id={safe_tenant}"] if connector_id: - parts.append(f"connector={connector_id}") - parts.append(f"config_name={config_name or '(default)'}") + parts.append(f"connector={_sanitize_log(connector_id)}") + parts.append(f"config_name={safe_config}") logger.info(" | ".join(parts)) diff --git a/src/node_wire_runtime/base_connector.py b/src/node_wire_runtime/base_connector.py index 11185352..47e879a2 100644 --- a/src/node_wire_runtime/base_connector.py +++ b/src/node_wire_runtime/base_connector.py @@ -573,6 +573,7 @@ async def run( async def _do_execute(*, trace_id: str) -> Any: return await self.internal_execute(input_model, trace_id=trace_id) + _start = time.monotonic() output_model = await _do_execute(trace_id=trace_id) logger.info( From aca26bc9d1538c705d46c210b44bb14a8fbd9e36 Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 29 Jul 2026 15:04:55 +0530 Subject: [PATCH 07/13] pytest issues resolved Signed-off-by: gokul-aot --- playground/scenarios.py | 2 ++ src/agents/llm_factory.py | 4 +++- src/agents/toolhive.py | 12 +++++++++++- src/bindings/rest_api/app.py | 12 ++++++++++++ src/node_wire_runtime/base_connector.py | 4 ++-- tests/test_mcp_multitenant.py | 4 +--- tests/test_multitenancy.py | 19 +++++++------------ 7 files changed, 38 insertions(+), 19 deletions(-) diff --git a/playground/scenarios.py b/playground/scenarios.py index ab805f43..efb8b53e 100644 --- a/playground/scenarios.py +++ b/playground/scenarios.py @@ -85,6 +85,8 @@ def _sanitize_log(value: str) -> str: """Strip newlines/carriage-returns from user-supplied log values (log injection).""" return value.replace("\n", " ").replace("\r", " ") if value else value + + router = APIRouter(prefix="/scenarios", tags=["scenarios"]) diff --git a/src/agents/llm_factory.py b/src/agents/llm_factory.py index ff41470d..aee3404a 100644 --- a/src/agents/llm_factory.py +++ b/src/agents/llm_factory.py @@ -30,7 +30,9 @@ # Re-export so existing callers can still do: # from agents.llm_factory import openai_compatible_tool_parameters -from agents.schema_utils import openai_compatible_tool_parameters as openai_compatible_tool_parameters # noqa: F401 +from agents.schema_utils import ( + openai_compatible_tool_parameters as openai_compatible_tool_parameters, +) # noqa: F401 # --------------------------------------------------------------------------- diff --git a/src/agents/toolhive.py b/src/agents/toolhive.py index 6a511206..b2b33cdc 100644 --- a/src/agents/toolhive.py +++ b/src/agents/toolhive.py @@ -48,7 +48,17 @@ import uuid from contextlib import AsyncExitStack from dataclasses import dataclass, field -from typing import Any, AsyncIterator, Dict, List, Mapping, Optional, Protocol, Union, runtime_checkable +from typing import ( + Any, + AsyncIterator, + Dict, + List, + Mapping, + Optional, + Protocol, + Union, + runtime_checkable, +) import re from dotenv import load_dotenv diff --git a/src/bindings/rest_api/app.py b/src/bindings/rest_api/app.py index ddee2d93..d428ac93 100644 --- a/src/bindings/rest_api/app.py +++ b/src/bindings/rest_api/app.py @@ -148,6 +148,18 @@ async def health() -> Dict[str, str]: return {"status": "ok"} +@app.get("/ready", tags=["system"]) +async def ready() -> Dict[str, str]: + try: + factory = get_factory() + connectors = factory.list_for_protocol("rest") + except Exception: + raise HTTPException(status_code=503, detail="factory not ready") + if not connectors: + raise HTTPException(status_code=503, detail="no connectors registered") + return {"status": "ready"} + + # --- Runtime connector config API (thin wrappers over ConnectorConfigStore) --- # Tenant scope comes from the tenant header (never the path). The embedding # application authenticates callers before these endpoints are reachable diff --git a/src/node_wire_runtime/base_connector.py b/src/node_wire_runtime/base_connector.py index 47e879a2..9b1f7bdf 100644 --- a/src/node_wire_runtime/base_connector.py +++ b/src/node_wire_runtime/base_connector.py @@ -485,7 +485,7 @@ async def run( "connector_id": self.connector_id, "config_name": config_name, "action": self.action, - "principal": principal, + "principal": principal, "scopes": list(scopes) if scopes else [], "audit": True, "audit_event": "invocation_start", @@ -494,6 +494,7 @@ async def run( ) _response: Optional[ConnectorResponse] = None + _start = time.monotonic() token = _caller_execution_ctx.set((principal, tenant_id, scopes)) try: try: @@ -573,7 +574,6 @@ async def run( async def _do_execute(*, trace_id: str) -> Any: return await self.internal_execute(input_model, trace_id=trace_id) - _start = time.monotonic() output_model = await _do_execute(trace_id=trace_id) logger.info( diff --git a/tests/test_mcp_multitenant.py b/tests/test_mcp_multitenant.py index ec050647..5dde32dc 100644 --- a/tests/test_mcp_multitenant.py +++ b/tests/test_mcp_multitenant.py @@ -160,9 +160,7 @@ async def test_mt_on_config_name_stripped_and_unknown_fail_closed( {"name": "primary", "default": True, "config": {}, "auth": {}}, ) - connector = await server._factory.get( - "http_generic", tenant_id="acme", config_name="primary" - ) + connector = await server._factory.get("http_generic", tenant_id="acme", config_name="primary") captured: dict[str, object] = {} async def fake_run(raw_input, *, principal=None, tenant_id=None, scopes=None): diff --git a/tests/test_multitenancy.py b/tests/test_multitenancy.py index f22dfd2c..598316ff 100644 --- a/tests/test_multitenancy.py +++ b/tests/test_multitenancy.py @@ -184,7 +184,9 @@ def test_env_pin_wins_over_everything(monkeypatch: pytest.MonkeyPatch): ident = MagicMock() ident.tenant_id = "t-1" assert ( - resolve_tenant_id(headers={"X-Tenant-ID": "acme"}, jwt_identity=ident, env_pin="stdio-tenant") + resolve_tenant_id( + headers={"X-Tenant-ID": "acme"}, jwt_identity=ident, env_pin="stdio-tenant" + ) == "stdio-tenant" ) @@ -215,9 +217,7 @@ def test_tenant_secret_provider_is_strict(monkeypatch: pytest.MonkeyPatch): def _bare_factory(monkeypatch: pytest.MonkeyPatch) -> ConnectorFactory: """Factory whose instantiation returns a fresh stub per call (no real connector).""" factory = ConnectorFactory() - monkeypatch.setattr( - factory, "_instantiate", lambda record: MagicMock(spec=BaseConnector) - ) + monkeypatch.setattr(factory, "_instantiate", lambda record: MagicMock(spec=BaseConnector)) return factory @@ -307,9 +307,7 @@ def test_rest_fail_closed_returns_indistinguishable_403(monkeypatch: pytest.Monk try: client = TestClient(app) unknown_scope = client.post("/connectors/http_generic/request", json={}) - unknown_name = client.post( - "/connectors/http_generic/request", json={"config_name": "nope"} - ) + unknown_name = client.post("/connectors/http_generic/request", json={"config_name": "nope"}) finally: app.dependency_overrides.clear() assert unknown_scope.status_code == 403 @@ -366,9 +364,7 @@ async def test_direct_integration_store_factory_run(monkeypatch: pytest.MonkeyPa factory.store.init( { "acme": { - "test_echo": [ - {"name": "primary", "default": True, "config": {"channel": "#eng"}} - ] + "test_echo": [{"name": "primary", "default": True, "config": {"channel": "#eng"}}] } } ) @@ -445,8 +441,7 @@ def test_resolve_tenant_id_enabled_missing_raises(monkeypatch: pytest.MonkeyPatc def test_resolve_tenant_id_enabled_env_pin_wins(monkeypatch: pytest.MonkeyPatch): monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") assert ( - resolve_tenant_id(headers={"X-Tenant-ID": "acme"}, env_pin="stdio-tenant") - == "stdio-tenant" + resolve_tenant_id(headers={"X-Tenant-ID": "acme"}, env_pin="stdio-tenant") == "stdio-tenant" ) From 128dcbedd75b6c7dc34a762645fb51dd04f31ec0 Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 29 Jul 2026 15:15:36 +0530 Subject: [PATCH 08/13] pytest issues resolved Signed-off-by: gokul-aot --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 5c297603..bbe8674e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,7 +125,7 @@ override-dependencies = [ ] [tool.pytest.ini_options] -pythonpath = ["src", "."] +pythonpath = ["src", ".", "src/bindings/grpc_server"] asyncio_mode = "auto" asyncio_default_fixture_loop_scope = "function" addopts = [ From 83d0be8b1d0c3a095fef7c2ea85dd6743a83fc6f Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 29 Jul 2026 15:27:16 +0530 Subject: [PATCH 09/13] pytest issues resolved Signed-off-by: gokul-aot --- playground/scenarios.py | 20 +++++++----------- src/agents/llm_factory.py | 6 ------ src/bindings/rest_api/app.py | 20 +++++++----------- tests/test_grpc_bindings.py | 41 +++++++++++++++++++++++++++--------- tests/test_toolhive_agent.py | 2 +- 5 files changed, 48 insertions(+), 41 deletions(-) diff --git a/playground/scenarios.py b/playground/scenarios.py index efb8b53e..50b0f97e 100644 --- a/playground/scenarios.py +++ b/playground/scenarios.py @@ -82,11 +82,6 @@ logger = logging.getLogger("playground.scenarios") -def _sanitize_log(value: str) -> str: - """Strip newlines/carriage-returns from user-supplied log values (log injection).""" - return value.replace("\n", " ").replace("\r", " ") if value else value - - router = APIRouter(prefix="/scenarios", tags=["scenarios"]) @@ -375,11 +370,12 @@ async def resolve_connector( raise HTTPException(status_code=400, detail=str(exc)) from exc name = resolve_config_name((config_name or "").strip() or None) if is_multitenancy_enabled(): + # Inline replace so CodeQL treats newline stripping as a sanitizer. logger.info( "Playground action | connector=%s | tenant_id=%s | config_name=%s", - _sanitize_log(connector_id), - _sanitize_log(tenant_id), - _sanitize_log(name or "(default)"), + str(connector_id).replace("\r", " ").replace("\n", " "), + str(tenant_id).replace("\r", " ").replace("\n", " "), + str(name or "(default)").replace("\r", " ").replace("\n", " "), ) try: return await factory.get( @@ -1966,8 +1962,8 @@ async def agent_chat(request: Request, payload: AgentChatInput) -> AgentChatResp if is_multitenancy_enabled(): logger.info( "Agent Chat | mcp_tenant_id=%s | config_name=%s", - tenant_id, - config_name or "(default)", + str(tenant_id).replace("\r", " ").replace("\n", " "), + str(config_name or "(default)").replace("\r", " ").replace("\n", " "), ) try: @@ -2114,8 +2110,8 @@ async def agent_chat_stream(request: Request, payload: AgentChatInput) -> Any: if is_multitenancy_enabled(): logger.info( "Agent Chat stream | mcp_tenant_id=%s | config_name=%s", - tenant_id, - config_name or "(default)", + str(tenant_id).replace("\r", " ").replace("\n", " "), + str(config_name or "(default)").replace("\r", " ").replace("\n", " "), ) async def stream_events(): diff --git a/src/agents/llm_factory.py b/src/agents/llm_factory.py index aee3404a..0d293b2f 100644 --- a/src/agents/llm_factory.py +++ b/src/agents/llm_factory.py @@ -28,12 +28,6 @@ from dataclasses import dataclass, field from typing import Any, Dict, List, Optional, Type -# Re-export so existing callers can still do: -# from agents.llm_factory import openai_compatible_tool_parameters -from agents.schema_utils import ( - openai_compatible_tool_parameters as openai_compatible_tool_parameters, -) # noqa: F401 - # --------------------------------------------------------------------------- # Data models (provider-agnostic) diff --git a/src/bindings/rest_api/app.py b/src/bindings/rest_api/app.py index d428ac93..9bd4ac48 100644 --- a/src/bindings/rest_api/app.py +++ b/src/bindings/rest_api/app.py @@ -175,11 +175,6 @@ def _config_tenant(request: Request) -> str: raise HTTPException(status_code=400, detail=str(exc)) from exc -def _sanitize_log(value: str) -> str: - """Strip newlines/carriage-returns from user-supplied log values (log injection).""" - return value.replace("\n", " ").replace("\r", " ") if value else value - - def _log_playground_tenant( *, action: str, @@ -190,13 +185,14 @@ def _log_playground_tenant( """Emit tenant context when multitenancy is on (visible in default formatters).""" if not is_multitenancy_enabled(): return - safe_tenant = _sanitize_log(tenant_id) - safe_config = _sanitize_log(config_name or "(default)") - parts = [f"Playground action | op={action}", f"tenant_id={safe_tenant}"] - if connector_id: - parts.append(f"connector={_sanitize_log(connector_id)}") - parts.append(f"config_name={safe_config}") - logger.info(" | ".join(parts)) + # Inline replace so CodeQL treats newline stripping as a sanitizer. + logger.info( + "Playground action | op=%s | tenant_id=%s | connector=%s | config_name=%s", + str(action).replace("\r", " ").replace("\n", " "), + str(tenant_id).replace("\r", " ").replace("\n", " "), + str(connector_id or "-").replace("\r", " ").replace("\n", " "), + str(config_name or "(default)").replace("\r", " ").replace("\n", " "), + ) def _map_config_error(exc: Exception) -> HTTPException: diff --git a/tests/test_grpc_bindings.py b/tests/test_grpc_bindings.py index d239ea27..4d86a1d2 100644 --- a/tests/test_grpc_bindings.py +++ b/tests/test_grpc_bindings.py @@ -201,7 +201,7 @@ async def _raise(*_a: Any, **_kw: Any) -> None: async def test_invoke_unknown_connector_returns_not_available( servicer: ConnectorServiceServicer, ) -> None: - with patch.object(servicer._factory, "get_for_protocol", return_value=None): + with patch.object(servicer._factory, "is_exposed", return_value=False): req = connector_pb2.InvokeRequest(connector_id="no_such", action="act") resp = await servicer._invoke_async(req) @@ -213,7 +213,10 @@ async def test_invoke_unknown_connector_returns_not_available( async def test_invoke_invalid_json_payload(servicer: ConnectorServiceServicer) -> None: fake_connector = MagicMock() - with patch.object(servicer._factory, "get_for_protocol", return_value=fake_connector): + with ( + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object(servicer._factory, "get", new=AsyncMock(return_value=fake_connector)), + ): req = connector_pb2.InvokeRequest( connector_id="x", action="y", payload_json="not-valid-json{" ) @@ -233,7 +236,10 @@ async def test_invoke_success_path(servicer: ConnectorServiceServicer) -> None: trace_id="trace-001", ) ) - with patch.object(servicer._factory, "get_for_protocol", return_value=fake_connector): + with ( + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object(servicer._factory, "get", new=AsyncMock(return_value=fake_connector)), + ): req = connector_pb2.InvokeRequest( connector_id="x", action="greet", payload_json='{"field": "val"}' ) @@ -253,7 +259,10 @@ async def mock_run(payload: Any, **_: Any) -> ConnectorResponse: fake_connector = MagicMock() fake_connector.run = mock_run - with patch.object(servicer._factory, "get_for_protocol", return_value=fake_connector): + with ( + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object(servicer._factory, "get", new=AsyncMock(return_value=fake_connector)), + ): req = connector_pb2.InvokeRequest( connector_id="x", action="do_thing", @@ -266,6 +275,7 @@ async def mock_run(payload: Any, **_: Any) -> ConnectorResponse: async def test_invoke_identity_propagated(servicer: ConnectorServiceServicer) -> None: from node_wire_runtime.caller_identity import build_caller_identity + from node_wire_runtime.config_store import DEFAULT_TENANT identity = build_caller_identity({"sub": "grpc-svc"}, auth_type="grpc_api_key") captured: list[Any] = [] @@ -277,17 +287,21 @@ async def mock_run(payload: Any, **kwargs: Any) -> ConnectorResponse: fake_connector = MagicMock() fake_connector.run = mock_run with ( - patch.object(servicer._factory, "get_for_protocol", return_value=fake_connector), + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object(servicer._factory, "get", new=AsyncMock(return_value=fake_connector)), patch("bindings.grpc_server.server.get_grpc_caller_identity", return_value=identity), ): req = connector_pb2.InvokeRequest(connector_id="x", action="act", payload_json="{}") await servicer._invoke_async(req) assert captured[0]["principal"] == identity.principal - assert captured[0]["tenant_id"] == identity.tenant_id + # Multitenancy off in tests: resolve_tenant_id always returns DEFAULT_TENANT. + assert captured[0]["tenant_id"] == DEFAULT_TENANT async def test_invoke_no_identity_passes_none(servicer: ConnectorServiceServicer) -> None: + from node_wire_runtime.config_store import DEFAULT_TENANT + captured: list[Any] = [] async def mock_run(payload: Any, **kwargs: Any) -> ConnectorResponse: @@ -297,20 +311,24 @@ async def mock_run(payload: Any, **kwargs: Any) -> ConnectorResponse: fake_connector = MagicMock() fake_connector.run = mock_run with ( - patch.object(servicer._factory, "get_for_protocol", return_value=fake_connector), + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object(servicer._factory, "get", new=AsyncMock(return_value=fake_connector)), patch("bindings.grpc_server.server.get_grpc_caller_identity", return_value=None), ): req = connector_pb2.InvokeRequest(connector_id="x", action="act", payload_json="{}") await servicer._invoke_async(req) assert captured[0]["principal"] is None - assert captured[0]["tenant_id"] is None + assert captured[0]["tenant_id"] == DEFAULT_TENANT async def test_invoke_empty_payload_json(servicer: ConnectorServiceServicer) -> None: fake_connector = MagicMock() fake_connector.run = AsyncMock(return_value=ConnectorResponse(success=True, trace_id="t4")) - with patch.object(servicer._factory, "get_for_protocol", return_value=fake_connector): + with ( + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object(servicer._factory, "get", new=AsyncMock(return_value=fake_connector)), + ): req = connector_pb2.InvokeRequest(connector_id="x", action="act") resp = await servicer._invoke_async(req) assert resp.success is True @@ -329,7 +347,10 @@ async def test_invoke_error_response_maps_error_category( trace_id="t5", ) ) - with patch.object(servicer._factory, "get_for_protocol", return_value=fake_connector): + with ( + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object(servicer._factory, "get", new=AsyncMock(return_value=fake_connector)), + ): req = connector_pb2.InvokeRequest(connector_id="x", action="act", payload_json="{}") resp = await servicer._invoke_async(req) diff --git a/tests/test_toolhive_agent.py b/tests/test_toolhive_agent.py index 118bcb98..0bb68df8 100644 --- a/tests/test_toolhive_agent.py +++ b/tests/test_toolhive_agent.py @@ -25,8 +25,8 @@ LLMProviderFactory, LLMResponse, ToolCall, - openai_compatible_tool_parameters, ) +from agents.schema_utils import openai_compatible_tool_parameters from agents.toolhive import ( ToolHiveAgent, ToolHiveMcpClient, From ca19428f8c710ba7d1a777cf7df124834dac942d Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Wed, 29 Jul 2026 15:41:39 +0530 Subject: [PATCH 10/13] pytest issues resolved Signed-off-by: gokul-aot --- tests/test_grpc_bindings.py | 48 ++++++++++++ tests/test_multitenancy.py | 138 +++++++++++++++++++++++++++++++++++ tests/test_toolhive_agent.py | 7 ++ 3 files changed, 193 insertions(+) diff --git a/tests/test_grpc_bindings.py b/tests/test_grpc_bindings.py index 4d86a1d2..147ef59c 100644 --- a/tests/test_grpc_bindings.py +++ b/tests/test_grpc_bindings.py @@ -357,3 +357,51 @@ async def test_invoke_error_response_maps_error_category( assert resp.success is False assert resp.error_code == "UPSTREAM_TIMEOUT" assert resp.error_category == ErrorCategory.RETRYABLE.value + + +async def test_invoke_missing_tenant_when_multitenancy_enabled( + servicer: ConnectorServiceServicer, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + with patch("bindings.grpc_server.server.get_grpc_caller_identity", return_value=None): + req = connector_pb2.InvokeRequest(connector_id="x", action="act") + resp = await servicer._invoke_async(req) + + assert resp.success is False + assert resp.error_code == "MISSING_TENANT" + assert resp.error_category == ErrorCategory.AUTH.value + + +async def test_invoke_config_not_found(servicer: ConnectorServiceServicer) -> None: + from node_wire_runtime.config_store import ConfigNotFoundError + + with ( + patch.object(servicer._factory, "is_exposed", return_value=True), + patch.object( + servicer._factory, + "get", + new=AsyncMock(side_effect=ConfigNotFoundError("no config")), + ), + ): + req = connector_pb2.InvokeRequest(connector_id="x", action="act", payload_json="{}") + resp = await servicer._invoke_async(req) + + assert resp.success is False + assert resp.error_code == "CONFIG_NOT_FOUND" + assert resp.error_category == ErrorCategory.AUTH.value + + +def test_invoke_passes_metadata_headers(servicer: ConnectorServiceServicer) -> None: + context = MagicMock() + context.invocation_metadata.return_value = (("x-tenant-id", "acme"),) + req = connector_pb2.InvokeRequest(connector_id="x", action="act") + + with patch("bindings.grpc_server.server._async_runner") as runner: + runner.run.return_value = connector_pb2.InvokeResponse(success=True, trace_id="t") + resp = servicer.Invoke(req, context) + + assert resp.success is True + call_args = runner.run.call_args[0][0] + # The coroutine was created with metadata; force close to avoid warnings. + call_args.close() diff --git a/tests/test_multitenancy.py b/tests/test_multitenancy.py index 598316ff..54b4b1ab 100644 --- a/tests/test_multitenancy.py +++ b/tests/test_multitenancy.py @@ -464,3 +464,141 @@ def test_resolve_config_name_enabled_rejects_non_string(monkeypatch: pytest.Monk monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") # JSON null and LLM quirks must map to omit (tenant default), not fail closed. assert resolve_config_name(None) is None # type: ignore[arg-type] + + +# --------------------------------------------------------------------------- # +# REST config CRUD (patch coverage for bindings.rest_api.app) +# --------------------------------------------------------------------------- # + + +def _config_factory() -> ConnectorFactory: + factory = ConnectorFactory() + # No YAML load needed — store starts empty; CRUD tests seed via API/store. + return factory + + +def test_rest_config_crud_roundtrip(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + factory = _config_factory() + app.dependency_overrides[get_factory] = lambda: factory + headers = {"X-Tenant-ID": "acme"} + try: + client = TestClient(app) + + create = client.post( + "/v1/connectors/slack/configs", + json={"name": "primary", "default": True, "config": {"channel": "#a"}}, + headers=headers, + ) + assert create.status_code == 201 + assert create.json() == {"name": "primary", "default": True} + + listed = client.get("/v1/connectors/slack/configs", headers=headers) + assert listed.status_code == 200 + assert any(item["name"] == "primary" for item in listed.json()) + + got = client.get("/v1/connectors/slack/configs/primary", headers=headers) + assert got.status_code == 200 + assert got.json()["name"] == "primary" + + client.post( + "/v1/connectors/slack/configs", + json={"name": "secondary", "config": {"channel": "#b"}}, + headers=headers, + ) + updated = client.put( + "/v1/connectors/slack/configs/secondary", + json={"name": "secondary", "config": {"channel": "#b2"}}, + headers=headers, + ) + assert updated.status_code == 200 + + set_def = client.put( + "/v1/connectors/slack/configs/secondary/default", + headers=headers, + ) + assert set_def.status_code == 200 + + deleted = client.delete( + "/v1/connectors/slack/configs/primary", + headers=headers, + ) + assert deleted.status_code == 200 + + missing = client.get("/v1/connectors/slack/configs/primary", headers=headers) + assert missing.status_code == 404 + finally: + app.dependency_overrides.clear() + + +def test_rest_config_missing_tenant_returns_400(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + factory = _config_factory() + app.dependency_overrides[get_factory] = lambda: factory + try: + client = TestClient(app) + resp = client.post( + "/v1/connectors/slack/configs", + json={"name": "x", "config": {}}, + ) + finally: + app.dependency_overrides.clear() + assert resp.status_code == 400 + assert "X-Tenant-ID" in resp.json()["detail"] + + +def test_rest_config_create_conflict_returns_409(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + factory = _config_factory() + factory.store.create("acme", "slack", _doc("dup", default=True)) + app.dependency_overrides[get_factory] = lambda: factory + try: + client = TestClient(app) + resp = client.post( + "/v1/connectors/slack/configs", + json={"name": "dup", "config": {}}, + headers={"X-Tenant-ID": "acme"}, + ) + finally: + app.dependency_overrides.clear() + assert resp.status_code == 409 + + +def test_rest_config_init_with_tenant_header(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + factory = _config_factory() + app.dependency_overrides[get_factory] = lambda: factory + try: + client = TestClient(app) + resp = client.post( + "/v1/config/init", + json={"slack": [{"name": "boot", "default": True, "config": {}}]}, + headers={"X-Tenant-ID": "acme"}, + ) + finally: + app.dependency_overrides.clear() + assert resp.status_code == 200 + assert factory.store.resolve("acme", "slack", None).name == "boot" + + +def test_rest_config_delete_default_without_replacement_returns_400( + monkeypatch: pytest.MonkeyPatch, +): + monkeypatch.setenv("NW_MULTITENANCY_ENABLED", "true") + factory = _config_factory() + factory.store.create("acme", "slack", _doc("keep", default=True)) + factory.store.create("acme", "slack", _doc("other")) + app.dependency_overrides[get_factory] = lambda: factory + try: + client = TestClient(app) + resp = client.delete( + "/v1/connectors/slack/configs/keep", + headers={"X-Tenant-ID": "acme"}, + ) + finally: + app.dependency_overrides.clear() + assert resp.status_code == 400 + + +def test_tenant_from_headers_none_value(): + assert tenant_from_headers({"X-Tenant-ID": None}) is None # type: ignore[dict-item] diff --git a/tests/test_toolhive_agent.py b/tests/test_toolhive_agent.py index 0bb68df8..3727f356 100644 --- a/tests/test_toolhive_agent.py +++ b/tests/test_toolhive_agent.py @@ -48,6 +48,9 @@ def test_openai_compatible_tool_parameters_allows_null_on_optional() -> None: "required_str": {"type": "string"}, "optional_str": {"type": "string"}, "already": {"type": ["string", "null"]}, + "list_no_null": {"type": ["string", "number"]}, + "null_only": {"type": "null"}, + "no_type": {"description": "untyped"}, }, "required": ["required_str"], } @@ -55,8 +58,12 @@ def test_openai_compatible_tool_parameters_allows_null_on_optional() -> None: assert out["properties"]["required_str"]["type"] == "string" assert out["properties"]["optional_str"]["type"] == ["string", "null"] assert out["properties"]["already"]["type"] == ["string", "null"] + assert out["properties"]["list_no_null"]["type"] == ["string", "number", "null"] + assert out["properties"]["null_only"]["type"] == "null" + assert "type" not in out["properties"]["no_type"] # Input schema must not be mutated. assert schema["properties"]["optional_str"]["type"] == "string" + assert schema["properties"]["list_no_null"]["type"] == ["string", "number"] def test_truncate_tool_result_for_llm_respects_limit(monkeypatch: pytest.MonkeyPatch) -> None: From ac9497e6b0746704192bb4fa2dd6325f18ba9edc Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Thu, 30 Jul 2026 10:12:54 +0530 Subject: [PATCH 11/13] reviewed comments resolved Signed-off-by: gokul-aot --- src/bindings/factory.py | 53 +++++++++++++++------------ src/bindings/rest_api/app.py | 10 +++-- tests/test_factory_and_rest.py | 67 ++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 27 deletions(-) diff --git a/src/bindings/factory.py b/src/bindings/factory.py index 791e8299..013aaca7 100644 --- a/src/bindings/factory.py +++ b/src/bindings/factory.py @@ -194,6 +194,9 @@ def __init__(self, config_path: str | Path | None = None) -> None: # protocol gating, and the MCP upstream-passthrough check. self._configs: Dict[str, ConnectorConfig] = {} self._instances: "OrderedDict[Tuple[str, str, str], BaseConnector]" = OrderedDict() + # Guards all OrderedDict mutations. asyncio.Lock is per-key single-flight only + # and does not protect sync/off-loop callers (invalidate, list_for_protocol). + self._instances_guard = threading.RLock() self._locks: Dict[Tuple[str, str, str], asyncio.Lock] = {} self._locks_guard = threading.Lock() self._max_instances = int(os.environ.get("NW_FACTORY_MAX_INSTANCES", "512")) @@ -449,21 +452,23 @@ async def get( record = self._store.resolve(tenant_id, connector_id, config_name) key = (tenant_id, connector_id, record.name) - inst = self._instances.get(key) - if inst is not None: - self._instances.move_to_end(key) - return inst - - lock = self._lock_for(key) - async with lock: + with self._instances_guard: inst = self._instances.get(key) if inst is not None: self._instances.move_to_end(key) return inst - inst = self._instantiate(record) - self._instances[key] = inst - self._evict_if_needed() - return inst + + lock = self._lock_for(key) + async with lock: + with self._instances_guard: + inst = self._instances.get(key) + if inst is not None: + self._instances.move_to_end(key) + return inst + inst = self._instantiate(record) + self._instances[key] = inst + self._evict_if_needed() + return inst def is_exposed(self, connector_id: str, protocol: str) -> bool: """Whether ``connector_id`` may be reached over ``protocol``. @@ -494,10 +499,11 @@ def invalidate_configs(self, tenant_id: str, connector_id: str, names: List[str] Called synchronously by the store on every mutating write; may run on a non-loop thread (store writes are plain sync Python).""" - for name in names: - old = self._instances.pop((tenant_id, connector_id, name), None) - if old is not None: - self._schedule_aclose(old) + with self._instances_guard: + for name in names: + old = self._instances.pop((tenant_id, connector_id, name), None) + if old is not None: + self._schedule_aclose(old) def _schedule_aclose(self, inst: BaseConnector) -> None: aclose = getattr(inst, "aclose", None) @@ -526,7 +532,7 @@ def _default_instance(self, connector_id: str) -> Optional[BaseConnector]: """Return (and cache) the ``__default__`` instance for a connector. Shares :attr:`_instances` with the async :meth:`get`, so enumeration and - the default-tenant invoke path resolve the SAME object. Sync (no lock): + the default-tenant invoke path resolve the SAME object. Sync: only invoked at import/enumeration and by the default-tenant fast path. """ try: @@ -534,13 +540,14 @@ def _default_instance(self, connector_id: str) -> Optional[BaseConnector]: except ConfigNotFoundError: return None key = (DEFAULT_TENANT, connector_id, record.name) - inst = self._instances.get(key) - if inst is None: - inst = self._instantiate(record) - self._instances[key] = inst - else: - self._instances.move_to_end(key) - return inst + with self._instances_guard: + inst = self._instances.get(key) + if inst is None: + inst = self._instantiate(record) + self._instances[key] = inst + else: + self._instances.move_to_end(key) + return inst def get_for_protocol( self, connector_id: str, protocol: str, action: Optional[str] = None diff --git a/src/bindings/rest_api/app.py b/src/bindings/rest_api/app.py index 9bd4ac48..e392ffd8 100644 --- a/src/bindings/rest_api/app.py +++ b/src/bindings/rest_api/app.py @@ -152,11 +152,13 @@ async def health() -> Dict[str, str]: async def ready() -> Dict[str, str]: try: factory = get_factory() - connectors = factory.list_for_protocol("rest") - except Exception: + if not factory.list_for_protocol("rest") and not factory.list_for_protocol("grpc"): + raise HTTPException(status_code=503, detail="no connectors loaded") + except HTTPException: + raise + except Exception as exc: + logger.warning("Readiness check failed: %s", exc) raise HTTPException(status_code=503, detail="factory not ready") - if not connectors: - raise HTTPException(status_code=503, detail="no connectors registered") return {"status": "ready"} diff --git a/tests/test_factory_and_rest.py b/tests/test_factory_and_rest.py index c3d9e2b4..dda2e539 100644 --- a/tests/test_factory_and_rest.py +++ b/tests/test_factory_and_rest.py @@ -365,3 +365,70 @@ def test_factory_default_scope_policy_is_deny_without_env( factory = ConnectorFactory() assert factory._policy_hook is not None + + +@pytest.mark.asyncio +async def test_factory_instances_cache_safe_under_concurrent_get_and_invalidate( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """_instances is shared by async get, sync invalidate, and _default_instance.""" + import asyncio + import threading + + from node_wire_runtime.config_store import DEFAULT_TENANT + + factory = ConnectorFactory() + factory.store.init( + { + DEFAULT_TENANT: { + "http_generic": [{"name": "default", "default": True, "config": {}}], + } + } + ) + + def _fake_instantiate(self: ConnectorFactory, record: object) -> MagicMock: + m = MagicMock() + m.aclose = AsyncMock() + return m + + monkeypatch.setattr(ConnectorFactory, "_instantiate", _fake_instantiate) + + errors: list[BaseException] = [] + + async def hammer_get() -> None: + try: + for _ in range(40): + await factory.get("http_generic", tenant_id=DEFAULT_TENANT) + except BaseException as exc: # noqa: BLE001 + errors.append(exc) + + def hammer_invalidate() -> None: + try: + for _ in range(40): + factory.invalidate_configs(DEFAULT_TENANT, "http_generic", ["default"]) + except BaseException as exc: # noqa: BLE001 + errors.append(exc) + + def hammer_default() -> None: + try: + for _ in range(40): + factory._default_instance("http_generic") + except BaseException as exc: # noqa: BLE001 + errors.append(exc) + + threads = [ + threading.Thread(target=hammer_invalidate), + threading.Thread(target=hammer_default), + threading.Thread(target=hammer_invalidate), + ] + for t in threads: + t.start() + await asyncio.gather(hammer_get(), hammer_get(), hammer_get()) + for t in threads: + t.join() + + assert errors == [] + # Cache remains a usable OrderedDict after concurrent mutation. + with factory._instances_guard: + _ = list(factory._instances.items()) + assert len(factory._instances) <= 1 From 33e8d533f3490101d5392e9654872ecf1e4d77b0 Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Thu, 30 Jul 2026 12:46:48 +0530 Subject: [PATCH 12/13] codeQL issues fixed Signed-off-by: gokul-aot --- tests/test_factory_and_rest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_factory_and_rest.py b/tests/test_factory_and_rest.py index dda2e539..aa8b0387 100644 --- a/tests/test_factory_and_rest.py +++ b/tests/test_factory_and_rest.py @@ -393,27 +393,27 @@ def _fake_instantiate(self: ConnectorFactory, record: object) -> MagicMock: monkeypatch.setattr(ConnectorFactory, "_instantiate", _fake_instantiate) - errors: list[BaseException] = [] + errors: list[Exception] = [] async def hammer_get() -> None: try: for _ in range(40): await factory.get("http_generic", tenant_id=DEFAULT_TENANT) - except BaseException as exc: # noqa: BLE001 + except Exception as exc: # noqa: BLE001 errors.append(exc) def hammer_invalidate() -> None: try: for _ in range(40): factory.invalidate_configs(DEFAULT_TENANT, "http_generic", ["default"]) - except BaseException as exc: # noqa: BLE001 + except Exception as exc: # noqa: BLE001 errors.append(exc) def hammer_default() -> None: try: for _ in range(40): factory._default_instance("http_generic") - except BaseException as exc: # noqa: BLE001 + except Exception as exc: # noqa: BLE001 errors.append(exc) threads = [ From 48ec72f705988f58a662ecd321a70478b07ffe59 Mon Sep 17 00:00:00 2001 From: gokul-aot Date: Thu, 6 Aug 2026 16:18:06 +0530 Subject: [PATCH 13/13] MT with configurable tenant id and configs Signed-off-by: gokul-aot --- .gitignore | 8 + docs/configuration.md | 3 +- playground/README.md | 8 +- playground/app.js | 578 +++++++++++++++++----- playground/index.html | 51 +- playground/style.css | 70 ++- src/bindings/factory.py | 13 +- src/bindings/rest_api/app.py | 163 +++++- src/bindings/rest_api/tenant_store.py | 485 ++++++++++++++++++ src/node_wire_runtime/config_store.py | 5 + src/node_wire_runtime/secrets/__init__.py | 4 + src/node_wire_runtime/secrets/base.py | 130 ++++- tests/conftest.py | 15 + tests/test_multitenancy.py | 19 +- tests/test_tenant_store.py | 405 +++++++++++++++ 15 files changed, 1740 insertions(+), 217 deletions(-) create mode 100644 src/bindings/rest_api/tenant_store.py create mode 100644 tests/test_tenant_store.py diff --git a/.gitignore b/.gitignore index eca270c9..47932757 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,10 @@ __pycache__/ *.egg-info/ dist/ .env +config/tenants.yaml +config/tenants.yaml.tmp +config/playground_tenants.yaml +config/playground_tenants.yaml.tmp .DS_Store **/.DS_Store .coverage @@ -46,3 +50,7 @@ launch.json # log mcp_debug.log .cursor/ + +# nw-mcp-builder +nw-mcp-builder/fixtures/ +nw-mcp-builder/out/ diff --git a/docs/configuration.md b/docs/configuration.md index 2d866962..d8d07ed0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -72,8 +72,9 @@ copy sample.env .env | `NW_MULTITENANCY_ENABLED` | When `true`, resolve tenant from header / `NW_TENANT_ID` / JWT and require a tenant (missing → error). When `false`, always `__default__`. | `false` | | `NW_TENANT_ID` | **MCP stdio only** — pins the process to one tenant. Do not set on multi-tenant streamable-http (use `X-Tenant-ID` instead). Required when multitenancy is enabled for stdio. | _(unset)_ | | `NW_TENANT_ID_HEADER` | HTTP/gRPC header name for tenant id (case-insensitive) | `X-Tenant-ID` | +| `NW_TENANTS_PATH` | Path to the YAML file that persists runtime named configs + tenant secret overlays (`config/tenants.yaml` by default; gitignored) | `config/tenants.yaml` | -Named-tenant secrets use `NW_{TENANT}_{CONNECTOR}_{KEY}` (see commented templates in `sample.env`). MCP transport details: [mcp-servers.md](mcp-servers.md#multi-tenancy-mcp). +Named-tenant secrets use `NW_{TENANT}_{CONNECTOR}_{CONFIG}_{KEY}` (one credential vault per named config). MCP transport details: [mcp-servers.md](mcp-servers.md#multi-tenancy-mcp). --- diff --git a/playground/README.md b/playground/README.md index 4ce2e716..6f7e1758 100644 --- a/playground/README.md +++ b/playground/README.md @@ -132,11 +132,11 @@ uv run node-wire ``` - **Tenant ID required**: connector/scenario calls without `X-Tenant-ID` return **400**. Explicit `__default__` is allowed. -- **Header**: Tenant ID, Config dropdown, and **Add config** appear in the header (next to Connectors Ready / Auto-retry) whenever multitenancy is on. -- **Agentic Workflow**: sends the same `X-Tenant-ID` (and optional `config_name` query). Local agent MCP runs **in-process** against the playground factory, so configs from **Add config** apply. Streamable-HTTP / ToolHive proxy URLs receive `X-Tenant-ID` on each MCP HTTP request. -- **Add config**: Use the header **Add config** button (Agent or Connectors) to open a modal for create/list/set-default/delete of a named config for **all** playground connectors under that tenant. Close via the X, backdrop click, or Escape. Each Create posts a self-contained auth block (secret refs matching `connectors.yaml`). Named-tenant secrets use `NW_{TENANT}_{CONNECTOR}_{KEY}` (e.g. `NW_ACME_GOOGLE_DRIVE_GOOGLE_DRIVE_SA_JSON`). +- **Header**: Tenant dropdown (existing tenants) and Config dropdown appear when multitenancy is on. Header **Add config** is hidden; use **Add config** on each System Connector page. +- **Config dropdown**: lists configs for the **active connector** only under the selected tenant. Switching connectors clears a name that does not exist for the new connector. +- **Agentic Workflow**: sends the same `X-Tenant-ID` (and optional `config_name` query). Local agent MCP runs **in-process** against the playground factory. Streamable-HTTP / ToolHive proxy URLs receive `X-Tenant-ID` on each MCP HTTP request. (Agent Add-config UI is unchanged / out of scope for this flow.) +- **Per-connector Add config**: On a connector page, **Add config** opens a modal for that connector only (tenant free-text for new tenants, config name, default flag, and varying credentials). Each named config has its **own** credential vault (`NW_{TENANT}_{CONNECTOR}_{CONFIG}_{KEY}`). Shared host env (e.g. `EPIC_FHIR_BASE_URL`, `EPIC_TOKEN_URL`) is copied into that config’s secret overlay when omitted. Persist file: gitignored `config/tenants.yaml` (holds secrets — do not commit). - **Tenant in logs**: When multitenancy is on, server INFO lines include `tenant_id` / `config_name` for Agent chat, connector scenarios, config mutations, REST connector calls, and MCP tool resolution. The playground Technical Audit panel also prints Tenant/Config for those actions. -- Per-connector panels do not embed their own runtime-config admin UI. #### Testing the MCP server with Inspector diff --git a/playground/app.js b/playground/app.js index f313abb3..12ea5a19 100644 --- a/playground/app.js +++ b/playground/app.js @@ -622,6 +622,10 @@ document.addEventListener('DOMContentLoaded', () => { if (slackPanel) slackPanel.classList.add('hidden'); if (extViewerPanel) extViewerPanel.classList.add('hidden'); + if (typeof syncMultitenancyUi === 'function') { + syncMultitenancyUi(); + } + if (mode === 'ehr') { ehrPanel.classList.remove('hidden'); @@ -796,25 +800,162 @@ document.addEventListener('DOMContentLoaded', () => { } const addConfigBtn = document.getElementById('add-config-btn'); if (addConfigBtn) { - // Global header control: show whenever multitenancy is on. - addConfigBtn.classList.toggle('hidden', !_multitenancyEnabled); + // Simplified: always hide header Add config (Agent work later). + addConfigBtn.classList.add('hidden'); + addConfigBtn.hidden = true; + } + const connectorAddBtn = document.getElementById('connector-add-config-btn'); + if (connectorAddBtn) { + const onConnector = + !!_modeToConnectorId[currentMode] && + playgroundView && + !playgroundView.classList.contains('hidden'); + connectorAddBtn.classList.toggle('hidden', !_multitenancyEnabled || !onConnector); } if (!_multitenancyEnabled) { closeConfigAdminModal(); } } - function openConfigAdminModal() { + async function openConfigAdminModal() { const modal = document.getElementById('config-admin-modal'); if (!modal) return; + const connectorId = currentConnectorId(); + if (!connectorId) { + log('Open a connector page before adding config.', 'error'); + return; + } + const tip = document.getElementById('config-admin-connector-tip'); + if (tip) tip.textContent = connectorId; + + const tenantId = getPlaygroundTenantId(); + const configName = getPlaygroundConfigName(); + if (_cfgTenantInput) { + _cfgTenantInput.value = tenantId || ''; + } + modal.classList.remove('hidden'); - if (_tenantInput) syncTenantInputs(_tenantInput); + setConfigAdminError(''); + await refreshModalConfigSelect(tenantId, connectorId, configName); + await applyModalConfigSelection(connectorId); refreshConfigDropdown(currentMode); } function closeConfigAdminModal() { const modal = document.getElementById('config-admin-modal'); if (modal) modal.classList.add('hidden'); + setConfigAdminError(''); + } + + function syncCfgNameGroupVisibility() { + const select = document.getElementById('cfg-select'); + const nameGroup = document.getElementById('cfg-name-group'); + const nameEl = document.getElementById('cfg-name'); + if (!select || !nameGroup || !nameEl) return; + const isNew = !select.value; + nameGroup.classList.toggle('hidden', !isNew); + if (!isNew) { + nameEl.value = select.value; + nameEl.readOnly = true; + } else { + nameEl.readOnly = false; + if (!nameEl.value.trim()) nameEl.value = 'test'; + } + } + + async function listConfigsForTenantConnector(tenantId, connectorId) { + if (!tenantId || !connectorId) return []; + const headers = { 'Content-Type': 'application/json', 'X-Tenant-ID': tenantId }; + const res = await fetch( + `/v1/connectors/${encodeURIComponent(connectorId)}/configs`, + { headers } + ); + if (!res.ok) return []; + const data = await res.json(); + return Array.isArray(data.configs) + ? data.configs + : Array.isArray(data) + ? data + : []; + } + + async function refreshModalConfigSelect(tenantId, connectorId, preferredName) { + const select = document.getElementById('cfg-select'); + if (!select) return; + const prev = preferredName != null ? preferredName : select.value; + select.innerHTML = ''; + if (!tenantId || !connectorId) { + syncCfgNameGroupVisibility(); + return; + } + try { + const configs = await listConfigsForTenantConnector(tenantId, connectorId); + configs.forEach((cfg) => { + const opt = document.createElement('option'); + opt.value = cfg.name || ''; + opt.textContent = (cfg.name || '') + (cfg.default ? ' (default)' : ''); + select.appendChild(opt); + }); + if (prev && [...select.options].some((o) => o.value === prev)) { + select.value = prev; + } else if (prev && configs.length === 0) { + // Deleted / unknown name: stay on new, keep typed name. + select.value = ''; + const nameEl = document.getElementById('cfg-name'); + if (nameEl && prev) nameEl.value = prev; + } else { + select.value = ''; + } + } catch (_) { + // ignore + } + syncCfgNameGroupVisibility(); + } + + async function applyModalConfigSelection(connectorId) { + const select = document.getElementById('cfg-select'); + const nameEl = document.getElementById('cfg-name'); + const defaultEl = document.getElementById('cfg-default'); + const tenantId = modalTenantId(); + const selected = select && select.value ? select.value : ''; + syncCfgNameGroupVisibility(); + + let existingKeys = []; + if (defaultEl) defaultEl.value = 'true'; + + if (tenantId && selected && connectorId) { + if (nameEl) nameEl.value = selected; + try { + const headers = { 'Content-Type': 'application/json', 'X-Tenant-ID': tenantId }; + const cfgRes = await fetch( + `/v1/connectors/${encodeURIComponent(connectorId)}/configs/${encodeURIComponent(selected)}`, + { headers } + ); + if (cfgRes.ok) { + const doc = await cfgRes.json(); + if (defaultEl && typeof doc.default === 'boolean') { + defaultEl.value = doc.default ? 'true' : 'false'; + } + const secRes = await fetch( + `/v1/connectors/${encodeURIComponent(connectorId)}/secrets?config_name=${encodeURIComponent(selected)}`, + { headers } + ); + if (secRes.ok) { + const sec = await secRes.json(); + existingKeys = Array.isArray(sec.keys) ? sec.keys : []; + } + } + } catch (_) { + // Prefill is best-effort. + } + } + renderSecretFields(connectorId || currentConnectorId(), existingKeys); + } + + function modalSelectedConfigName() { + const select = document.getElementById('cfg-select'); + if (select && select.value) return select.value.trim(); + return (document.getElementById('cfg-name')?.value || 'test').trim(); } async function loadFeatureFlags() { @@ -828,11 +969,13 @@ document.addEventListener('DOMContentLoaded', () => { // If endpoint is unreachable, keep default (disabled). } syncMultitenancyUi(); + if (_multitenancyEnabled) { + await refreshTenantDropdown(); + await refreshConfigDropdown(currentMode); + } } - loadFeatureFlags(); - - // Keep header Tenant and Runtime config Tenant fields in sync (shared state). + // Header Tenant is a select of existing tenants; modal Tenant ID is free-text for new ones. const _tenantInput = document.getElementById('playground-tenant-id'); const _cfgTenantInput = document.getElementById('cfg-tenant-id'); let _tenantSyncLock = false; @@ -841,30 +984,97 @@ document.addEventListener('DOMContentLoaded', () => { if (_tenantSyncLock) return; _tenantSyncLock = true; try { - const value = source && source.value != null ? source.value : ''; - if (_tenantInput && source !== _tenantInput) _tenantInput.value = value; - if (_cfgTenantInput && source !== _cfgTenantInput) _cfgTenantInput.value = value; + const value = source && source.value != null ? String(source.value).trim() : ''; + if (_cfgTenantInput && source !== _cfgTenantInput) { + // Do not overwrite free-text while user types a new tenant in the modal. + if (source === _tenantInput) _cfgTenantInput.value = value; + } + if (_tenantInput && source === _cfgTenantInput && value) { + ensureTenantOption(value, true); + } } finally { _tenantSyncLock = false; } } + function ensureTenantOption(tenantId, selectIt) { + if (!_tenantInput || !tenantId) return; + let found = false; + for (const opt of _tenantInput.options) { + if (opt.value === tenantId) { + found = true; + break; + } + } + if (!found) { + const opt = document.createElement('option'); + opt.value = tenantId; + opt.textContent = tenantId; + _tenantInput.appendChild(opt); + } + if (selectIt) _tenantInput.value = tenantId; + } + + async function refreshTenantDropdown() { + if (!_multitenancyEnabled || !_tenantInput) return; + const prev = _tenantInput.value; + try { + const res = await fetch('/v1/tenants'); + if (!res.ok) return; + const data = await res.json(); + const tenants = Array.isArray(data.tenants) ? data.tenants : []; + _tenantInput.innerHTML = ''; + tenants.forEach((tid) => { + const opt = document.createElement('option'); + opt.value = tid; + opt.textContent = tid; + _tenantInput.appendChild(opt); + }); + if (prev && tenants.includes(prev)) { + _tenantInput.value = prev; + } else if (prev) { + ensureTenantOption(prev, true); + } + } catch (_) { + // ignore + } + } + let _tenantDebounce = null; - function onTenantInput(ev) { - syncTenantInputs(ev.target); + function onTenantChanged() { clearTimeout(_tenantDebounce); - _tenantDebounce = setTimeout(() => refreshConfigDropdown(currentMode), 400); + _tenantDebounce = setTimeout(() => refreshConfigDropdown(currentMode), 200); + if (_cfgTenantInput && _tenantInput) { + syncTenantInputs(_tenantInput); + } } if (_tenantInput) { - if (_cfgTenantInput && !_cfgTenantInput.value) { - _cfgTenantInput.value = _tenantInput.value || ''; - } - _tenantInput.addEventListener('input', onTenantInput); + _tenantInput.addEventListener('change', onTenantChanged); } if (_cfgTenantInput) { - _cfgTenantInput.addEventListener('input', onTenantInput); + _cfgTenantInput.addEventListener('input', () => syncTenantInputs(_cfgTenantInput)); + let _cfgTenantDebounce = null; + _cfgTenantInput.addEventListener('change', () => { + clearTimeout(_cfgTenantDebounce); + _cfgTenantDebounce = setTimeout(async () => { + const connectorId = currentConnectorId(); + const tenantId = modalTenantId(); + await refreshModalConfigSelect(tenantId, connectorId, ''); + await applyModalConfigSelection(connectorId); + }, 150); + }); + _cfgTenantInput.addEventListener('blur', async () => { + const connectorId = currentConnectorId(); + const tenantId = modalTenantId(); + await refreshModalConfigSelect(tenantId, connectorId, document.getElementById('cfg-select')?.value || ''); + await applyModalConfigSelection(connectorId); + }); } + document.getElementById('cfg-select')?.addEventListener('change', async () => { + await applyModalConfigSelection(currentConnectorId()); + }); + // Map playground mode names to connector_id strings used by the config API. const _modeToConnectorId = { ehr: 'fhir_epic', @@ -875,19 +1085,12 @@ document.addEventListener('DOMContentLoaded', () => { stripe: 'stripe', salesforce: 'salesforce', }; - // Fixed list for tenant-global config CRUD (store is still per-connector). - const _allPlaygroundConnectors = [ - 'fhir_epic', - 'fhir_cerner', - 'google_drive', - 'http_generic', - 'slack', - 'stripe', - 'salesforce', - ]; + + function currentConnectorId() { + return _modeToConnectorId[currentMode] || ''; + } // Self-contained auth/config templates matching config/connectors.yaml (secret refs only). - // Simplified: kept inline so Create stays playground-local; no new API. const _connectorCreateDocs = { http_generic: { config: {}, @@ -965,49 +1168,158 @@ document.addEventListener('DOMContentLoaded', () => { }, }; + // Varying-only secret fields shown in Add config (shared env auto-copied server-side). + // Format / required secret validation is server-side only (tenant_store). + const _connectorSecretFields = { + google_drive: [ + { key: 'GOOGLE_DRIVE_SA_JSON', label: 'Service account JSON', type: 'textarea', required: true }, + ], + fhir_epic: [ + { key: 'EPIC_CLIENT_ID', label: 'Client ID', type: 'text', required: true }, + { key: 'EPIC_PRIVATE_KEY', label: 'Private key (PEM)', type: 'textarea', required: true }, + { key: 'EPIC_KID', label: 'Key ID (kid)', type: 'text', required: true }, + ], + fhir_cerner: [ + { key: 'CERNER_CLIENT_ID', label: 'Client ID', type: 'text', required: true }, + { key: 'CERNER_PRIVATE_KEY', label: 'Private key (PEM)', type: 'textarea', required: true }, + { key: 'CERNER_KID', label: 'Key ID (kid)', type: 'text', required: true }, + { key: 'CERNER_SCOPES', label: 'Scopes (space-separated)', type: 'text', required: false }, + ], + slack: [ + { key: 'SLACK_BOT_TOKEN', label: 'Bot token', type: 'text', required: true }, + ], + stripe: [ + { key: 'stripe_api_key', label: 'API key', type: 'text', required: true }, + ], + salesforce: [ + { key: 'SALESFORCE_CLIENT_ID', label: 'Client ID', type: 'text', required: true }, + { key: 'SALESFORCE_CLIENT_SECRET', label: 'Client secret', type: 'text', required: true }, + { key: 'SALESFORCE_REFRESH_TOKEN', label: 'Refresh token', type: 'text', required: true }, + ], + http_generic: [], + }; + + function setConfigAdminError(message) { + const el = document.getElementById('cfg-error'); + if (!el) return; + if (message) { + el.textContent = message; + el.classList.remove('hidden'); + } else { + el.textContent = ''; + el.classList.add('hidden'); + } + } + + function renderSecretFields(connectorId, existingKeys = []) { + const host = document.getElementById('cfg-secret-fields'); + if (!host) return; + host.innerHTML = ''; + const fields = _connectorSecretFields[connectorId] || []; + const existing = new Set(existingKeys || []); + if (!fields.length) { + host.innerHTML = '

No credential fields for this connector.

'; + return; + } + fields.forEach((f) => { + const wrap = document.createElement('div'); + wrap.className = 'field-group'; + const alreadySet = existing.has(f.key); + const label = document.createElement('label'); + label.setAttribute('for', `cfg-secret-${f.key}`); + label.textContent = + f.label + (f.required && !alreadySet ? ' *' : '') + (alreadySet ? ' (saved)' : ''); + let input; + if (f.type === 'textarea') { + input = document.createElement('textarea'); + input.rows = 5; + } else { + input = document.createElement('input'); + input.type = 'text'; + } + input.id = `cfg-secret-${f.key}`; + input.dataset.secretKey = f.key; + input.dataset.alreadySet = alreadySet ? '1' : '0'; + input.autocomplete = 'off'; + input.value = ''; + if (alreadySet) { + input.placeholder = '(already set — leave blank to keep)'; + } else if (f.required) { + input.placeholder = 'Required'; + } + wrap.appendChild(label); + wrap.appendChild(input); + host.appendChild(wrap); + }); + if ([...existing].some((k) => fields.some((f) => f.key === k))) { + const hint = document.createElement('p'); + hint.className = 'field-help'; + hint.textContent = + 'Blank credential fields keep the previously saved value. Only type a field to change it.'; + host.appendChild(hint); + } + } + + function collectSecretFields() { + const host = document.getElementById('cfg-secret-fields'); + const secrets = {}; + if (!host) return secrets; + host.querySelectorAll('[data-secret-key]').forEach((el) => { + const key = el.dataset.secretKey; + const val = (el.value || '').trim(); + if (key && val) secrets[key] = val; + }); + return secrets; + } + async function refreshConfigDropdown(modeOrConnectorId) { if (!_multitenancyEnabled) return; const tenantId = getPlaygroundTenantId(); const select = document.getElementById('playground-config-name'); if (!select) return; + const previous = select.value; select.innerHTML = ''; if (!tenantId) return; - // Prefer an explicit connector (opened mode), else google_drive, else first success. - const preferred = + const connectorId = _modeToConnectorId[modeOrConnectorId] || - modeOrConnectorId || - 'google_drive'; - const tryOrder = [ - preferred, - ..._allPlaygroundConnectors.filter((c) => c !== preferred), - ]; + (Object.values(_modeToConnectorId).includes(modeOrConnectorId) + ? modeOrConnectorId + : currentConnectorId()); + if (!connectorId) return; + const headers = { 'Content-Type': 'application/json', 'X-Tenant-ID': tenantId }; - for (const connectorId of tryOrder) { - try { - const res = await fetch( - `/v1/connectors/${encodeURIComponent(connectorId)}/configs`, - { headers } - ); - if (!res.ok) continue; - const data = await res.json(); - const configs = Array.isArray(data.configs) - ? data.configs - : Array.isArray(data) - ? data - : []; - if (!configs.length) continue; - configs.forEach((cfg) => { - const opt = document.createElement('option'); - opt.value = cfg.name || ''; - opt.textContent = cfg.name + (cfg.default ? ' (default)' : ''); - if (cfg.default) opt.selected = true; - select.appendChild(opt); - }); - return; - } catch (_) { - // try next connector + try { + const res = await fetch( + `/v1/connectors/${encodeURIComponent(connectorId)}/configs`, + { headers } + ); + if (!res.ok) return; + const data = await res.json(); + const configs = Array.isArray(data.configs) + ? data.configs + : Array.isArray(data) + ? data + : []; + let matched = false; + configs.forEach((cfg) => { + const opt = document.createElement('option'); + opt.value = cfg.name || ''; + opt.textContent = cfg.name + (cfg.default ? ' (default)' : ''); + select.appendChild(opt); + if (previous && opt.value === previous) matched = true; + }); + if (previous && matched) { + select.value = previous; + } else if (previous && !matched) { + // Clear stale name from another connector (e.g. Drive → Epic). + select.value = ''; + } else { + const def = configs.find((c) => c.default); + if (def) select.value = def.name || ''; } + } catch (_) { + // ignore } } @@ -1023,6 +1335,8 @@ document.addEventListener('DOMContentLoaded', () => { return (el && el.value ? el.value : '').trim(); } + loadFeatureFlags(); + function logTenancyContext(actionLabel) { if (!_multitenancyEnabled) return; const tenantId = getPlaygroundTenantId(); @@ -1080,7 +1394,11 @@ document.addEventListener('DOMContentLoaded', () => { if (!response.ok) { const detail = data.detail || data.message || `Server returned ${response.status}`; - throw new Error(typeof detail === 'string' ? detail : JSON.stringify(detail)); + let msg = typeof detail === 'string' ? detail : JSON.stringify(detail); + if (response.status === 403 && /No connector configuration/i.test(msg)) { + msg = `${msg} Open Add config on this connector page.`; + } + throw new Error(msg); } traceDisplay.textContent = (data.trace_id || '').toUpperCase() || '—'; @@ -1533,22 +1851,24 @@ document.addEventListener('DOMContentLoaded', () => { } }); - // --- Tenant-global runtime config CRUD (bulk across all connectors) --- - const cfgResult = document.getElementById('cfg-result'); + // --- Per-connector runtime config CRUD --- const addConfigBtn = document.getElementById('add-config-btn'); + const connectorAddConfigBtn = document.getElementById('connector-add-config-btn'); - function showCfgResult(obj) { - if (!cfgResult) return; - cfgResult.classList.remove('hidden'); - cfgResult.textContent = typeof obj === 'string' ? obj : JSON.stringify(obj, null, 2); + function modalTenantId() { + const fromModal = (_cfgTenantInput && _cfgTenantInput.value ? _cfgTenantInput.value : '').trim(); + return fromModal || getPlaygroundTenantId(); } - async function cfgFetchFor(connectorId, method, path = '', body = null) { - const tenantId = getPlaygroundTenantId(); + async function cfgFetchFor(connectorId, method, path = '', body = null, tenantOverride = null) { + const tenantId = (tenantOverride || getPlaygroundTenantId() || '').trim(); if (!tenantId) { throw new Error('Set Tenant ID before managing configs (e.g. acme).'); } - const opts = { method, headers: playgroundRequestHeaders() }; + const opts = { + method, + headers: { 'Content-Type': 'application/json', 'X-Tenant-ID': tenantId }, + }; if (body != null) { opts.body = JSON.stringify(body); } @@ -1564,28 +1884,16 @@ document.addEventListener('DOMContentLoaded', () => { return data; } - // Simplified: run the same CRUD against every playground connector; report per-id results. - // body may be a plain object or (connectorId) => object for per-connector Create docs. - async function cfgBulk(method, pathBuilder, body = null) { - logTenancyContext(`Config ${method}`); - const results = {}; - for (const connectorId of _allPlaygroundConnectors) { - try { - const path = typeof pathBuilder === 'function' ? pathBuilder(connectorId) : pathBuilder || ''; - const payload = typeof body === 'function' ? body(connectorId) : body; - results[connectorId] = await cfgFetchFor(connectorId, method, path, payload); - } catch (err) { - results[connectorId] = { error: err.message }; - } - } - return results; - } - if (addConfigBtn) { addConfigBtn.addEventListener('click', () => { openConfigAdminModal(); }); } + if (connectorAddConfigBtn) { + connectorAddConfigBtn.addEventListener('click', () => { + openConfigAdminModal(); + }); + } document.getElementById('config-admin-close')?.addEventListener('click', () => { closeConfigAdminModal(); @@ -1606,66 +1914,68 @@ document.addEventListener('DOMContentLoaded', () => { }); document.getElementById('cfg-create')?.addEventListener('click', async () => { + setConfigAdminError(''); try { - const name = (document.getElementById('cfg-name')?.value || 'default').trim(); - const isDefault = document.getElementById('cfg-default')?.value === 'true'; - // Self-contained docs per connector (auth secret refs from connectors.yaml shapes). - const data = await cfgBulk('POST', '', (connectorId) => { - const tmpl = _connectorCreateDocs[connectorId] || { config: {}, auth: {} }; - return { - name, - default: isDefault, - config: tmpl.config || {}, - auth: tmpl.auth || {}, - }; - }); - showCfgResult(data); - log(`Created config '${name}' for tenant '${getPlaygroundTenantId()}' (all connectors)`, 'success'); - await refreshConfigDropdown(currentMode); - } catch (err) { - showCfgResult({ error: err.message }); - log(`Config create failed: ${err.message}`, 'error'); - } - }); - - document.getElementById('cfg-list')?.addEventListener('click', async () => { - try { - const data = await cfgBulk('GET', ''); - showCfgResult(data); - log(`Listed configs for tenant '${getPlaygroundTenantId()}' (all connectors)`, 'success'); - await refreshConfigDropdown(currentMode); - } catch (err) { - showCfgResult({ error: err.message }); - log(`Config list failed: ${err.message}`, 'error'); - } - }); - - document.getElementById('cfg-set-default')?.addEventListener('click', async () => { - try { - const name = (document.getElementById('cfg-name')?.value || '').trim(); + const connectorId = currentConnectorId(); + if (!connectorId) throw new Error('Open a connector page first.'); + const tenantId = modalTenantId(); + if (!tenantId) throw new Error('Tenant ID is required'); + const name = modalSelectedConfigName(); if (!name) throw new Error('Config name is required'); - const data = await cfgBulk('PUT', `/${encodeURIComponent(name)}/default`); - showCfgResult(data); - log(`Set '${name}' as default for tenant '${getPlaygroundTenantId()}'`, 'success'); + const isDefault = document.getElementById('cfg-default')?.value === 'true'; + const secrets = collectSecretFields(); + const tmpl = _connectorCreateDocs[connectorId] || { config: {}, auth: {} }; + const body = { + name, + default: isDefault, + config: tmpl.config || {}, + auth: tmpl.auth || {}, + secrets, + }; + logTenancyContext(`Config SAVE ${connectorId}`); + await cfgFetchFor(connectorId, 'POST', '', body, tenantId); + ensureTenantOption(tenantId, true); + await refreshTenantDropdown(); + ensureTenantOption(tenantId, true); await refreshConfigDropdown(currentMode); + const headerCfg = document.getElementById('playground-config-name'); + if (headerCfg) headerCfg.value = name; + await refreshModalConfigSelect(tenantId, connectorId, name); + await applyModalConfigSelection(connectorId); + log(`Saved config '${name}' for ${connectorId} / tenant '${tenantId}'`, 'success'); } catch (err) { - showCfgResult({ error: err.message }); - log(`Set default failed: ${err.message}`, 'error'); + setConfigAdminError(err.message); + log(`Config save failed: ${err.message}`, 'error'); } }); document.getElementById('cfg-delete')?.addEventListener('click', async () => { + setConfigAdminError(''); try { - const name = (document.getElementById('cfg-name')?.value || '').trim(); + const connectorId = currentConnectorId(); + if (!connectorId) throw new Error('Open a connector page first.'); + const tenantId = modalTenantId(); + const name = modalSelectedConfigName(); if (!name) throw new Error('Config name is required'); const newDefault = name === 'default' ? '' : 'default'; const q = newDefault ? `?new_default=${encodeURIComponent(newDefault)}` : ''; - const data = await cfgBulk('DELETE', `/${encodeURIComponent(name)}${q}`); - showCfgResult(data); - log(`Deleted config '${name}' for tenant '${getPlaygroundTenantId()}'`, 'success'); + await cfgFetchFor( + connectorId, + 'DELETE', + `/${encodeURIComponent(name)}${q}`, + null, + tenantId + ); + log(`Deleted config '${name}' for ${connectorId} / tenant '${tenantId}'`, 'success'); + const configSelect = document.getElementById('playground-config-name'); + if (configSelect && configSelect.value === name) { + configSelect.value = ''; + } await refreshConfigDropdown(currentMode); + await refreshModalConfigSelect(tenantId, connectorId, ''); + await applyModalConfigSelection(connectorId); } catch (err) { - showCfgResult({ error: err.message }); + setConfigAdminError(err.message); log(`Config delete failed: ${err.message}`, 'error'); } }); diff --git a/playground/index.html b/playground/index.html index 1d23d396..6bf17cf4 100644 --- a/playground/index.html +++ b/playground/index.html @@ -58,8 +58,9 @@

node-wire

@@ -309,10 +311,13 @@

Salesforce