test: consolidate live MCP E2E suites

[msureshkumar88]

Closes #6517

Summary

Consolidates the live MCP protocol and RBAC transport E2E suites into one authoritative module: tests/live_gateway/e2e/test_e2e.py.

• Migrates all 15 existing test classes without adding lifecycle or cross-replica coverage.
• Adds make test-e2e as the canonical command for the consolidated suite.
• Retains test-mcp-protocol-e2e and test-mcp-rbac as deprecated aliases that delegate to make test-e2e with the issue-specified v1.3.0 warning.
• Removes the two superseded MCP suite modules.

Implementation details

• Preserves the async MCP SDK client and the synchronous Playwright/MCP client paths in one module.
• Uses one module-level E2E and unreachable-gateway skip marker.
• Keeps shared fixtures and constants declared once, including the common client timeout.
• Preserves existing flaky and Rust-transport skip markers.
• Updates package metadata, test guidance, runtime documentation, and source comments that referenced the migrated files.

Validation results

• Verified an exact AST match for all 15 migrated classes and their 65 class methods.
• Collected 64 executable tests from the consolidated suite.
• Confirmed all 64 tests self-skip cleanly when no gateway is reachable.
• Ran Ruff, pre-commit, diff whitespace checks, Make target dry-runs, and the changed-files secret scan successfully.
• Confirmed no active repository references remain to either deleted test module.

Environment notes

A live Docker gateway stack was not available, so the consolidated suite was not executed against running replicas. Repository-wide Bandit reports pre-existing findings in unchanged production files, and Pylint could not install the existing pylint-pydantic wheel because its metadata is invalid.

[gandhipratik203]

1. Aliases now run the same suite twice. ~60 refs across 15 files still pair make test-mcp-protocol-e2e with make test-mcp-rbac, which is a doubled 64-test run plus duplicate fixture setup. Fix AGENTS.md:123 first; it's the pre-merge gate.

2. test-e2e lost the old targets' env guidance (MCP_CLI_BASE_URL, JWT_SECRET_KEY, PLATFORM_ADMIN_EMAIL, MCP Apps flag, MCP_E2E_CLIENT_TIMEOUT, K filter, SSE note). The code still reads MCP_E2E_CLIENT_TIMEOUT, so keep that hint.

3. Playwright installs on every run. The 24 protocol tests never needed a browser, and --with-deps needs root on Linux. Worth gating.

[msureshkumar88]

Addressed after rebasing onto origin/main in 37ce992.

1. The legacy aliases now share test-e2e as a prerequisite, so invoking both aliases executes the consolidated suite once. The pre-merge gate and active validation guidance now use test-e2e directly.

2. Restored the target-level setup guidance for JWT_SECRET_KEY, PLATFORM_ADMIN_EMAIL, MCP Apps, MCP_E2E_CLIENT_TIMEOUT, K filtering, and the SSE gateway requirement. One clarification: MCP_CLI_BASE_URL and K support were already retained, and the consolidated module docstring already documented the URL, JWT, admin email, and MCP Apps flag.

3. Removed playwright install --with-deps chromium. The suite uses Playwright API request contexts and does not launch a browser, so the browser and privileged system-dependency installation was unnecessary.

Validated with make -n test-mcp-protocol-e2e test-mcp-rbac (one pytest invocation), make test-e2e (64 tests self-skip cleanly without a gateway), make pre-commit, and make detect-secrets-scan.

[gandhipratik203]

All three points are addressed and verified: single pytest invocation across both aliases, setup guidance restored, and the browser install correctly dropped. Worth one make test-e2e against a live stack before merge to close acceptance criterion 2.

[msureshkumar88]

Local validation completed against the three-replica Docker testing stack.

• make test-e2e collected 64 tests and exited successfully.
• The local test process needs the stack’s JWT_SECRET_KEY exported so authenticated MCP setup uses the same signing key.

No test-source changes were required.

[ja8zyjits]

---

🚨 Blocking Issues

• tests/e2e/README.md "What's Tested" omits all 9 RBAC classes

  • File: tests/e2e/README.md:52-65
  • Code: tests/live_gateway/e2e/test_e2e.py has 15 classes total — 6 protocol (TestConnectivity, TestTools, TestDiscovery, TestToolCalls, TestRawJsonRpc, TestRawHttpTransportParity) + 9 RBAC (TestServerVisibilityViaAPI, TestMcpToolsVisibilityByRole, TestMcpResourcesPromptsByRole, TestMcpToolCallByRole, TestMcpScopedTokenPermissions, TestMcpStreamableHttpTransport, TestMcpPerServerEndpoint, TestDenyPaths, TestCrossTransportConsistency). README documents only the 6 protocol classes.
  • Fix: Extend "What's Tested" to cover all 9 RBAC classes. Also fix the "Architecture" diagram — it only shows async MCP SDK path; RBAC classes use Playwright APIRequestContext + sync MCP helpers via _run_async thread-pool.

• docs/docs/development/release-management.md §5.4 table lists deprecated targets as independent suites

  • File: docs/docs/development/release-management.md:386-389
  • Code: Makefile:946-968 — test-mcp-rbac and test-mcp-protocol-e2e are deprecated aliases for test-e2e, not distinct suites.
  • Fix: Remove the two-row table (the command block above already shows make test-e2e), or replace with a note that both targets are deprecated aliases removed in v1.3.0.

---

⚠️ Warnings

• Deprecated Makefile aliases run test suite BEFORE printing deprecation warning, and lose K= filter

  • File: Makefile:946-968
  • Issue: test-mcp-protocol-e2e: test-e2e declares test-e2e as a prerequisite — Make runs it first, then the recipe prints the warning. User sees 15+ min of test output before the deprecation notice. Also, prerequisite dependencies don't inherit $(K), so make test-mcp-rbac K=foo silently ignores the filter. Issue spec §7 required explicit @$(MAKE) delegation.
  • Current:

    test-mcp-protocol-e2e: test-e2e
        $(call deprecated_target,test-mcp-protocol-e2e,make test-e2e,1.3.0)

  • Fix:

    test-mcp-protocol-e2e:
        $(call deprecated_target,test-mcp-protocol-e2e,make test-e2e,1.3.0)
        @$(MAKE) --no-print-directory test-e2e K="$(K)"
    
    test-mcp-cli:
        $(call deprecated_target,test-mcp-cli,make test-e2e,1.3.0)
        @$(MAKE) --no-print-directory test-e2e K="$(K)"
    
    test-mcp-rbac:
        $(call deprecated_target,test-mcp-rbac,make test-e2e,1.3.0)
        @$(MAKE) --no-print-directory test-e2e K="$(K)"

• test-e2e Makefile target omits playwright install step

  • File: Makefile:935-944
  • Original test-mcp-rbac ran @$(UV_BIN) run playwright install --with-deps chromium >/dev/null. New test-e2e drops this — CI/fresh environments will fail with "Executable doesn't exist".
  • Fix: Add @$(UV_BIN) run playwright install --with-deps chromium >/dev/null before the pytest invocation.

---

🔍 Missing Coverage

• TestMcpScopedTokenPermissions has no deny-path: tools.read-only token can execute tools

  • File: tests/live_gateway/e2e/test_e2e.py:1291-1307
  • Class docstring (line 1285) states: token with ["servers.use", "tools.read"] can list tools but not call them. Current tests only call _mcp_initialize_only — never attempt _mcp_tool_call.
  • Fix: Add test_read_only_token_cannot_execute_tools() — call a tool with scoped_token_read_only, assert result.isError is True or McpError raised. This is a security invariant with zero live coverage.

• TestMcpResourcesPromptsByRole — all 4 test methods have zero assertions

  • File: tests/live_gateway/e2e/test_e2e.py:1196-1212
  • All methods call the list helper and print counts only. Outsider should NOT see team-scoped resources — same isolation rule as servers — but this is never checked.
  • Fix: Assert len(outsider_resources) <= len(admin_resources); add cross-team isolation check matching TestServerVisibilityViaAPI.test_outsider_sees_only_public.

• TestMcpToolsVisibilityByRole.test_outsider_sees_only_public_tools — no deny side

  • File: tests/live_gateway/e2e/test_e2e.py:1175-1181
  • Only asserts outsider CAN see public tools; never asserts team-scoped tools are absent. Contrast with TestServerVisibilityViaAPI.test_outsider_sees_only_public (lines 1122-1128) which asserts both include AND exclude.
  • Fix: Add assert not any("team" in n for n in tool_names) or check team server prefix absent.

• tests/e2e/README.md "Architecture" diagram shows only the async path

  • File: tests/e2e/README.md:69-77
  • RBAC classes use playwright.sync_api.APIRequestContext + _run_async thread-pool helpers — not shown.
  • Fix: Add/extend diagram showing Playwright sync path used by RBAC test classes.

---

🧪 Synthetic Tests (assert nothing)

• TestMcpResourcesPromptsByRole.test_admin_resources — prints only, zero asserts (tests/live_gateway/e2e/test_e2e.py:1196-1198)
• TestMcpResourcesPromptsByRole.test_admin_prompts — prints only, zero asserts (tests/live_gateway/e2e/test_e2e.py:1200-1202)
• TestMcpResourcesPromptsByRole.test_developer_resources_and_prompts — calls two helpers, prints counts, asserts nothing (tests/live_gateway/e2e/test_e2e.py:1204-1207)
• TestMcpResourcesPromptsByRole.test_outsider_resources_and_prompts — security-sensitive; verifies only no crash, not that access is restricted (tests/live_gateway/e2e/test_e2e.py:1209-1212)
• TestMcpStreamableHttpTransport.test_streamable_http_resources_discoverable — lists resources, prints count, no assertion (tests/live_gateway/e2e/test_e2e.py:1346-1348)
• TestMcpStreamableHttpTransport.test_streamable_http_prompts_discoverable — same pattern (tests/live_gateway/e2e/test_e2e.py:1350-1352)

---

[msureshkumar88]

Thanks for the detailed pass — went through all four categories against the current tree.

Blocking — fixed in b22666541 (rebase this in before merge):

1. tests/e2e/README.md "What's Tested" now documents all 9 RBAC classes (was only listing the 6 async-protocol ones), and the architecture diagram now shows both the async MCP SDK path and the Playwright sync path.
2. docs/docs/development/release-management.md §5.4 no longer lists test-mcp-rbac/test-mcp-protocol-e2e as independent suites — replaced the stale table with a note that they're deprecated aliases for test-e2e (removed in v1.3.0).

Warnings — already resolved, no action taken:

• Alias ordering / lost K= filter: this was addressed in the earlier round (37ce992) before this comment — test-mcp-protocol-e2e/test-mcp-cli/test-mcp-rbac now delegate via test-e2e as a prerequisite with a single pytest invocation, filter included.
• playwright install --with-deps chromium: checked — the RBAC classes only use playwright.sync_api.APIRequestContext via ApiTestHelper.new_context() (no browser.launch() anywhere in the module), so no browser binary is needed. The install step was correctly dropped; restoring it would just add an unnecessary root-privileged step.

Missing Coverage / Synthetic Tests — valid, but out of scope for this PR:
Checked each cited method (test_admin_resources, test_outsider_resources_and_prompts, test_tools_read_only_token_can_initialize, test_streamable_http_resources_discoverable, etc.) against the pre-migration source (test_mcp_rbac_transport.py on main) — all of them pre-date this PR verbatim. #6517 scopes this issue as a pure migration ("does not add new tests"), so adding assertions/new test cases here would be scope creep on top of the migration. These are legitimate gaps worth tracking — happy to file a follow-up issue for the read-only-token deny-path and the assertion-free resource/prompt/streamable-HTTP tests if that's useful.

[gandhipratik203]

Running the suite twice against the same stack, the second run fails 6 TestToolCalls tests (McpError: Tool not found: fast-time-get-system-time). First run is clean.

streamable_http_gateway deletes any gateway matching http://fast_time_server:9080/mcp (the stack's fast_time) and at teardown deletes only its own, leaving zero gateways — so TestToolCalls, which calls fast-time-* via the global /mcp/, passes once and fails after. Restoring fast_time makes it pass again.

Leave a pre-existing registration alone, or restore it at teardown?

[msureshkumar88]

Confirmed — streamable_http_gateway deletes any gateway matching the fast_time_server URL (needed to dodge the URL-uniqueness check) but only ever removes its own registration at teardown, so the compose-seeded fast_time gateway is gone for good after the first run. Fixed in f36b2a43b: setup now captures the displaced gateway(s) and re-registers them at teardown.

[gandhipratik203]

Verified on a live stack — 3 consecutive full runs on f36b2a43b, all clean (run 2 previously failed 6 TestToolCalls tests). Thanks for the quick fix.

[ja8zyjits]

LGTM