fix: enhance tool generation and execution with improved flag handling and logging #1982
Conversation
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
…g and logging - Add ItemsType to FlagDef for JSON schema array items. - Filter parameters in handleConsolidatedTool to prevent unknown flag errors. - Implement detailed logging for command execution and failures. - Introduce tests for array parameters and excluded commands. - Update ToolOptions to include a logger for debug logging. Signed-off-by: Nikolai Emil Damm <nikolaiemildamm@icloud.com>
Contributor
✅
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…es; optimize component count logic Signed-off-by: Nikolai Emil Damm <nikolaiemildamm@icloud.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens MCP tool generation and execution by fixing JSON schema array definitions, excluding meta commands and their children from tool generation, and adding structured logging to command execution, while also tuning installer timeouts.
Changes:
- Ensure all array-typed tool parameters include proper JSON Schema
itemsdefinitions and add tests to guard against regressions, fixing MCP validation errors. - Improve consolidated tool handling by filtering parameters to only those applicable to the chosen subcommand and by excluding meta commands (e.g.,
ksail completion,ksail mcp) and their children from tool generation. - Wire an optional
slog.Loggerthrough MCP server and tool execution for detailed debug/error logging, and introduce a dedicated cert-manager install timeout, plus a small cleanup to component counting.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
pkg/svc/mcp/server.go |
Passes the configured logger into toolgen options so MCP-executed commands emit structured debug/error logs. |
pkg/svc/installer/helpers.go |
Adds a dedicated CertManagerInstallTimeout constant to allow cert‑manager installs extra time. |
pkg/cli/setup/post_cni.go |
Simplifies ComponentRequirements.Count() using a boolean slice while still counting all component flags, including ArgoCD and Flux. |
pkg/cli/setup/components.go |
Centralizes installer error variables (including CSI and policy engine) and updates the cert‑manager factory to use the new minimum timeout while keeping test expectations intact. |
pkg/ai/toolgen/options.go |
Extends ToolOptions with an optional *slog.Logger used during command execution, keeping default options unchanged aside from the new field. |
pkg/ai/toolgen/mcp.go |
Enhances MCP error responses to include both command output and the wrapped error (with exit code), improving diagnosability for failed tool calls. |
pkg/ai/toolgen/generator.go |
Adds prefix-based exclusion of commands and their children (e.g., ksail completion …), introduces ItemsType for flags, and ensures consolidated-tool schemas emit array types with proper items. |
pkg/ai/toolgen/definition.go |
Extends FlagDef with an ItemsType field so array flags can carry their element JSON Schema type into consolidated schemas. |
pkg/ai/toolgen/executor.go |
Filters consolidated-tool parameters to only those flags defined for the selected subcommand to avoid “unknown flag” errors, and adds structured logging around command execution and failures. |
pkg/ai/toolgen/generator_test.go |
Adds regression tests verifying all array parameters define items with allowed types and that excluded commands (chat, mcp, completion, help) and their children are not generated as tools. |
pkg/ai/toolgen/executor_test.go |
Extends consolidated execution tests to assert that flags not applicable to the chosen subcommand are filtered out, preventing MCP-driven invocations from failing on inapplicable defaults. |
…nce array parameter validation in tests Signed-off-by: Nikolai Emil Damm <nikolaiemildamm@icloud.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
github-project-automation
bot
moved this from 🏃🏻♂️ In Progress
to 🚀 In Finalization
in 🌊 Project Board
…istency Signed-off-by: Nikolai Emil Damm <nikolaiemildamm@icloud.com>
Signed-off-by: Nikolai Emil Damm <nikolaiemildamm@icloud.com>
github-project-automation
bot
moved this from 🚀 In Finalization
to ✅ Done
in 🌊 Project Board
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.
Enhancements include improved flag handling to prevent unknown flag errors and detailed logging for command execution and failures. Tests for array parameters and excluded commands were also introduced to ensure robustness.
Type of change