feat(mcp): add a dry run to kill_port - #67
Merged
Merged
Conversation
The preview resolves its targets through the same code the real call uses, so the two cannot disagree. It also reports the action that would actually be taken: Windows ignores force and always terminates hard, so promising a SIGTERM there would be a lie. kill_port stays withheld under --read-only, dry run included: that server's contract is that no destructive tool is listed at all.
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
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.
First of Tier 3.
kill_portis the one MCP tool with consequences, and an agent had no way to check what it was about to do —inspect_portanswers a different question, since it lists every process on the port including non-listeners, while a kill targets only listeners with a resolvable PID, deduplicated across tcp/tcp6.The preview cannot disagree with the kill
That is the one bug a dry run must not have, so target resolution moved into
kill_targets(port)and both paths call it. A preview computed any other way could drift from what actually gets signalled.It reports what would really happen
Windows has no signal equivalent —
kill_processignoresforceand always callsTerminateProcess. A dry run reportingSIGTERMthere would promise a graceful shutdown that cannot happen, soplanned_kill_actionmirrors the platform split:force=falseforce=trueSIGTERMSIGKILLTerminateProcessTerminateProcessStill withheld under
--read-onlyA dry run is read-only in effect, so allowing it there was tempting. I did not: that server's contract is that no destructive tool is listed, and a
kill_portin the list that refuses half its arguments is a worse contract than an absent one.inspect_portalready answers what is on the port.Verification
Driving the real server over stdio, against a live listener:
The dry run named pid 7 and the real call signalled pid 7 — same target, which is the point.
One test asserts the description advertises
dry_run: the description is what the model actually reads, and an undiscoverable preview is no safer than none.185 tests, fmt/clippy clean, macOS + Windows type-check.
Next in Tier 3: process tree in
inspect_port, and a change-diff tool.