Skip to content

Add exact MCP server policy handler - #40

Merged
Deathcharge merged 3 commits into
mainfrom
codex/mcp-server-policy-handler
Aug 11, 2026
Merged

Add exact MCP server policy handler#40
Deathcharge merged 3 commits into
mainfrom
codex/mcp-server-policy-handler

Conversation

@Deathcharge

Copy link
Copy Markdown
Owner

Summary

  • add exact-registry, fail-closed enforcement for the stable MCP Python SDK low-level server handler
  • add application-owned one-shot review with fingerprint binding and post-review fact refresh
  • pin the patched mcp 1.28.1 graph with hashes and add a real in-memory client/server CI contract
  • document the supported boundary, transport security, deployment guidance, and release process

Security and compatibility

  • the dependency audit caught PYSEC-2026-3483 in the initially evaluated SDK; this PR pins patched mcp 1.28.1
  • MCP annotations and descriptions never become trusted capability labels
  • schema-invalid calls, direct handlers, FastMCP internals, non-tool primitives, and proxy/provider paths are explicitly outside the adapter boundary
  • the dependency-free base wheel still imports with MCP absent

Verification

  • ruff format --check .
  • ruff check .
  • mypy
  • pytest: 563 passed, 95.01% branch-aware coverage
  • exact mcp 1.28.1 SDK contract: 3 passed
  • no-network MCP demo
  • hash-locked clean install and pip check
  • pip-audit: no known vulnerabilities in requirements-mcp.lock
  • Bandit: no findings in the adapter
  • credential-pattern scan: no matches
  • build --no-isolation and twine check
  • clean base-wheel install, optional dependency metadata and CLI smoke

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Deathcharge, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6a6226cf-9311-4dbf-87b6-2cd7702cc28b

📥 Commits

Reviewing files that changed from the base of the PR and between d832b8e and 4eb71bd.

⛔ Files ignored due to path filters (1)
  • requirements-mcp.lock is excluded by !**/*.lock
📒 Files selected for processing (10)
  • README.md
  • RELEASING.md
  • docs/ADOPTION.md
  • docs/API.md
  • docs/ARCHITECTURE.md
  • docs/MCP.md
  • docs/PRODUCTIZATION.md
  • examples/mcp_server_policy_demo.py
  • integration_tests/test_mcp_sdk.py
  • requirements-mcp.txt

Summary by CodeRabbit

  • New Features
    • Added optional MCP Python SDK server integration with protected tool registration and dispatch.
    • Added authorization, schema validation, review approvals, fail-closed handling, and registry consistency checks.
    • Added a runnable in-memory MCP server example.
  • Documentation
    • Added installation, configuration, security, API, architecture, adoption, and production guidance for MCP integration.
  • Tests
    • Added comprehensive no-network coverage for tool listing, authorization, reviews, validation, error handling, and integration behavior.
  • Chores
    • Added the pinned MCP SDK dependency and CI validation contract.

Walkthrough

Adds a pinned MCP Python SDK server adapter with fail-closed tool authorization, review approval, immutable request data, in-memory examples, integration tests, CI validation, public exports, and documentation.

Changes

MCP server adapter

Layer / File(s) Summary
Adapter contract and policy execution
src/samsarix_ethics/mcp.py, src/samsarix_ethics/__init__.py
Adds MCP policy types, immutable review requests, validation, approval handling, fail-closed execution, and public exports.
In-memory integration and adversarial tests
examples/mcp_server_policy_demo.py, integration_tests/test_mcp_sdk.py, tests/test_mcp.py, tests/test_public_api.py
Adds an executable MCP client/server example and tests for registration, authorization outcomes, schema validation, approval integrity, and fail-closed behavior.
Pinned dependency and CI contract
pyproject.toml, requirements-mcp.txt, CONTRIBUTING.md, .github/workflows/ci.yml, RELEASING.md, CHANGELOG.md
Pins mcp==1.28.1 and adds dependency, CI, release, and changelog coverage.
MCP usage and adoption documentation
README.md, docs/MCP.md, docs/API.md, docs/ARCHITECTURE.md, docs/ADOPTION.md, SECURITY.md, ROADMAP.md, docs/PRODUCTIZATION.md, docs/OPENAI_AGENTS.md, docs/PYDANTIC_AI.md, docs/TOOL_CALLS.md, docs/TOOL_DISPATCH.md
Documents setup, enforcement flow, security boundaries, adoption evidence, release status, and supported integration coverage.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant MCPServer
  participant Policy
  participant ToolGate
  Client->>MCPServer: Call registered tool
  MCPServer->>Policy: Validate tool and arguments
  Policy->>ToolGate: Authorize request
  ToolGate-->>Policy: Allow, deny, or review
  Policy->>ToolGate: Execute approved call
  ToolGate-->>MCPServer: Return handler result
  MCPServer-->>Client: Return tool response
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.06% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: adding an exact MCP server policy handler.
Description check ✅ Passed The description explains the change, security and compatibility impact, and verification results; the Summary section covers the template’s What changed section.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/mcp-server-policy-handler

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/PRODUCTIZATION.md`:
- Around line 129-133: Update the MCP tools specification statement in
PRODUCTIZATION.md to replace “human-deniable invocation” with “human in the loop
with the ability to deny tool invocations,” preserving the surrounding meaning
and links.

In `@examples/mcp_server_policy_demo.py`:
- Around line 81-83: Update the result.isError assertion in the policy-protected
MCP call demo to include result.content in the failure message, preserving the
existing success path and assertion behavior.

In `@integration_tests/test_mcp_sdk.py`:
- Around line 124-156: Exercise the unused approve_review=False branch by adding
a real MCP transport test, such as
test_real_server_rejected_review_fails_closed, that calls the tool through
create_connected_server_and_client_session and asserts result.isError is true.
Also verify the handler was not called and exactly one review was recorded;
alternatively, remove the False branch from _server if rejection is
intentionally out of scope.

In `@README.md`:
- Around line 637-640: Update the MCP registry documentation to describe
name-set matching rather than full Tool-definition equality: in README.md lines
637-640, state that create_mcp_server_tool_policy exact-matches the complete
tool-name set; in docs/ADOPTION.md lines 28-30, state that the adapter
exact-matches the complete MCP tool-name set; in docs/API.md lines 469-471,
clarify that Tool objects are validated and copied while catalog matching uses
their names; and in docs/ARCHITECTURE.md lines 289-291, describe exact matching
of the registry name set.

In `@RELEASING.md`:
- Around line 68-76: Update the fourth fresh virtual environment instructions in
RELEASING.md to install the local package after the requirements-mcp.lock
installation and before the MCP test and example commands, using the specified
editable, no-build-isolation, no-dependencies pip invocation.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7dbcc6bb-a618-4ea8-8f3c-5a2c2bd250dd

📥 Commits

Reviewing files that changed from the base of the PR and between b7c00b1 and d832b8e.

⛔ Files ignored due to path filters (1)
  • requirements-mcp.lock is excluded by !**/*.lock
📒 Files selected for processing (24)
  • .github/workflows/ci.yml
  • CHANGELOG.md
  • CONTRIBUTING.md
  • README.md
  • RELEASING.md
  • ROADMAP.md
  • SECURITY.md
  • docs/ADOPTION.md
  • docs/API.md
  • docs/ARCHITECTURE.md
  • docs/MCP.md
  • docs/OPENAI_AGENTS.md
  • docs/PRODUCTIZATION.md
  • docs/PYDANTIC_AI.md
  • docs/TOOL_CALLS.md
  • docs/TOOL_DISPATCH.md
  • examples/mcp_server_policy_demo.py
  • integration_tests/test_mcp_sdk.py
  • pyproject.toml
  • requirements-mcp.txt
  • src/samsarix_ethics/__init__.py
  • src/samsarix_ethics/mcp.py
  • tests/test_mcp.py
  • tests/test_public_api.py

Comment thread docs/PRODUCTIZATION.md
Comment thread examples/mcp_server_policy_demo.py
Comment thread integration_tests/test_mcp_sdk.py
Comment thread README.md Outdated
Comment thread RELEASING.md
@Deathcharge
Deathcharge merged commit a99233a into main Aug 11, 2026
11 checks passed
@Deathcharge
Deathcharge deleted the codex/mcp-server-policy-handler branch August 11, 2026 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant