develop-aditya-2 #382

Closed
aditya wants to merge 3 commits from develop-aditya-2 into master
Member

Metadata

  • Commit Message: feat(skill): add MCP adapter for external tools
  • Branch: develop-aditya-2

Background

This PR implements the MCP (Model Context Protocol) adapter for external tool integration as part of Milestone 2 (v3.1.0). It enables CleverAgents to discover and route external tools via the MCP protocol.

Related issue: #159

Expected Behavior

The MCP adapter can connect to external MCP servers, discover available tools, and route tool invocations through the skill system. Tool results are properly marshalled back to the actor execution pipeline.

Acceptance Criteria

  • MCP adapter connects to external MCP servers
  • Tool discovery works via MCP protocol
  • Tool invocations route correctly through skill system
  • Tool results marshal back to actor pipeline
  • Existing tests pass with no regressions
  • Coverage remains >=97%

Definition of Done

This PR is complete when:

  • All acceptance criteria above are met.
  • Code review is approved.
  • CI passes (nox full suite, coverage >=97%).
  • PR is merged to master.
## Metadata - **Commit Message**: `feat(skill): add MCP adapter for external tools` - **Branch**: `develop-aditya-2` ## Background This PR implements the MCP (Model Context Protocol) adapter for external tool integration as part of Milestone 2 (v3.1.0). It enables CleverAgents to discover and route external tools via the MCP protocol. Related issue: #159 ## Expected Behavior The MCP adapter can connect to external MCP servers, discover available tools, and route tool invocations through the skill system. Tool results are properly marshalled back to the actor execution pipeline. ## Acceptance Criteria - [ ] MCP adapter connects to external MCP servers - [ ] Tool discovery works via MCP protocol - [ ] Tool invocations route correctly through skill system - [ ] Tool results marshal back to actor pipeline - [ ] Existing tests pass with no regressions - [ ] Coverage remains >=97% ## Definition of Done This PR is complete when: - All acceptance criteria above are met. - Code review is approved. - CI passes (nox full suite, coverage >=97%). - PR is merged to master.
feat(skill): add MCP adapter for external tools
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 21s
CI / security (pull_request) Successful in 33s
CI / typecheck (pull_request) Successful in 47s
CI / integration_tests (pull_request) Successful in 2m54s
CI / unit_tests (pull_request) Successful in 6m13s
CI / docker (pull_request) Successful in 39s
CI / benchmark-regression (pull_request) Successful in 19m58s
CI / coverage (pull_request) Failing after 22m0s
a6e56fe96e
freemo added this to the v3.1.0 milestone 2026-02-23 17:26:21 +00:00
Merge branch 'master' into develop-aditya-2
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
CI / build (pull_request) Successful in 17s
CI / quality (pull_request) Successful in 20s
CI / typecheck (pull_request) Successful in 47s
CI / security (pull_request) Successful in 50s
CI / integration_tests (pull_request) Successful in 3m41s
CI / unit_tests (pull_request) Successful in 7m9s
CI / docker (pull_request) Successful in 38s
CI / benchmark-regression (pull_request) Successful in 20m12s
CI / coverage (pull_request) Successful in 24m15s
80a56846fe
# Conflicts:
#	implementation_plan.md
brent.edwards left a comment

This passes all tests, and it's been merged with master since Jeff's changes. So I approve it.

This passes all tests, and it's been merged with master since Jeff's changes. So I approve it.
freemo requested changes 2026-02-24 22:27:25 +00:00
Dismissed
freemo left a comment

Code Review: PR #382develop-aditya-2

This PR has several structural and process issues that prevent it from meeting the project's merge requirements. Additionally, it appears to overlap significantly with PR #416 (which contains the same MCP adapter and actor hierarchy code).

Critical Issues

1. Non-descriptive PR title (CONTRIBUTING.md §5)
The title "develop-aditya-2" is simply a branch name and does not follow the Conventional Changelog format. The title should describe the change (e.g., feat(mcp): add MCP adapter for external tools).

2. Wrong branch name (issue #159 mismatch)
Issue #159 specifies the branch should be feature/m3-mcp-adapter, but this PR uses develop-aditya-2. Per the issue's Definition of Done: "The commit is pushed to the remote on the branch matching the Branch in Metadata exactly."

3. Multiple unrelated features bundled (CONTRIBUTING.md §2)
This PR bundles two independent features with no code dependency between them:

  • MCP Tool Adapter (src/cleveragents/mcp/*, features, robot tests, docs, benchmarks) — 7 files
  • Hierarchical Actor YAML Schema Extensions (src/cleveragents/actor/schema.py, features, robot tests, docs, benchmarks) — 8 files

These should be separate PRs, each scoped to one issue.

4. Overlap with PR #416
This PR contains the same MCP adapter and actor hierarchy code that is also in PR #416. This duplication needs to be resolved — the work should exist in exactly one PR.

5. Improper labels on PR (CONTRIBUTING.md §12)
This PR carries MoSCoW/Must have, Points/8, and enhancement labels. Per guidelines:

  • "No other label scopes (State/, Priority/, MoSCoW/) are applied to Pull Requests; those scopes are reserved for issues only."
  • enhancement is not part of the defined label system. PRs should have exactly one Type/ label.
  • Points/ labels are not defined in CONTRIBUTING.md and should be on issues only.

6. No issue references in commit footers (CONTRIBUTING.md §4)
Neither commit has a Refs:, Closes:, or ISSUES CLOSED: reference in its footer. All commits must reference their associated issues.

7. No CHANGELOG update (CONTRIBUTING.md §6) — Two new features with no CHANGELOG entries.

8. No CONTRIBUTORS.md update (CONTRIBUTING.md §8)

Code Quality Issues

9. Mock placement violation (CONTRIBUTING.md — Mock Placement)
MockMCPTransport is embedded inline in features/steps/mcp_adapter_steps.py and robot/helper_mcp_adapter.py has its own _MockTransport. Per guidelines: "ALL mocks, test doubles, and mock implementations must exist only in the features/ directory" — specifically features/mocks/. Extract shared mocks to features/mocks/mock_mcp_transport.py.

10. registry: Any type annotation
MCPToolAdapter.register_tools() uses registry: Any instead of the concrete type. Use a TYPE_CHECKING forward reference.

11. Constructor validation gap
MCPToolAdapter.__init__ does not validate transport configuration (stdio requires command, sse requires url) — only from_server_config does. Direct constructor callers skip validation.

12. Misleading commit scope
Commit a6e56fe uses feat(skill) scope but the feature is an MCP adapter under src/cleveragents/mcp/. The scope should be feat(mcp).

  1. Close this PR and create properly scoped PRs from correctly named branches matching the issue metadata.
  2. For the MCP adapter: use branch feature/m3-mcp-adapter as specified in issue #159.
  3. For the actor hierarchy: create a separate issue and branch.
  4. Extract mocks to features/mocks/.
  5. Add CHANGELOG, CONTRIBUTORS.md entries.
  6. Fix labels to use only Type/Feature.
## Code Review: PR #382 — `develop-aditya-2` This PR has several structural and process issues that prevent it from meeting the project's merge requirements. Additionally, it appears to overlap significantly with PR #416 (which contains the same MCP adapter and actor hierarchy code). ### Critical Issues **1. Non-descriptive PR title (CONTRIBUTING.md §5)** The title "develop-aditya-2" is simply a branch name and does not follow the Conventional Changelog format. The title should describe the change (e.g., `feat(mcp): add MCP adapter for external tools`). **2. Wrong branch name (issue #159 mismatch)** Issue #159 specifies the branch should be `feature/m3-mcp-adapter`, but this PR uses `develop-aditya-2`. Per the issue's Definition of Done: "The commit is pushed to the remote on the branch matching the Branch in Metadata exactly." **3. Multiple unrelated features bundled (CONTRIBUTING.md §2)** This PR bundles **two independent features** with no code dependency between them: - **MCP Tool Adapter** (`src/cleveragents/mcp/*`, features, robot tests, docs, benchmarks) — 7 files - **Hierarchical Actor YAML Schema Extensions** (`src/cleveragents/actor/schema.py`, features, robot tests, docs, benchmarks) — 8 files These should be separate PRs, each scoped to one issue. **4. Overlap with PR #416** This PR contains the same MCP adapter and actor hierarchy code that is also in PR #416. This duplication needs to be resolved — the work should exist in exactly one PR. **5. Improper labels on PR (CONTRIBUTING.md §12)** This PR carries `MoSCoW/Must have`, `Points/8`, and `enhancement` labels. Per guidelines: - "No other label scopes (`State/`, `Priority/`, `MoSCoW/`) are applied to Pull Requests; those scopes are reserved for issues only." - `enhancement` is not part of the defined label system. PRs should have exactly one `Type/` label. - `Points/` labels are not defined in CONTRIBUTING.md and should be on issues only. **6. No issue references in commit footers (CONTRIBUTING.md §4)** Neither commit has a `Refs:`, `Closes:`, or `ISSUES CLOSED:` reference in its footer. All commits must reference their associated issues. **7. No CHANGELOG update (CONTRIBUTING.md §6)** — Two new features with no CHANGELOG entries. **8. No CONTRIBUTORS.md update (CONTRIBUTING.md §8)** ### Code Quality Issues **9. Mock placement violation (CONTRIBUTING.md — Mock Placement)** `MockMCPTransport` is embedded inline in `features/steps/mcp_adapter_steps.py` and `robot/helper_mcp_adapter.py` has its own `_MockTransport`. Per guidelines: "ALL mocks, test doubles, and mock implementations must exist only in the `features/` directory" — specifically `features/mocks/`. Extract shared mocks to `features/mocks/mock_mcp_transport.py`. **10. `registry: Any` type annotation** `MCPToolAdapter.register_tools()` uses `registry: Any` instead of the concrete type. Use a `TYPE_CHECKING` forward reference. **11. Constructor validation gap** `MCPToolAdapter.__init__` does not validate transport configuration (stdio requires `command`, sse requires `url`) — only `from_server_config` does. Direct constructor callers skip validation. **12. Misleading commit scope** Commit `a6e56fe` uses `feat(skill)` scope but the feature is an MCP adapter under `src/cleveragents/mcp/`. The scope should be `feat(mcp)`. ### Recommended Path Forward 1. Close this PR and create properly scoped PRs from correctly named branches matching the issue metadata. 2. For the MCP adapter: use branch `feature/m3-mcp-adapter` as specified in issue #159. 3. For the actor hierarchy: create a separate issue and branch. 4. Extract mocks to `features/mocks/`. 5. Add CHANGELOG, CONTRIBUTORS.md entries. 6. Fix labels to use only `Type/Feature`.
freemo dismissed freemo's review 2026-02-24 22:45:20 +00:00
Reason:

dont want hard block

aditya closed this pull request 2026-02-25 14:19:43 +00:00
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
Required
Details
CI / build (pull_request) Successful in 17s
Required
Details
CI / quality (pull_request) Successful in 20s
Required
Details
CI / typecheck (pull_request) Successful in 47s
Required
Details
CI / security (pull_request) Successful in 50s
Required
Details
CI / integration_tests (pull_request) Successful in 3m41s
Required
Details
CI / unit_tests (pull_request) Successful in 7m9s
Required
Details
CI / docker (pull_request) Successful in 38s
Required
Details
CI / benchmark-regression (pull_request) Successful in 20m12s
CI / coverage (pull_request) Successful in 24m15s
Required
Details

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!382
No description provided.