From d9a3d26153248f7847b44881ce4be82bcb8b5ab6 Mon Sep 17 00:00:00 2001 From: Ling-Sen Peng Date: Thu, 13 Aug 2026 16:19:12 -0700 Subject: [PATCH] Restore the e2e test bodies removed in #441 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #441 deleted four test functions along with the cli_credentials conftest fixture, leaving suites 2 and 3 collecting nothing and suites 4 and 5 unable to run. Restored verbatim, with one substantive change: credentials now go through the server's /api/secrets REST API instead of shelling out to the agentspan CLI. The CLI targeted /api/credentials, which only Orkes serves (404 on conductor-oss) — that was the sole reason these suites could not run there. They now run on both, and need no conftest fixture. suite collected Orkes conductor-oss 2 tool_calling 0 -> 1 pass skip (store read-only) 3 cli_tools 0 -> 1 pass skip at the credential write 4 mcp_tools 1 -> 2 pass pass 5 http_tools 1 -> 2 pass pass Suites 4 and 5 adopt a pre-provisioned credential when the store is read-only, so their authenticated phases run on conductor-oss too. Suites 2 and 3 set and then update values, which requires a writable store, so they skip there with a message naming the cause. Also: suite 3's whitelist checks moved ahead of the credential write (they need no store, and were otherwise stranded behind the skip), and suite 4 gains an assertion that a tool result the model cannot invent appears in the answer. test_suite16_cli_skills.py was also removed by #441 — intentionally, so not restored. --- e2e/test_suite2_tool_calling.py | 249 +++++++++++++++++++++++++++++++- e2e/test_suite3_cli_tools.py | 202 +++++++++++++++++++++++++- e2e/test_suite4_mcp_tools.py | 218 +++++++++++++++++++++++++++- e2e/test_suite5_http_tools.py | 152 ++++++++++++++++++- 4 files changed, 812 insertions(+), 9 deletions(-) diff --git a/e2e/test_suite2_tool_calling.py b/e2e/test_suite2_tool_calling.py index a641945f..7d42264b 100644 --- a/e2e/test_suite2_tool_calling.py +++ b/e2e/test_suite2_tool_calling.py @@ -3,11 +3,11 @@ Tests the credential pipeline end-to-end: 1. Tools fail when credentials are missing 2. Env vars are NOT read (security boundary) - 3. Credentials added via CLI are resolved at execution time + 3. Credentials added to the server store are resolved at execution time 4. Credential updates propagate to subsequent runs Single sequential test with try/finally cleanup. -No mocks. Real server, real CLI, real LLM. +No mocks. Real server, real LLM. """ import os @@ -28,6 +28,38 @@ CRED_B = "E2E_CRED_B" TIMEOUT = 300 # 5 min per agent run — CI runners are slower +API = os.environ.get("CONDUCTOR_SERVER_URL", "http://localhost:8080/api").rstrip("/") + + +# ── Credential store (server API — no CLI) ────────────────────────────── + + +def _put_secret(name: str, value: str) -> None: + """Store a credential, skipping the suite when the store is read-only. + + Unlike a suite that only *consumes* a credential, this one sets specific + values and then updates them, so it needs a writable store: conductor-oss + serves secrets from the server process env and rejects writes with 501. + """ + r = requests.put( + f"{API}/secrets/{name}", + data=value, + headers={"Content-Type": "text/plain"}, + timeout=10, + ) + if not r.ok: + pytest.skip( + f"server credential store rejected a write (HTTP {r.status_code}) — " + f"this suite needs a writable store to set and update credentials" + ) + + +def _delete_secret(name: str) -> None: + try: + requests.delete(f"{API}/secrets/{name}", timeout=10) + except Exception: + pass # best-effort cleanup + # ── Tools ─────────────────────────────────────────────────────────────── @@ -204,9 +236,9 @@ def _credential_audit(agent: Agent) -> str: # Fetch stored credentials from server try: - resp = requests.get(f"{base_url}/api/credentials", timeout=5) + resp = requests.get(f"{base_url}/api/secrets", timeout=5) resp.raise_for_status() - stored = {c["name"] for c in resp.json()} + stored = {c if isinstance(c, str) else c.get("name") for c in resp.json()} except Exception as e: return f"(could not fetch credentials from server: {e})" @@ -291,6 +323,215 @@ def _get_output_text(result) -> str: # ── Test ──────────────────────────────────────────────────────────────── +@pytest.mark.timeout(300) +class TestSuite2ToolCalling: + """Credential lifecycle: missing -> env ignored -> add -> update.""" + + @pytest.mark.usefixtures("requires_runtime_metadata") + def test_credential_lifecycle(self, runtime, model): + """Full credential lifecycle test — sequential steps with cleanup.""" + try: + self._run_lifecycle(runtime, model) + finally: + # Always clean up credentials + _delete_secret(CRED_A) + _delete_secret(CRED_B) + # Clean env vars if they leaked + os.environ.pop(CRED_A, None) + os.environ.pop(CRED_B, None) + + def _run_lifecycle(self, runtime, model): + agent = _make_agent(model) + owned_runtimes: list[AgentRuntime] = [] + + def restart_runtime(current: AgentRuntime) -> AgentRuntime: + current.shutdown() + # Let old poll loops drain before new workers start with fresh + # execution tokens for the updated credential state. + time.sleep(2) + fresh = AgentRuntime() + owned_runtimes.append(fresh) + return fresh + + try: + # ── Step 1: Clean slate ───────────────────────────────────── + _delete_secret(CRED_A) + _delete_secret(CRED_B) + + # ── Step 2: No credentials — paid tools should fail ───────── + result = runtime.run(agent, "Call all three tools.", timeout=TIMEOUT) + + assert result.execution_id, ( + f"[Step 2: No credentials] No execution_id returned. " + f"{_run_diagnostic(result)}" + ) + + # The run should reach a terminal state (COMPLETED or FAILED). + # Paid tools should raise RuntimeError because credentials are missing. + assert result.status in ("COMPLETED", "FAILED", "TERMINATED"), ( + f"[Step 2: No credentials] Expected terminal status, " + f"got '{result.status}'. The agent should either complete " + f"(reporting tool errors) or fail outright when credentials " + f"are missing.\n" + f" {_run_diagnostic(result)}\n" + f" {_tool_diagnostics(result.execution_id)}" + ) + + # Verify via workflow tasks: paid tools must be terminal (not retryable). + # Conductor maps TaskResult.FAILED_WITH_TERMINAL_ERROR → Task.COMPLETED_WITH_ERRORS + tool_tasks_s2 = _find_tool_tasks_for(result.execution_id) + terminal_statuses = {"FAILED_WITH_TERMINAL_ERROR", "COMPLETED_WITH_ERRORS"} + for paid in ("paid_tool_a", "paid_tool_b"): + if paid in tool_tasks_s2: + task_info = tool_tasks_s2[paid] + assert task_info["status"] in terminal_statuses, ( + f"[Step 2: No credentials] {paid} should be terminal " + f"(not retryable), got '{task_info['status']}'. Missing " + f"credentials are a config issue — retries are pointless.\n" + f" task={task_info}" + ) + + # ── Step 3: Env vars should NOT be read ───────────────────── + os.environ[CRED_A] = "from-env-aaa" + os.environ[CRED_B] = "from-env-bbb" + try: + result_env = runtime.run( + agent, "Call all three tools.", timeout=TIMEOUT + ) + + # The paid tools should STILL fail despite env vars being set. + # The SDK resolves credentials from the server, not env. + output_env = _get_output_text(result_env) + + # Check for "from-env" (unique prefix of our test env values). + # Using "fro" caused false positives when LLM prose contained + # "from" in normal words. + assert "from-env" not in output_env, ( + "SECURITY VIOLATION: env vars were read for credential " + "resolution! The SDK MUST NOT resolve credentials from " + "environment variables — only from the server.\n" + f" {_run_diagnostic(result_env)}\n" + f" output_text={output_env[:300]}" + ) + finally: + os.environ.pop(CRED_A, None) + os.environ.pop(CRED_B, None) + + # ── Step 4: Add credentials ───────────────────────────────── + runtime = restart_runtime(runtime) + _put_secret(CRED_A, "secret-aaa-value") + _put_secret(CRED_B, "secret-bbb-value") + + result_with_creds = runtime.run( + agent, "Call all three tools.", timeout=TIMEOUT + ) + _assert_run_completed(result_with_creds, "Step 4: With credentials", agent) + + # Primary: validate via workflow task data + tool_tasks_s4 = _find_tool_tasks_for(result_with_creds.execution_id) + + assert "free_tool" in tool_tasks_s4, ( + f"[Step 4] free_tool task not found in workflow.\n" + f" found_tasks={list(tool_tasks_s4.keys())}" + ) + assert tool_tasks_s4["free_tool"]["status"] == "COMPLETED", ( + f"[Step 4] free_tool not COMPLETED.\n" + f" task={tool_tasks_s4['free_tool']}" + ) + + assert "paid_tool_a" in tool_tasks_s4, ( + f"[Step 4] paid_tool_a task not found in workflow.\n" + f" found_tasks={list(tool_tasks_s4.keys())}" + ) + assert tool_tasks_s4["paid_tool_a"]["status"] == "COMPLETED", ( + f"[Step 4] paid_tool_a not COMPLETED.\n" + f" task={tool_tasks_s4['paid_tool_a']}" + ) + s4_paid_a_output = str(tool_tasks_s4["paid_tool_a"]["output"]) + assert "sec" in s4_paid_a_output, ( + f"[Step 4] paid_tool_a output should contain 'sec' " + f"(first 3 chars of 'secret-aaa-value').\n" + f" task_output={s4_paid_a_output}" + ) + + assert "paid_tool_b" in tool_tasks_s4, ( + f"[Step 4] paid_tool_b task not found in workflow.\n" + f" found_tasks={list(tool_tasks_s4.keys())}" + ) + assert tool_tasks_s4["paid_tool_b"]["status"] == "COMPLETED", ( + f"[Step 4] paid_tool_b not COMPLETED.\n" + f" task={tool_tasks_s4['paid_tool_b']}" + ) + s4_paid_b_output = str(tool_tasks_s4["paid_tool_b"]["output"]) + assert "sec" in s4_paid_b_output, ( + f"[Step 4] paid_tool_b output should contain 'sec' " + f"(first 3 chars of 'secret-bbb-value').\n" + f" task_output={s4_paid_b_output}" + ) + + # Secondary: also check LLM output text + output_creds = _get_output_text(result_with_creds) + + assert "free" in output_creds.lower(), ( + f"[Step 4: With credentials] free_tool output not found in " + f"agent response. free_tool always returns 'free:ok' — if " + f"missing, the agent may not have called it.\n" + f" {_run_diagnostic(result_with_creds)}\n" + f" output_text={output_creds[:300]}\n" + f" {_tool_diagnostics(result_with_creds.execution_id)}" + ) + assert "sec" in output_creds, ( + f"[Step 4: With credentials] paid_tool_a should return 'sec' " + f"(first 3 chars of 'secret-aaa-value'). If missing, credential " + f"'{CRED_A}' may not have been resolved correctly.\n" + f" {_run_diagnostic(result_with_creds)}\n" + f" output_text={output_creds[:300]}\n" + f" {_tool_diagnostics(result_with_creds.execution_id)}" + ) + + # ── Step 5: Update credentials ────────────────────────────── + runtime = restart_runtime(runtime) + _put_secret(CRED_A, "newval-xxx-updated") + _put_secret(CRED_B, "newval-yyy-updated") + + result_updated = runtime.run( + agent, "Call all three tools.", timeout=TIMEOUT + ) + _assert_run_completed(result_updated, "Step 5: Updated credentials", agent) + + # Primary: validate via workflow task data + tool_tasks_s5 = _find_tool_tasks_for(result_updated.execution_id) + + assert "paid_tool_a" in tool_tasks_s5, ( + f"[Step 5] paid_tool_a task not found in workflow.\n" + f" found_tasks={list(tool_tasks_s5.keys())}" + ) + assert tool_tasks_s5["paid_tool_a"]["status"] == "COMPLETED", ( + f"[Step 5] paid_tool_a not COMPLETED.\n" + f" task={tool_tasks_s5['paid_tool_a']}" + ) + s5_paid_a_output = str(tool_tasks_s5["paid_tool_a"]["output"]) + assert "new" in s5_paid_a_output, ( + f"[Step 5] paid_tool_a output should contain 'new' " + f"(first 3 chars of 'newval-xxx-updated').\n" + f" task_output={s5_paid_a_output}" + ) + + # Secondary: also check LLM output text + output_updated = _get_output_text(result_updated) + + assert "new" in output_updated, ( + f"[Step 5: Updated credentials] paid_tool_a should return 'new' " + f"(first 3 chars of 'newval-xxx-updated'). If missing, the " + f"credential update may not have propagated.\n" + f" {_run_diagnostic(result_updated)}\n" + f" output_text={output_updated[:300]}\n" + f" {_tool_diagnostics(result_updated.execution_id)}" + ) + finally: + for owned in reversed(owned_runtimes): + owned.shutdown() + # Output masking (Audit gap D) is covered deterministically by the server's # SecretMaskingIntegrationTest (MockMvc + @MockBean AgentService). An e2e diff --git a/e2e/test_suite3_cli_tools.py b/e2e/test_suite3_cli_tools.py index 1c3821ad..e8935a71 100644 --- a/e2e/test_suite3_cli_tools.py +++ b/e2e/test_suite3_cli_tools.py @@ -7,7 +7,7 @@ 4. Commands outside whitelist are rejected (cd) Single sequential test with try/finally cleanup. -No mocks. Real server, real CLI, real LLM. +No mocks. Real server, real gh CLI, real LLM. """ import os @@ -28,6 +28,38 @@ CRED_NAME = "GITHUB_TOKEN" TIMEOUT = 120 +API = os.environ.get("CONDUCTOR_SERVER_URL", "http://localhost:8080/api").rstrip("/") + + +# ── Credential store (server API — no agentspan CLI) ──────────────────── + + +def _put_secret(name: str, value: str) -> None: + """Store a credential, skipping the suite when the store is read-only. + + This suite removes the credential, proves ``gh`` fails without it, then adds + the real token back — so it needs a writable store. conductor-oss serves + secrets from the server process env and rejects writes with 501. + """ + r = requests.put( + f"{API}/secrets/{name}", + data=value, + headers={"Content-Type": "text/plain"}, + timeout=10, + ) + if not r.ok: + pytest.skip( + f"server credential store rejected a write (HTTP {r.status_code}) — " + f"this suite needs a writable store to add and remove the token" + ) + + +def _delete_secret(name: str) -> None: + try: + requests.delete(f"{API}/secrets/{name}", timeout=10) + except Exception: + pass # best-effort cleanup + # ── Tools ─────────────────────────────────────────────────────────────── @@ -231,3 +263,171 @@ def _assert_run_completed(result, step_name: str): f"[{step_name}] Run did not complete. {diag}\n" f" {_tool_diagnostics(result.execution_id, {'cli_ls', 'cli_mktemp', 'cli_gh'})}" ) + + +@pytest.mark.timeout(600) +class TestSuite3CliTools: + """CLI tools: credential lifecycle + command whitelist.""" + + @pytest.mark.usefixtures("requires_runtime_metadata") + def test_cli_credential_lifecycle(self, runtime, model): + """Full CLI credential lifecycle — sequential steps with cleanup.""" + real_token = os.environ.get("GITHUB_TOKEN") + if not real_token: + pytest.skip( + "GITHUB_TOKEN not set in environment — " + "required for Suite 3 CLI tools test" + ) + + # Verify gh CLI is installed + try: + subprocess.run( + ["gh", "--version"], capture_output=True, text=True, timeout=5 + ) + except FileNotFoundError: + pytest.skip("gh CLI not installed — required for Suite 3 CLI tools test") + + try: + self._run_lifecycle(runtime, model, real_token) + finally: + _delete_secret(CRED_NAME) + os.environ.pop(CRED_NAME, None) + + def _run_lifecycle(self, runtime, model, real_token): + agent = _make_agent(model) + + # ── Step 1: Clean slate — remove credential from server ───── + _delete_secret(CRED_NAME) + + # ── Step 2: Export GITHUB_TOKEN to env ────────────────────── + # This validates the SDK does NOT read credentials from env. + # The real token is in the env but NOT in the server store. + os.environ["GITHUB_TOKEN"] = real_token + + # ── Step 3: Run agent — ls/mktemp succeed, gh fails ──────── + result = runtime.run(agent, PROMPT_ALL_THREE, timeout=TIMEOUT) + + assert result.execution_id, ( + f"[Step 3: No credential] No execution_id. " + f"{_run_diagnostic(result)}" + ) + assert result.status in ("COMPLETED", "FAILED", "TERMINATED"), ( + f"[Step 3: No credential] Expected terminal status, " + f"got '{result.status}'. The agent should complete or fail " + f"when gh credential is missing.\n" + f" {_run_diagnostic(result)}\n" + f" {_tool_diagnostics(result.execution_id, {'cli_ls', 'cli_mktemp', 'cli_gh'})}" + ) + + output = _get_output_text(result) + + # ls and mktemp should succeed (no credentials needed) + assert "ls_ok" in output, ( + f"[Step 3: No credential] cli_ls should succeed — it needs no " + f"credentials.\n" + f" output={output[:500]}\n" + f" {_run_diagnostic(result)}\n" + f" {_tool_diagnostics(result.execution_id, {'cli_ls', 'cli_mktemp', 'cli_gh'})}" + ) + assert "mktemp_ok" in output, ( + f"[Step 3: No credential] cli_mktemp should succeed — it needs " + f"no credentials.\n" + f" output={output[:500]}\n" + f" {_run_diagnostic(result)}" + ) + + # gh should fail — credential not in server, env must NOT be used + assert "gh_ok" not in output, ( + f"[Step 3: No credential] SECURITY: cli_gh should NOT succeed — " + f"GITHUB_TOKEN is in env but NOT in the server credential store. " + f"If it succeeded, env vars are leaking through credential " + f"isolation.\n" + f" output={output[:500]}" + ) + + # ── Step 4: cd command — not allowed ───────────────────────── + # All validation is algorithmic — no LLM output parsing. + + EXPECTED_ALLOWED = ["ls", "mktemp", "gh"] + whitelist_agent = _make_whitelist_agent(model) + + # 4a. Validate whitelist via plan() — the compiled tool description + # must list exactly the expected allowed commands. + plan = runtime.plan(whitelist_agent) + ad = plan["workflowDef"]["metadata"]["agentDef"] + cli_tool = next( + (t for t in ad.get("tools", []) if "run_command" in t["name"]), + None, + ) + assert cli_tool is not None, ( + f"[Step 4: cd blocked] No run_command tool in compiled agent. " + f"Tools: {[t['name'] for t in ad.get('tools', [])]}" + ) + # Parse the exact allowed commands from the tool description. + # Format: "... Allowed commands: gh, ls, mktemp. ..." + tool_desc = cli_tool.get("description", "") + match = re.search(r"Allowed commands:\s*(.+?)\.", tool_desc) + assert match, ( + f"[Step 4: cd blocked] Could not find 'Allowed commands:' in " + f"compiled run_command tool description.\n" + f" description={tool_desc}" + ) + actual_commands = sorted(c.strip() for c in match.group(1).split(",")) + assert actual_commands == sorted(EXPECTED_ALLOWED), ( + f"[Step 4: cd blocked] Allowed commands mismatch.\n" + f" expected={sorted(EXPECTED_ALLOWED)}\n" + f" actual={actual_commands}" + ) + + # 4b. Validate cd rejection directly — call the validation function + # and assert it raises ValueError with the correct message. + with pytest.raises(ValueError, match="not allowed") as exc_info: + _validate_cli_command("cd", EXPECTED_ALLOWED) + + error_msg = str(exc_info.value) + for cmd in EXPECTED_ALLOWED: + assert cmd in error_msg, ( + f"[Step 4: cd blocked] Rejection error must list '{cmd}' " + f"as an allowed command.\n" + f" error_msg={error_msg}" + ) + + # 4c. Run the agent to verify it reaches terminal status. + result_cd = runtime.run(whitelist_agent, PROMPT_CD, timeout=TIMEOUT) + + assert result_cd.execution_id, ( + f"[Step 4: cd blocked] No execution_id. " + f"{_run_diagnostic(result_cd)}" + ) + assert result_cd.status in ("COMPLETED", "FAILED", "TERMINATED"), ( + f"[Step 4: cd blocked] Expected terminal status, " + f"got '{result_cd.status}'.\n" + f" {_run_diagnostic(result_cd)}" + ) + # ── Step 5: Add credential to the server store ────────────── + _put_secret(CRED_NAME, real_token) + + # ── Step 6: Run agent — all three should succeed ──────────── + result = runtime.run(agent, PROMPT_ALL_THREE, timeout=TIMEOUT) + _assert_run_completed(result, "Step 5: With credential") + + output = _get_output_text(result) + + assert "ls_ok" in output, ( + f"[Step 6: With credential] cli_ls should succeed.\n" + f" output={output[:500]}\n" + f" {_run_diagnostic(result)}" + ) + assert "mktemp_ok" in output, ( + f"[Step 6: With credential] cli_mktemp should succeed.\n" + f" output={output[:500]}\n" + f" {_run_diagnostic(result)}" + ) + assert "gh_ok" in output, ( + f"[Step 6: With credential] cli_gh should succeed — " + f"GITHUB_TOKEN was added to server credential store.\n" + f" output={output[:500]}\n" + f" {_run_diagnostic(result)}\n" + f" {_tool_diagnostics(result.execution_id, {'cli_ls', 'cli_mktemp', 'cli_gh'})}" + ) + diff --git a/e2e/test_suite4_mcp_tools.py b/e2e/test_suite4_mcp_tools.py index 4bcdfd51..b579f8dc 100644 --- a/e2e/test_suite4_mcp_tools.py +++ b/e2e/test_suite4_mcp_tools.py @@ -6,7 +6,7 @@ Manages its own mcp-testkit instance on a dedicated port. Single sequential test with try/finally cleanup. -No mocks. Real server, real CLI, real LLM. +No mocks. Real server, real LLM. """ import asyncio @@ -35,6 +35,51 @@ CRED_NAME = "MCP_AUTH_KEY" TIMEOUT = 120 +API = os.environ.get("CONDUCTOR_SERVER_URL", "http://localhost:8080/api").rstrip("/") + + +# ── Credential store (server API — no CLI) ─────────────────────────────── + + +def _ensure_credential(name: str, preferred: str) -> tuple[str, bool]: + """Make a credential available to the server. Returns (auth_key, created_by_us). + + Provisioning differs by server flavor, but *consuming* a credential is core + behaviour on both, so the auth phase should run on either: + + - Orkes: the store is writable, so store ``preferred``. + - conductor-oss: the store is env-backed and read-only (writes return 501), + so adopt whatever ``CONDUCTOR_SECRET_`` already holds rather than + insisting on our own value. + + Skips only when neither is possible. + """ + r = requests.put( + f"{API}/secrets/{name}", + data=preferred, + headers={"Content-Type": "text/plain"}, + timeout=10, + ) + if r.ok: + return preferred, True + + existing = requests.get(f"{API}/secrets/{name}", timeout=10) + if existing.ok and existing.text.strip(): + return existing.text.strip(), False + + pytest.skip( + f"no credential available for {name}: the store rejected the write " + f"(HTTP {r.status_code}) and the name is not provisioned. Set " + f"CONDUCTOR_SECRET_{name} in the server environment to run this phase." + ) + + +def _delete_secret(name: str) -> None: + try: + requests.delete(f"{API}/secrets/{name}", timeout=10) + except Exception: + pass # best-effort cleanup + # ── Expected tools (from mcp-testkit source) ───────────────────────────── def _expected_tools_from_source(): @@ -61,6 +106,12 @@ def _expected_tools_from_source(): "encoding_base64_encode": "dGVzdA==", # base64("test") } +# mcp-testkit's get_weather returns these fixed values for any city. Unlike the +# tools above — whose results an LLM can work out unaided — these are arbitrary, +# so an answer containing them proves the tool's result reached the model. +WEATHER_PROMPT = "What is the weather in San Francisco right now?" +WEATHER_EXPECTED = ["77", "45"] # temperature_f, humidity_pct + # ── MCP Server Management ─────────────────────────────────────────────── @@ -159,6 +210,24 @@ def _make_agent(model, server_url): ) +def _make_weather_agent(model, server_url): + """Agent asked an open question, so it must rely on what the tool returned.""" + mt = mcp_tool( + server_url=server_url, + name="weather_mcp", + description="Weather tools via MCP — current conditions for a city", + ) + return Agent( + name="e2e_mcp_weather", + model=model, + instructions=( + "You are a weather assistant. Use the available MCP tools to answer " + "questions about weather conditions." + ), + tools=[mt], + ) + + def _make_auth_agent(model, server_url, cred_name): """Agent with authenticated MCP tools (credential in headers).""" mt = mcp_tool( @@ -355,3 +424,150 @@ def _validate_tool_execution(result, step_name): f"expected value '{expected}'.\n" f" output={output_str[:300]}" ) + + + +# ── Test ───────────────────────────────────────────────────────────────── + + +@pytest.mark.timeout(600) +class TestSuite4McpTools: + """MCP tools: discovery, execution, and authenticated access.""" + + def test_mcp_result_reaches_the_answer(self, runtime, model): + """The tool's result must be present in the agent's answer. + + Asserts only on what a caller sees. The values are arbitrary fixtures the + model cannot work out on its own, so an answer carrying them proves the + tool's output travelled back into the conversation. + """ + # Verify mcp-testkit is installed + try: + subprocess.run( + ["mcp-testkit", "--help"], + capture_output=True, + text=True, + timeout=5, + ) + except FileNotFoundError: + pytest.skip( + "mcp-testkit not installed — required for Suite 4 MCP tools test" + ) + + server_proc = None + try: + server_proc = _start_mcp_server(MCP_PORT) + agent = _make_weather_agent(model, MCP_SERVER_URL) + result = runtime.run(agent, WEATHER_PROMPT, timeout=TIMEOUT) + _assert_run_completed(result, "Tool result in answer") + + answer = str(result.output) + missing = [v for v in WEATHER_EXPECTED if v not in answer] + assert not missing, ( + f"[Tool result in answer] answer omits {missing} from " + f"get_weather's result — the tool ran but its output never " + f"reached the model.\n answer={answer[:400]}" + ) + finally: + if server_proc: + _stop_mcp_server(server_proc) + + def test_mcp_lifecycle(self, runtime, model): + """Full MCP lifecycle — unauthenticated → authenticated.""" + # Verify mcp-testkit is installed + try: + subprocess.run( + ["mcp-testkit", "--help"], + capture_output=True, + text=True, + timeout=5, + ) + except FileNotFoundError: + pytest.skip( + "mcp-testkit not installed — required for Suite 4 MCP tools test" + ) + + server_proc = None + created = False + try: + created = self._run_lifecycle(runtime, model) + finally: + # Only remove what we stored; a pre-provisioned credential belongs + # to the environment. + if created: + _delete_secret(CRED_NAME) + + def _run_lifecycle(self, runtime, model) -> bool: + server_proc = None + created = False + try: + # ── Phase 1: Unauthenticated ────────────────────────────── + + # Step d: Start MCP server without auth + server_proc = _start_mcp_server(MCP_PORT) + + # Step e: Discover tools, validate all are present + discovered = _discover_tools_via_mcp(MCP_SERVER_URL) + assert len(discovered) == EXPECTED_TOOL_COUNT, ( + f"[Phase 1: Discovery] Expected {EXPECTED_TOOL_COUNT} tools, " + f"discovered {len(discovered)}.\n" + f" Missing: {sorted(set(EXPECTED_TOOL_NAMES) - set(discovered))}\n" + f" Extra: {sorted(set(discovered) - set(EXPECTED_TOOL_NAMES))}" + ) + assert set(discovered) == set(EXPECTED_TOOL_NAMES), ( + f"[Phase 1: Discovery] Tool names mismatch.\n" + f" Missing: {sorted(set(EXPECTED_TOOL_NAMES) - set(discovered))}\n" + f" Extra: {sorted(set(discovered) - set(EXPECTED_TOOL_NAMES))}" + ) + + # Steps b+c+f: Create agent, run with 3 tools, validate + agent = _make_agent(model, MCP_SERVER_URL) + result = runtime.run(agent, PROMPT_USE_3_TOOLS, timeout=TIMEOUT) + _validate_tool_execution(result, "Phase 1: Unauthenticated execution") + + # ── Phase 2: Authenticated ──────────────────────────────── + + # Resolve the credential before restarting: mcp-testkit's --auth key + # has to match whatever the server will inject, and on a read-only + # store that value is the environment's, not ours. + auth_key, created = _ensure_credential(CRED_NAME, MCP_AUTH_KEY) + + # Step g: Stop server, restart with auth + _stop_mcp_server(server_proc) + server_proc = None + time.sleep(1) # Let port release + server_proc = _start_mcp_server(MCP_PORT, auth_key=auth_key) + + # Verify auth is enforced — unauthenticated call should fail + with pytest.raises(Exception): + _discover_tools_via_mcp(MCP_SERVER_URL) + + # Step h: Create auth agent with credential placeholder + auth_agent = _make_auth_agent(model, MCP_SERVER_URL, CRED_NAME) + + # Step j: Discover tools with auth, validate all present + discovered_auth = _discover_tools_via_mcp( + MCP_SERVER_URL, auth_key=auth_key + ) + assert len(discovered_auth) == EXPECTED_TOOL_COUNT, ( + f"[Phase 2: Auth Discovery] Expected {EXPECTED_TOOL_COUNT} tools, " + f"discovered {len(discovered_auth)}." + ) + assert set(discovered_auth) == set(EXPECTED_TOOL_NAMES), ( + f"[Phase 2: Auth Discovery] Tool names mismatch.\n" + f" Missing: {sorted(set(EXPECTED_TOOL_NAMES) - set(discovered_auth))}\n" + f" Extra: {sorted(set(discovered_auth) - set(EXPECTED_TOOL_NAMES))}" + ) + + # Step k: Execute and validate + result_auth = runtime.run( + auth_agent, PROMPT_USE_3_TOOLS, timeout=TIMEOUT + ) + _validate_tool_execution( + result_auth, "Phase 2: Authenticated execution" + ) + return created + + finally: + if server_proc: + _stop_mcp_server(server_proc) diff --git a/e2e/test_suite5_http_tools.py b/e2e/test_suite5_http_tools.py index cbabe799..01e01b9c 100644 --- a/e2e/test_suite5_http_tools.py +++ b/e2e/test_suite5_http_tools.py @@ -7,12 +7,10 @@ Manages its own mcp-testkit instance on a dedicated port. Single sequential test with try/finally cleanup. -No mocks. Real server, real CLI, real LLM. +No mocks. Real server, real LLM. """ -import inspect import os -import re import subprocess import time @@ -37,6 +35,51 @@ ORKES_SPEC_URL = "https://developer.orkescloud.com/api-docs" +API = os.environ.get("CONDUCTOR_SERVER_URL", "http://localhost:8080/api").rstrip("/") + + +# ── Credential store (server API — no CLI) ─────────────────────────────── + + +def _ensure_credential(name: str, preferred: str) -> tuple[str, bool]: + """Make a credential available to the server. Returns (auth_key, created_by_us). + + Provisioning differs by server flavor, but *consuming* a credential is core + behaviour on both, so the auth phase should run on either: + + - Orkes: the store is writable, so store ``preferred``. + - conductor-oss: the store is env-backed and read-only (writes return 501), + so adopt whatever ``CONDUCTOR_SECRET_`` already holds rather than + insisting on our own value. + + Skips only when neither is possible. + """ + r = requests.put( + f"{API}/secrets/{name}", + data=preferred, + headers={"Content-Type": "text/plain"}, + timeout=10, + ) + if r.ok: + return preferred, True + + existing = requests.get(f"{API}/secrets/{name}", timeout=10) + if existing.ok and existing.text.strip(): + return existing.text.strip(), False + + pytest.skip( + f"no credential available for {name}: the store rejected the write " + f"(HTTP {r.status_code}) and the name is not provisioned. Set " + f"CONDUCTOR_SECRET_{name} in the server environment to run this phase." + ) + + +def _delete_secret(name: str) -> None: + try: + requests.delete(f"{API}/secrets/{name}", timeout=10) + except Exception: + pass # best-effort cleanup + # ── Expected tools (from mcp-testkit endpoint registry) ────────────────── @@ -450,6 +493,109 @@ def _validate_tool_execution(result, step_name): class TestSuite5HttpTools: """HTTP tools: API discovery, execution, and authenticated access.""" + def test_http_lifecycle(self, runtime, model): + """Full HTTP lifecycle — unauthenticated → authenticated.""" + try: + subprocess.run( + ["mcp-testkit", "--help"], + capture_output=True, + text=True, + timeout=15, + ) + except (FileNotFoundError, subprocess.TimeoutExpired): + pytest.skip( + "mcp-testkit not installed or unresponsive — required for " + "Suite 5 HTTP tools test" + ) + + server_proc = None + created = False + try: + created = self._run_lifecycle(runtime, model) + finally: + # Only remove what we stored; a pre-provisioned credential belongs + # to the environment. + if created: + _delete_secret(CRED_NAME) + + def _run_lifecycle(self, runtime, model) -> bool: + server_proc = None + created = False + try: + # ── Phase 1: Unauthenticated ────────────────────────────── + + # Step d: Start HTTP server without auth + server_proc = _start_http_server(HTTP_PORT) + + # Step e: Discover tools via OpenAPI spec, validate all present + discovered = _discover_tools_via_openapi(HTTP_SPEC_URL) + assert len(discovered) == EXPECTED_TOOL_COUNT, ( + f"[Phase 1: Discovery] Expected {EXPECTED_TOOL_COUNT} tools, " + f"discovered {len(discovered)}.\n" + f" Missing: {sorted(set(EXPECTED_TOOL_NAMES) - set(discovered))}\n" + f" Extra: {sorted(set(discovered) - set(EXPECTED_TOOL_NAMES))}" + ) + assert set(discovered) == set(EXPECTED_TOOL_NAMES), ( + f"[Phase 1: Discovery] Tool names mismatch.\n" + f" Missing: {sorted(set(EXPECTED_TOOL_NAMES) - set(discovered))}\n" + f" Extra: {sorted(set(discovered) - set(EXPECTED_TOOL_NAMES))}" + ) + + # Steps b+c+f: Create agent, run with 3 tools, validate + agent = _make_agent(model, HTTP_BASE_URL) + result = runtime.run(agent, PROMPT_USE_3_TOOLS, timeout=TIMEOUT) + _validate_tool_execution(result, "Phase 1: Unauthenticated execution") + + # ── Phase 2: Authenticated ──────────────────────────────── + + # Resolve the credential before restarting: the HTTP server's auth key + # has to match whatever the server will inject, and on a read-only + # store that value is the environment's, not ours. + auth_key, created = _ensure_credential(CRED_NAME, HTTP_AUTH_KEY) + + # Step g: Stop server, restart with auth + _stop_http_server(server_proc) + server_proc = None + time.sleep(1) # Let port release + server_proc = _start_http_server(HTTP_PORT, auth_key=auth_key) + + # Verify auth is enforced — unauthenticated spec fetch should fail + unauth_resp = requests.get(HTTP_SPEC_URL, timeout=5) + assert unauth_resp.status_code in (401, 403), ( + f"[Phase 2: Auth check] Expected 401/403 without auth, " + f"got {unauth_resp.status_code}" + ) + + # Step h: Create auth agent with credential placeholder + auth_agent = _make_auth_agent(model, HTTP_BASE_URL, CRED_NAME) + + # Step i: Discover tools with auth, validate all present + discovered_auth = _discover_tools_via_openapi( + HTTP_SPEC_URL, auth_key=auth_key + ) + assert len(discovered_auth) == EXPECTED_TOOL_COUNT, ( + f"[Phase 2: Auth Discovery] Expected {EXPECTED_TOOL_COUNT} tools, " + f"discovered {len(discovered_auth)}." + ) + assert set(discovered_auth) == set(EXPECTED_TOOL_NAMES), ( + f"[Phase 2: Auth Discovery] Tool names mismatch.\n" + f" Missing: {sorted(set(EXPECTED_TOOL_NAMES) - set(discovered_auth))}\n" + f" Extra: {sorted(set(discovered_auth) - set(EXPECTED_TOOL_NAMES))}" + ) + + # Step j: Execute and validate + result_auth = runtime.run( + auth_agent, PROMPT_USE_3_TOOLS, timeout=TIMEOUT + ) + _validate_tool_execution( + result_auth, "Phase 2: Authenticated execution" + ) + return created + + finally: + if server_proc: + _stop_http_server(server_proc) + def test_external_openapi_spec(self, runtime, model): """External OpenAPI spec — validate startWorkflow discovery (steps l-n).