Repository navigation
fix(mcp): stop masking the agent's own arguments in tool error text - #1366
Conversation
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1791849842-agent-error-no-arg-masking' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1791849842-agent-error-no-arg-masking'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation matches the stated behavior and preserves existing non-argument redaction and telemetry protections.
0 open findings
What changed in this PR
Updates MCP agent-facing errors to preserve tool argument values while retaining credential and URL redaction.
Changes:
- Removes argument-aware masking and related dead code.
- Updates error middleware calls and tests for raw argument passthrough.
- Confirms telemetry redaction remains unchanged.
| File | Description |
|---|---|
airbyte/mcp/_error_handling.py |
Simplifies agent-text redaction to pattern-based safeguards. |
tests/unit_tests/test_mcp_tool_errors.py |
Updates argument-passthrough coverage. |
tests/unit_tests/test_mcp_datadog.py |
Verifies passthrough does not affect telemetry privacy. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code Coverage OverviewLanguages: Python Python / code-coverage/pytest-no-credsThe overall line coverage in commit 5c84ee3 in the Show a line coverage summary of the most impacted files.
Python / code-coverage/pytestThe overall line coverage in commit 5c84ee3 in the Show a line coverage summary of the most impacted files.
Python / code-coverage/pytest-unitThe overall line coverage in commit 5c84ee3 in the Show a line coverage summary of the most covered files.
Updated |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
5577ff7
into
release-candidate/v1
Summary
AgentErrorTextMiddlewareno longer masks tool-argument values in the error text the agent sees. The agent sent those arguments itself, so masking them protects nothing, and it broke useful output. For example, connector links in guidance came back as/source/<value of source_id>. Span/telemetry arg tracing is unchanged.connection_idin an error<value of connection_id>Bearertokens, URL credentials,input_value=echoesChanges
redact_agent_text(text, arguments, tool)becomesredact_agent_text(text), which keeps only the four pattern redactions._collect_leaves,_secret_leaf,_allowed_choices,_JSON_LITERALS,_call_arguments, and theclassify_tool/declared_parameters/TraceArgimports.<value of ...>now assert the raw value passes through. Added a test that an error containing a connector ID and Cloud URL comes back unchanged.Review Spotlight
The middleware still redacts text the agent did not send, such as server-side bearer tokens, URL userinfo and signed-URL query strings.
Stack
Risks / Follow-ups
airbyte/mcp/_error_handling.pyis a private module, so no public API changes.Link to Devin session: https://app.devin.ai/sessions/bb884d35f6d24ad993d7f494e660acbc
Open in Devin Desktop: https://app.devin.ai/desktop/session/bb884d35f6d24ad993d7f494e660acbc?variant=devin
Requested by: Aaron ("AJ") Steers (@aaronsteers)