fix(mcp): remove status from dd_incidents_update, add needs_wrench_bot_check#17
Merged
Merged
Conversation
…t_check dd_incidents_update accepted a status param that silently did nothing — the Datadog API ignores status on the incident update endpoint. Callers must use dd_incidents_set_status instead. Changes: - Add explicit docstring warning in dd_incidents_update that status must go through dd_incidents_set_status - Fix two misleading docstrings that told callers to use dd_incidents_update for status transitions (dd_incidents_create and dd_incidents_delete) - Add needs_wrench_bot_check param to dd_incidents_update MCP tool, matching the CLI flag added in v0.9.12 - Expand arg descriptions in dd_incidents_update to be actionable rather than just restating the param name - Mention NeedsWrenchBotCheck in dd_incidents_get field list
Kydoimos97
requested review from
a team,
Wrench-Service-Bot and
Copilot
and removed request for
Copilot
Wrench-Service-Bot
approved these changes
Jun 10, 2026
Wrench-Service-Bot
left a comment
Wrench-Service-Bot
left a comment
There was a problem hiding this comment.
PR Review
Verdict: APPROVED
Description
Corrects three misleading docstrings that directed callers to use dd_incidents_update for status transitions (which silently does nothing), and wires the needs_wrench_bot_check field into the MCP tool to match the CLI flag added in v0.9.12.
Highlights
- The
dd_incidents_deletefix is the highest-impact change: the old docstring explicitly told callersdd_incidents_update(status='resolved')was how to close an incident — a silent no-op that would leave incidents stuck open. - The
dd_incidents_createdocstring fix closes the same footgun in the creation flow. needs_wrench_bot_checkpassthrough follows the existingis_duplicate/triage_completedpattern exactly.- Arg descriptions are now actionable rather than restating the param name — a clear improvement for AI consumers of this tool.
Style Notes
- None
Overall Assessment
Clean, targeted fix with no risk of regression. The docstring corrections align with the documented behavior of the Datadog v2 API (status transitions require a separate endpoint). Ready to merge.
Sources
- Diff: 1 file, +22/-15
- Prior reviews: none
- Related PRs/issues: N/A
— Claude
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
2 participants
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.
Summary
dd_incidents_updatecan change incident status — it cannot, the Datadog API ignores status on the update endpointdd_incidents_updatefor status transitions (dd_incidents_createanddd_incidents_delete)dd_incidents_updatedocstring: usedd_incidents_set_statusfor status changesneeds_wrench_bot_checkintodd_incidents_updateMCP tool (matching the CLI flag added in v0.9.12)dd_incidents_updateto be actionable rather than restating the param nameNeedsWrenchBotCheckto the field list indd_incidents_getdocstringRoot cause
dd_incidents_updatehad astatusparam in older versions (and in confusing docstrings) that silently had no effect. The dedicateddd_incidents_set_statustool exists precisely because the Datadog v2 API separates status transitions from field updates. This PR aligns the documentation with that reality.Test plan
uv run pytest tests/ -m "not integration" -q— 561 passedEnd-user impact
AI agents calling
dd_incidents_update(status="resolved")will now see clear documentation that this does nothing and they needdd_incidents_set_statusinstead. Eliminates silent no-ops where status appears to be set but isn't.