Add 'get_network_routes' tool#404
Conversation
|
For team members: test commit |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/retest |
|
For team members: test commit |
|
For team members: test commit |
owtaylor
left a comment
owtaylor
left a comment
There was a problem hiding this comment.
I think there are a lot of other possible tools of roughly this priority, but I don't think it hurts to go ahead and add this to the fixed toolset. It does seem to use a pattern that I think we're trying to move away from.
| if is_successful_output(returncode, stdout): | ||
| routes = parse_ip_route(stdout) | ||
| return format_routes(routes) |
There was a problem hiding this comment.
My understanding is that we're trying to move away from the parse => format path - either the tool output is well understood by the model and we're only going to make it worse by parsing and reformatting it, or we parse and return it in JSON form - perhaps with an idea of supporting "code mode" programmatic use of MCP servers in the future.
There was a problem hiding this comment.
Yea, I think I did the format pattern to follow the structure of the rest of network.py, since that file hasn't been converted to structured output. It didn't seem right to convert network.py to structured output as part of this PR. I can refactor get_network_routes to return structured output though, since it's a net-new tool.
There was a problem hiding this comment.
I opened #517 to convert all the network tools to structured output.
- Switch `ip route` to `ip -json route` for structured output - Update Route model with validation aliases matching JSON fields - Remove text parsing and formatting functions - Change get_network_routes to return list[Route] instead of formatted string - Raise ToolError on failure instead of returning error text - Add "fixed" tag to indicate stable tool interface
3 participants
Add a fixed 'get_network_routes' tool. Inspired by the now stale #263.