refactor(mcp): relocate tool result envelope into core to clear a layering back-edge#381
Merged
Merged
Conversation
core/tool_middleware.short_circuit_error imported tools/tool_error_envelope, a back-edge against the inward-only layering where tools sits above core. The error/success envelope is the shared result currency used both by the tool adapters and by the core middleware, so it belongs at core, not in tools. Move the module to core/tool_error_envelope.py (and its test to tests/core/), repoint every importer, and hoist the previously lazy import in short_circuit_error now that relocating the module removes the load-time cycle that forced it. No behavior change: the module content is untouched. This clears the pipefy_mcp.core -> pipefy_mcp.tools edge so the intra-package layering contract holds.
gbrlcustodio
added a commit
that referenced
this pull request
Jul 9, 2026
PR #381 relocated tool_error_envelope.py from tools/ to core/. Refresh the two spots that still described it under tools/: the domain mapping in docs/architecture.md and the disabled-ratchet comment in pyproject.toml. Also name core/tool_middleware.py alongside fastmcp_tool_lifecycle.py as adapter code that currently lives under core/.
adriannoes
added a commit
that referenced
this pull request
Jul 9, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Moves
tool_error_envelopefromtools/tocore/sopipefy_mcp.coreno longer importspipefy_mcp.tools.Why
core/tool_middleware.py(short_circuit_error) importedtools/tool_error_envelope. That is a back-edge against the intended inward-only layering, wheretools(an adapter) sits abovecore. #378 introduced the edge when the tool-call middleware landed on dev. The error/success envelope is the shared result currency used both by the tool adapters and by the core middleware, so its home iscore, nottools.Changes
git mvthe module tocore/tool_error_envelope.pyand its unit test totests/core/.pipefy_mcp.tools.tool_error_envelopetopipefy_mcp.core.tool_error_envelope.short_circuit_errorto a top-level import: relocating the module removes the load-time cycle that forced the lazy form.Relationship to #380
The intra-package import-linter contract lives in #380, not on dev. This fix is what makes that contract pass once #380 rebases onto the updated dev: I verified locally by transiently applying #380's contract (plus its
core/__init__.py) and runninglint-imports, which reports 1 kept, 0 broken. Two prose spots in #380 still describe the envelope as living undertools/(the layer-model mapping and the disabled-ratchet comment); those should be refreshed on that branch after this merges.Testing
uv run python -m pytest -m 'not integration'inpackages/mcp: 1436 passed, 5 skipped.uv run ruff check packages/mcpandruff format --check: clean.