From d02a3eb58b2937ec68c5b4f455937ef20c50a9ee Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 16 Sep 2026 12:46:10 +0530 Subject: [PATCH 1/2] =?UTF-8?q?refactor(tool):=20migrate=20Jobs/LSP/ToolSe?= =?UTF-8?q?arch/Toolset/Worktree=C3=972/PowerShell=20to=20typed=20schemas?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-source Schema()+Parameters() pattern, Execute parses via DecodeInput[T], wire-shape parity tests covering enums, bools, numbers, and required arrays. --- internal/tool/jobs_tool.go | 67 +++++++++------------- internal/tool/lsp.go | 54 +++++++++++------- internal/tool/powershell.go | 44 ++++++++------- internal/tool/schema_batch_test.go | 89 ++++++++++++++++++++++++++++++ internal/tool/tool_search.go | 36 ++++++++---- internal/tool/toolset_tool.go | 45 ++++++++------- internal/tool/worktree.go | 66 +++++++++++++++------- 7 files changed, 269 insertions(+), 132 deletions(-) diff --git a/internal/tool/jobs_tool.go b/internal/tool/jobs_tool.go index dd494f685..799b8cc6b 100644 --- a/internal/tool/jobs_tool.go +++ b/internal/tool/jobs_tool.go @@ -44,49 +44,34 @@ func (JobsTool) Description() string { "session id: a session sees its own jobs plus every unowned job." } -func (JobsTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "action": map[string]interface{}{ - "type": "string", - "enum": []interface{}{"list", "run", "read", "wait", "kill"}, - "description": "Operation to perform", - }, - "command": map[string]interface{}{ - "type": "string", - "description": "Shell command to run in the background (required for run)", - }, - "label": map[string]interface{}{ - "type": "string", - "description": "One-line model-facing label for the job (defaults to the command)", - }, - "session": map[string]interface{}{ - "type": "string", - "description": "Owner session id. Jobs owned by a session are visible only to it; omitted jobs are unowned and visible to any caller", - }, - "id": map[string]interface{}{ - "type": "string", - "description": "Job id, e.g. bash-3 (required for read/wait/kill)", - }, - "reason": map[string]interface{}{ - "type": "string", - "description": "Kill reason forwarded verbatim to the job (default: 'user requested')", - }, - "timeout_sec": map[string]interface{}{ - "type": "integer", - "description": "Max seconds to wait before returning the current snapshot (default: 60)", - }, - "output_limit": map[string]interface{}{ - "type": "integer", - "description": "UTF-8 byte cap for stored output (default 100000)", - }, +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (JobsTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "action": {Type: "string", Enum: []interface{}{"list", "run", "read", "wait", "kill"}, Description: "Operation to perform"}, + "command": {Type: "string", Description: "Shell command to run in the background (required for run)"}, + "label": {Type: "string", Description: "One-line model-facing label for the job (defaults to the command)"}, + "session": {Type: "string", Description: "Owner session id. Jobs owned by a session are visible only to it; omitted jobs are unowned and visible to any caller"}, + "id": {Type: "string", Description: "Job id, e.g. bash-3 (required for read/wait/kill)"}, + "reason": {Type: "string", Description: "Kill reason forwarded verbatim to the job (default: 'user requested')"}, + "timeout_sec": {Type: "integer", Description: "Max seconds to wait before returning the current snapshot (default: 60)"}, + "output_limit": {Type: "integer", Description: "UTF-8 byte cap for stored output (default 100000)"}, }, - "required": []string{"action"}, + Required: []string{"action"}, } } -type jobsInput struct { +func (JobsTool) Parameters() map[string]interface{} { + return jobsSchema.ToJSONSchema() +} + +// jobsSchema is the single source of truth for Jobs' input schema. +var jobsSchema = JobsTool{}.Schema() + +// JobsInput is the typed input for JobsTool. +type JobsInput struct { Action string `json:"action"` Command string `json:"command"` Label string `json:"label"` @@ -112,8 +97,8 @@ type jobsSnapshotJSON struct { } func (JobsTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p jobsInput - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[JobsInput]("Jobs", input) + if err != nil { return "", err } if p.Action == "" { diff --git a/internal/tool/lsp.go b/internal/tool/lsp.go index 19f5be997..78ceb0a54 100644 --- a/internal/tool/lsp.go +++ b/internal/tool/lsp.go @@ -15,40 +15,54 @@ import ( ) type LSPTool struct { + + // placeholder not used Manager *lsp.LSPManager } +// LSPInput is the typed input for LSPTool. +type LSPInput struct { + Action string `json:"action"` + Path string `json:"path"` + Line int `json:"line"` + Column int `json:"column"` + Symbol string `json:"symbol"` + Root string `json:"root"` +} + func (LSPTool) Name() string { return "LSP" } func (LSPTool) Aliases() []string { return []string{"lsp"} } func (LSPTool) Description() string { return "Get code intelligence through configured language servers, with codegraph and local-tool fallbacks." } -func (LSPTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "action": map[string]interface{}{"type": "string", "enum": []string{"diagnostics", "definition", "references", "implementations"}, "description": "LSP action"}, - "path": map[string]interface{}{"type": "string", "description": "File path"}, - "line": map[string]interface{}{"type": "integer", "description": "Line number (1-based)"}, - "column": map[string]interface{}{"type": "integer", "description": "Column number (1-based)"}, - "symbol": map[string]interface{}{"type": "string", "description": "Symbol name to look up (alternative to line/column)"}, - "root": map[string]interface{}{"type": "string", "description": "Project root directory (default: current dir)"}, +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (LSPTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "action": {Type: "string", Enum: []interface{}{"diagnostics", "definition", "references", "implementations"}, Description: "LSP action"}, + "path": {Type: "string", Description: "File path"}, + "line": {Type: "integer", Description: "Line number (1-based)"}, + "column": {Type: "integer", Description: "Column number (1-based)"}, + "symbol": {Type: "string", Description: "Symbol name to look up (alternative to line/column)"}, + "root": {Type: "string", Description: "Project root directory (default: current dir)"}, }, - "required": []string{"action", "path"}, + Required: []string{"action", "path"}, } } +func (LSPTool) Parameters() map[string]interface{} { + return lspSchema.ToJSONSchema() +} + +// lspSchema is the single source of truth for LSP's input schema. +var lspSchema = LSPTool{}.Schema() + func (t LSPTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Action string `json:"action"` - Path string `json:"path"` - Line int `json:"line"` - Column int `json:"column"` - Symbol string `json:"symbol"` - Root string `json:"root"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[LSPInput]("LSP", input) + if err != nil { return "", err } diff --git a/internal/tool/powershell.go b/internal/tool/powershell.go index d6ec8ab8b..27d713d9c 100644 --- a/internal/tool/powershell.go +++ b/internal/tool/powershell.go @@ -14,6 +14,12 @@ import ( // PowerShellTool executes PowerShell commands (Windows/cross-platform pwsh). type PowerShellTool struct{} +// PowerShellInput is the typed input for PowerShellTool. +type PowerShellInput struct { + Command string `json:"command"` + Timeout int64 `json:"timeout"` +} + func (PowerShellTool) Name() string { return "PowerShell" } func (PowerShellTool) RiskLevel() string { return "high" } func (PowerShellTool) Aliases() []string { return []string{"powershell"} } @@ -21,29 +27,29 @@ func (PowerShellTool) Description() string { return "Execute a PowerShell command. Use this instead of Bash when running on Windows or when PowerShell-specific cmdlets are needed." } -func (PowerShellTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "command": map[string]interface{}{ - "type": "string", - "description": "The PowerShell command to execute", - }, - "timeout": map[string]interface{}{ - "type": "number", - "description": "Timeout in milliseconds (max 600000, default 120000)", - }, +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (PowerShellTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "command": {Type: "string", Description: "The PowerShell command to execute"}, + "timeout": {Type: "number", Description: "Timeout in milliseconds (max 600000, default 120000)"}, }, - "required": []string{"command"}, + Required: []string{"command"}, } } +func (PowerShellTool) Parameters() map[string]interface{} { + return powershellSchema.ToJSONSchema() +} + +// powershellSchema is the single source of truth for PowerShell's input schema. +var powershellSchema = PowerShellTool{}.Schema() + func (PowerShellTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Command string `json:"command"` - Timeout int64 `json:"timeout"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[PowerShellInput]("PowerShell", input) + if err != nil { return "", err } if p.Command == "" { @@ -78,7 +84,7 @@ func (PowerShellTool) Execute(ctx context.Context, input json.RawMessage) (strin cmd.Stdout = &stdout cmd.Stderr = &stderr - err := cmd.Run() + err = cmd.Run() result := stdout.String() if stderr.Len() > 0 { if result != "" { diff --git a/internal/tool/schema_batch_test.go b/internal/tool/schema_batch_test.go index 93bbde204..3f300596d 100644 --- a/internal/tool/schema_batch_test.go +++ b/internal/tool/schema_batch_test.go @@ -576,3 +576,92 @@ func TestImportOrganizerSchemaProvider(t *testing.T) { t.Fatalf("required = %v, want [path]", ImportOrganizerTool{}.Parameters()["required"]) } } + +func TestJobsSchemaProvider(t *testing.T) { + var _ SchemaProvider = JobsTool{} + props := schemaProps(t, JobsTool{}.Parameters()) + enum, ok := props["action"].(map[string]interface{})["enum"].([]interface{}) + if !ok || len(enum) != 5 || enum[0] != "list" || enum[4] != "kill" { + t.Fatalf("action enum = %v, want 5 options", props["action"]) + } + if props["command"].(map[string]interface{})["type"] != "string" { + t.Fatal("command type wrong") + } + req, _ := JobsTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "action" { + t.Fatalf("required = %v, want [action]", JobsTool{}.Parameters()["required"]) + } +} + +func TestLSPToolSchemaProvider(t *testing.T) { + var _ SchemaProvider = LSPTool{} + props := schemaProps(t, LSPTool{}.Parameters()) + enum, ok := props["action"].(map[string]interface{})["enum"].([]interface{}) + if !ok || len(enum) != 4 { + t.Fatalf("action enum = %v, want 4 options", props["action"]) + } + req, _ := LSPTool{}.Parameters()["required"].([]string) + if len(req) != 2 || req[0] != "action" || req[1] != "path" { + t.Fatalf("required = %v, want [action path]", LSPTool{}.Parameters()["required"]) + } +} + +func TestToolSearchSchemaProvider(t *testing.T) { + var _ SchemaProvider = ToolSearchTool{} + props := schemaProps(t, ToolSearchTool{}.Parameters()) + if props["query"].(map[string]interface{})["type"] != "string" { + t.Fatal("query type wrong") + } + if props["max_results"].(map[string]interface{})["type"] != "integer" { + t.Fatal("max_results type wrong") + } + req, _ := ToolSearchTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "query" { + t.Fatalf("required = %v, want [query]", ToolSearchTool{}.Parameters()["required"]) + } +} + +func TestToolsetSchemaProvider(t *testing.T) { + var _ SchemaProvider = ToolsetTool{} + props := schemaProps(t, ToolsetTool{}.Parameters()) + enum, ok := props["action"].(map[string]interface{})["enum"].([]interface{}) + if !ok || len(enum) != 2 || enum[0] != "list" || enum[1] != "resolve" { + t.Fatalf("action enum = %v, want [list resolve]", props["action"]) + } + req, _ := ToolsetTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "action" { + t.Fatalf("required = %v, want [action]", ToolsetTool{}.Parameters()["required"]) + } +} + +func TestWorktreeSchemasProvider(t *testing.T) { + var _ SchemaProvider = EnterWorktreeTool{} + var _ SchemaProvider = ExitWorktreeTool{} + props := schemaProps(t, EnterWorktreeTool{}.Parameters()) + if props["path"].(map[string]interface{})["type"] != "string" { + t.Fatal("enter path type wrong") + } + req, _ := EnterWorktreeTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "path" { + t.Fatalf("enter required = %v, want [path]", EnterWorktreeTool{}.Parameters()["required"]) + } + eprops := schemaProps(t, ExitWorktreeTool{}.Parameters()) + if eprops["cleanup"].(map[string]interface{})["type"] != "boolean" { + t.Fatal("exit cleanup type wrong") + } +} + +func TestPowerShellSchemaProvider(t *testing.T) { + var _ SchemaProvider = PowerShellTool{} + props := schemaProps(t, PowerShellTool{}.Parameters()) + if props["command"].(map[string]interface{})["type"] != "string" { + t.Fatal("command type wrong") + } + if props["timeout"].(map[string]interface{})["type"] != "number" { + t.Fatal("timeout type wrong") + } + req, _ := PowerShellTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "command" { + t.Fatalf("required = %v, want [command]", PowerShellTool{}.Parameters()["required"]) + } +} diff --git a/internal/tool/tool_search.go b/internal/tool/tool_search.go index d1f27b1c3..f9fb45cc6 100644 --- a/internal/tool/tool_search.go +++ b/internal/tool/tool_search.go @@ -10,6 +10,12 @@ import ( type ToolSearchTool struct{} +// ToolSearchInput is the typed input for ToolSearchTool. +type ToolSearchInput struct { + Query string `json:"query"` + MaxResults int `json:"max_results"` +} + func (ToolSearchTool) Name() string { return "ToolSearch" } func (ToolSearchTool) RiskLevel() string { return "low" } func (ToolSearchTool) Aliases() []string { return []string{"tool_search"} } @@ -17,23 +23,29 @@ func (ToolSearchTool) Description() string { return `Search available tools by name or description. Use query "select:" for direct selection.` } -func (ToolSearchTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "query": map[string]interface{}{"type": "string", "description": `Search terms, or "select:"`}, - "max_results": map[string]interface{}{"type": "integer", "description": "Maximum results to return (default 5)"}, +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (ToolSearchTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "query": {Type: "string", Description: `Search terms, or "select:"`}, + "max_results": {Type: "integer", Description: "Maximum results to return (default 5)"}, }, - "required": []string{"query"}, + Required: []string{"query"}, } } +func (ToolSearchTool) Parameters() map[string]interface{} { + return toolSearchSchema.ToJSONSchema() +} + +// toolSearchSchema is the single source of truth for ToolSearch's input schema. +var toolSearchSchema = ToolSearchTool{}.Schema() + func (ToolSearchTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Query string `json:"query"` - MaxResults int `json:"max_results"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[ToolSearchInput]("ToolSearch", input) + if err != nil { return "", err } p.Query = strings.TrimSpace(p.Query) diff --git a/internal/tool/toolset_tool.go b/internal/tool/toolset_tool.go index 25ed9e24a..a2cb20bdb 100644 --- a/internal/tool/toolset_tool.go +++ b/internal/tool/toolset_tool.go @@ -15,6 +15,12 @@ import ( // system. type ToolsetTool struct{} +// ToolsetInput is the typed input for ToolsetTool. +type ToolsetInput struct { + Action string `json:"action"` + Name string `json:"name"` +} + func (ToolsetTool) Name() string { return "Toolset" } func (ToolsetTool) RiskLevel() string { return "low" } func (ToolsetTool) Aliases() []string { return []string{"toolset"} } @@ -22,31 +28,30 @@ func (ToolsetTool) Description() string { return "List available toolsets or resolve one to its concrete tool list. Toolsets are named, composable groups (research, dev, ops, full_stack); resolving expands required toolsets transitively." } -func (ToolsetTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "action": map[string]interface{}{ - "type": "string", - "enum": []string{"list", "resolve"}, - "description": "list: show available toolsets; resolve: expand a toolset to its tools.", - }, - "name": map[string]interface{}{ - "type": "string", - "description": "Toolset name to resolve (action=resolve).", - }, +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (ToolsetTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "action": {Type: "string", Enum: []interface{}{"list", "resolve"}, Description: "list: show available toolsets; resolve: expand a toolset to its tools."}, + "name": {Type: "string", Description: "Toolset name to resolve (action=resolve)."}, }, - "required": []string{"action"}, + Required: []string{"action"}, } } +func (ToolsetTool) Parameters() map[string]interface{} { + return toolsetSchema.ToJSONSchema() +} + +// toolsetSchema is the single source of truth for Toolset's input schema. +var toolsetSchema = ToolsetTool{}.Schema() + func (ToolsetTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Action string `json:"action"` - Name string `json:"name"` - } - if err := json.Unmarshal(input, &p); err != nil { - return "", fmt.Errorf("invalid input: %w", err) + p, err := DecodeInput[ToolsetInput]("Toolset", input) + if err != nil { + return "", err } reg, err := toolset.NewRegistry(toolset.Defaults()) if err != nil { diff --git a/internal/tool/worktree.go b/internal/tool/worktree.go index 07b9be18e..42f2a1fb7 100644 --- a/internal/tool/worktree.go +++ b/internal/tool/worktree.go @@ -13,29 +13,41 @@ import ( // EnterWorktreeTool switches to a git worktree. type EnterWorktreeTool struct{} +// EnterWorktreeInput is the typed input for EnterWorktreeTool. +type EnterWorktreeInput struct { + Path string `json:"path"` + Branch string `json:"branch,omitempty"` +} + func (EnterWorktreeTool) Name() string { return "EnterWorktree" } func (EnterWorktreeTool) Aliases() []string { return nil } func (EnterWorktreeTool) Description() string { return "Switch to a git worktree directory." } -func (EnterWorktreeTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "path": map[string]interface{}{"type": "string", "description": "Path to the worktree directory"}, - "branch": map[string]interface{}{"type": "string", "description": "Branch to create/checkout (optional, creates worktree if path doesn't exist)"}, +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (EnterWorktreeTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "path": {Type: "string", Description: "Path to the worktree directory"}, + "branch": {Type: "string", Description: "Branch to create/checkout (optional, creates worktree if path doesn't exist)"}, }, - "required": []string{"path"}, + Required: []string{"path"}, } } +func (EnterWorktreeTool) Parameters() map[string]interface{} { + return enterWorktreeSchema.ToJSONSchema() +} + +// enterWorktreeSchema is the single source of truth for EnterWorktree's input schema. +var enterWorktreeSchema = EnterWorktreeTool{}.Schema() + func (EnterWorktreeTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Path string `json:"path"` - Branch string `json:"branch,omitempty"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[EnterWorktreeInput]("EnterWorktree", input) + if err != nil { return "", err } if p.Path == "" { @@ -86,26 +98,40 @@ func (EnterWorktreeTool) Execute(ctx context.Context, input json.RawMessage) (st // ExitWorktreeTool returns to the main repository from a worktree. type ExitWorktreeTool struct{} +// ExitWorktreeInput is the typed input for ExitWorktreeTool. +type ExitWorktreeInput struct { + Cleanup bool `json:"cleanup,omitempty"` +} + func (ExitWorktreeTool) Name() string { return "ExitWorktree" } func (ExitWorktreeTool) Aliases() []string { return nil } func (ExitWorktreeTool) Description() string { return "Return to the main repository from a git worktree." } -func (ExitWorktreeTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "cleanup": map[string]interface{}{"type": "boolean", "description": "Remove the worktree after exiting (default: false)"}, +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (ExitWorktreeTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "cleanup": {Type: "boolean", Description: "Remove the worktree after exiting (default: false)"}, }, } } +func (ExitWorktreeTool) Parameters() map[string]interface{} { + return exitWorktreeSchema.ToJSONSchema() +} + +// exitWorktreeSchema is the single source of truth for ExitWorktree's input schema. +var exitWorktreeSchema = ExitWorktreeTool{}.Schema() + func (ExitWorktreeTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Cleanup bool `json:"cleanup,omitempty"` + p, err := DecodeInput[ExitWorktreeInput]("ExitWorktree", input) + if err != nil { + return "", err } - _ = json.Unmarshal(input, &p) // Find the main repository out, err := exec.CommandContext(ctx, "git", "rev-parse", "--show-toplevel").CombinedOutput() From 29dceaee9319427994b4f316c8a4fda807e45d2c Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 16 Sep 2026 12:59:02 +0530 Subject: [PATCH 2/2] style(tool): apply gofumpt formatting and drop stray placeholder --- internal/tool/lsp.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/tool/lsp.go b/internal/tool/lsp.go index 78ceb0a54..a5351bd50 100644 --- a/internal/tool/lsp.go +++ b/internal/tool/lsp.go @@ -15,8 +15,6 @@ import ( ) type LSPTool struct { - - // placeholder not used Manager *lsp.LSPManager }