From 47172f3f57548673243bad24111d1c61e8a647aa Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 16 Sep 2026 00:25:38 +0530 Subject: [PATCH] refactor(tool): migrate Glob/LS/Grep/Skill/Screenshot to typed schemas Follow the established Schema()+Parameters() delegation pattern (single source of truth) with schema-parity tests locking the wire shape, and route Execute input parsing through DecodeInput[T]. Empty-input semantics preserved for LS/Skill (list modes). FuzzyFind excluded: its minimum/maximum constraints need a SchemaProperty extension first. --- internal/tool/glob.go | 39 +++++++++----- internal/tool/grep.go | 41 +++++++++----- internal/tool/ls.go | 37 +++++++++---- internal/tool/schema_batch_test.go | 85 ++++++++++++++++++++++++++++++ internal/tool/screenshot.go | 52 +++++++++++------- internal/tool/skill.go | 33 ++++++++---- 6 files changed, 220 insertions(+), 67 deletions(-) create mode 100644 internal/tool/schema_batch_test.go diff --git a/internal/tool/glob.go b/internal/tool/glob.go index 2c3673dcc..7a92bf2f4 100644 --- a/internal/tool/glob.go +++ b/internal/tool/glob.go @@ -15,23 +15,36 @@ func (GlobTool) Name() string { return "Glob" } func (GlobTool) RiskLevel() string { return "low" } func (GlobTool) Aliases() []string { return []string{"glob"} } func (GlobTool) Description() string { return "Find files matching a glob pattern." } -func (GlobTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "pattern": map[string]interface{}{"type": "string", "description": "Glob pattern (e.g. **/*.go)"}, - "path": map[string]interface{}{"type": "string", "description": "Root directory (default: current dir)"}, + +// GlobInput is the typed input for GlobTool. +type GlobInput struct { + Pattern string `json:"pattern"` + Path string `json:"path"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (GlobTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "pattern": {Type: "string", Description: "Glob pattern (e.g. **/*.go)"}, + "path": {Type: "string", Description: "Root directory (default: current dir)"}, }, - "required": []string{"pattern"}, + Required: []string{"pattern"}, } } +func (GlobTool) Parameters() map[string]interface{} { + return globSchema.ToJSONSchema() +} + +// globSchema is the single source of truth for Glob's input schema. +var globSchema = GlobTool{}.Schema() + func (GlobTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Pattern string `json:"pattern"` - Path string `json:"path"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[GlobInput]("Glob", input) + if err != nil { return "", err } root := p.Path @@ -42,7 +55,7 @@ func (GlobTool) Execute(ctx context.Context, input json.RawMessage) (string, err return "", err } var matches []string - err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + err = filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { if err != nil { return nil } diff --git a/internal/tool/grep.go b/internal/tool/grep.go index 5cbc3d20a..2bd60dff0 100644 --- a/internal/tool/grep.go +++ b/internal/tool/grep.go @@ -17,25 +17,38 @@ func (GrepTool) Name() string { return "Grep" } func (GrepTool) RiskLevel() string { return "low" } func (GrepTool) Aliases() []string { return []string{"grep"} } func (GrepTool) Description() string { return "Search for a regex pattern in files." } -func (GrepTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "pattern": map[string]interface{}{"type": "string", "description": "Regex pattern to search for"}, - "path": map[string]interface{}{"type": "string", "description": "Directory to search (default: current dir)"}, - "include": map[string]interface{}{"type": "string", "description": "File glob filter (e.g. *.go)"}, + +// GrepInput is the typed input for GrepTool. +type GrepInput struct { + Pattern string `json:"pattern"` + Path string `json:"path"` + Include string `json:"include"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (GrepTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "pattern": {Type: "string", Description: "Regex pattern to search for"}, + "path": {Type: "string", Description: "Directory to search (default: current dir)"}, + "include": {Type: "string", Description: "File glob filter (e.g. *.go)"}, }, - "required": []string{"pattern"}, + Required: []string{"pattern"}, } } +func (GrepTool) Parameters() map[string]interface{} { + return grepSchema.ToJSONSchema() +} + +// grepSchema is the single source of truth for Grep's input schema. +var grepSchema = GrepTool{}.Schema() + func (GrepTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Pattern string `json:"pattern"` - Path string `json:"path"` - Include string `json:"include"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[GrepInput]("Grep", input) + if err != nil { return "", err } re, err := regexp.Compile(p.Pattern) diff --git a/internal/tool/ls.go b/internal/tool/ls.go index ce168afdf..2e3986c5d 100644 --- a/internal/tool/ls.go +++ b/internal/tool/ls.go @@ -19,23 +19,38 @@ func (LSTool) Description() string { return "List files and directories in a directory." } -func (LSTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "path": map[string]interface{}{"type": "string", "description": "Directory path to list (default: current directory)"}, - "ignore": map[string]interface{}{"type": "array", "items": map[string]interface{}{"type": "string"}, "description": "Optional file or glob patterns to exclude"}, +// LSInput is the typed input for LSTool. +type LSInput struct { + Path string `json:"path"` + Ignore []string `json:"ignore"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (LSTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "path": {Type: "string", Description: "Directory path to list (default: current directory)"}, + "ignore": {Type: "array", Description: "Optional file or glob patterns to exclude", Items: &SchemaProperty{Type: "string"}}, }, } } +func (LSTool) Parameters() map[string]interface{} { + return lsSchema.ToJSONSchema() +} + +// lsSchema is the single source of truth for LS's input schema. +var lsSchema = LSTool{}.Schema() + func (LSTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Path string `json:"path"` - Ignore []string `json:"ignore"` - } + // Empty input lists the current directory; only decode when present. + var p LSInput if len(input) > 0 { - if err := json.Unmarshal(input, &p); err != nil { + var err error + p, err = DecodeInput[LSInput]("LS", input) + if err != nil { return "", err } } diff --git a/internal/tool/schema_batch_test.go b/internal/tool/schema_batch_test.go new file mode 100644 index 000000000..f2f4466ec --- /dev/null +++ b/internal/tool/schema_batch_test.go @@ -0,0 +1,85 @@ +package tool + +import "testing" + +func schemaProps(t *testing.T, params map[string]interface{}) map[string]interface{} { + t.Helper() + props, ok := params["properties"].(map[string]interface{}) + if !ok { + t.Fatal("properties missing") + } + return props +} + +func TestGlobSchemaProvider(t *testing.T) { + var _ SchemaProvider = GlobTool{} + props := schemaProps(t, GlobTool{}.Parameters()) + if props["pattern"].(map[string]interface{})["type"] != "string" { + t.Fatal("pattern type wrong") + } + if props["path"].(map[string]interface{})["type"] != "string" { + t.Fatal("path type wrong") + } + req, _ := GlobTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "pattern" { + t.Fatalf("required = %v, want [pattern]", GlobTool{}.Parameters()["required"]) + } +} + +func TestLSSchemaProvider(t *testing.T) { + var _ SchemaProvider = LSTool{} + props := schemaProps(t, LSTool{}.Parameters()) + ignore, ok := props["ignore"].(map[string]interface{}) + if !ok || ignore["type"] != "array" { + t.Fatalf("ignore prop = %v, want array type", props["ignore"]) + } + items, ok := ignore["items"].(map[string]interface{}) + if !ok || items["type"] != "string" { + t.Fatalf("ignore items = %v, want string type", ignore["items"]) + } + _, hasRequired := LSTool{}.Parameters()["required"] + if hasRequired { + t.Fatalf("LS has no required fields, got %v", LSTool{}.Parameters()["required"]) + } +} + +func TestGrepSchemaProvider(t *testing.T) { + var _ SchemaProvider = GrepTool{} + props := schemaProps(t, GrepTool{}.Parameters()) + for _, f := range []string{"pattern", "path", "include"} { + if props[f].(map[string]interface{})["type"] != "string" { + t.Fatalf("%s type wrong", f) + } + } + req, _ := GrepTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "pattern" { + t.Fatalf("required = %v, want [pattern]", GrepTool{}.Parameters()["required"]) + } +} + +func TestSkillSchemaProvider(t *testing.T) { + var _ SchemaProvider = SkillTool{} + props := schemaProps(t, SkillTool{}.Parameters()) + if props["skill"].(map[string]interface{})["type"] != "string" { + t.Fatal("skill type wrong") + } + _, hasRequired := SkillTool{}.Parameters()["required"] + if hasRequired { + t.Fatalf("Skill has no required fields, got %v", SkillTool{}.Parameters()["required"]) + } +} + +func TestScreenshotSchemaProvider(t *testing.T) { + var _ SchemaProvider = ScreenshotTool{} + props := schemaProps(t, ScreenshotTool{}.Parameters()) + if props["url"].(map[string]interface{})["type"] != "string" { + t.Fatal("url type wrong") + } + if props["width"].(map[string]interface{})["type"] != "number" { + t.Fatal("width type wrong") + } + req, _ := ScreenshotTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "url" { + t.Fatalf("required = %v, want [url]", ScreenshotTool{}.Parameters()["required"]) + } +} diff --git a/internal/tool/screenshot.go b/internal/tool/screenshot.go index cef90d77a..e006c4c65 100644 --- a/internal/tool/screenshot.go +++ b/internal/tool/screenshot.go @@ -21,31 +21,43 @@ func (ScreenshotTool) Description() string { return "Capture a full-page screenshot of a URL with headless Chrome and save it as a PNG file." } -func (ScreenshotTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "url": map[string]interface{}{"type": "string", "description": "URL (http/https) to capture"}, - "path": map[string]interface{}{"type": "string", "description": "Destination PNG path; defaults to a temp file"}, - "width": map[string]interface{}{"type": "number", "description": "Viewport width in pixels (default 1280)"}, - "height": map[string]interface{}{"type": "number", "description": "Viewport height in pixels (default 800)"}, - "wait_ms": map[string]interface{}{"type": "number", "description": "Milliseconds to wait for the page to render (default 1500)"}, - "selector": map[string]interface{}{"type": "string", "description": "Optional CSS selector to wait for before capturing"}, +// ScreenshotInput is the typed input for ScreenshotTool. +type ScreenshotInput struct { + URL string `json:"url"` + Path string `json:"path"` + Width int `json:"width"` + Height int `json:"height"` + WaitMS int `json:"wait_ms"` + Selector string `json:"selector"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (ScreenshotTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "url": {Type: "string", Description: "URL (http/https) to capture"}, + "path": {Type: "string", Description: "Destination PNG path; defaults to a temp file"}, + "width": {Type: "number", Description: "Viewport width in pixels (default 1280)"}, + "height": {Type: "number", Description: "Viewport height in pixels (default 800)"}, + "wait_ms": {Type: "number", Description: "Milliseconds to wait for the page to render (default 1500)"}, + "selector": {Type: "string", Description: "Optional CSS selector to wait for before capturing"}, }, - "required": []string{"url"}, + Required: []string{"url"}, } } +func (ScreenshotTool) Parameters() map[string]interface{} { + return screenshotSchema.ToJSONSchema() +} + +// screenshotSchema is the single source of truth for Screenshot's input schema. +var screenshotSchema = ScreenshotTool{}.Schema() + func (ScreenshotTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - URL string `json:"url"` - Path string `json:"path"` - Width int `json:"width"` - Height int `json:"height"` - WaitMS int `json:"wait_ms"` - Selector string `json:"selector"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[ScreenshotInput]("Screenshot", input) + if err != nil { return "", err } diff --git a/internal/tool/skill.go b/internal/tool/skill.go index 6b7b39556..f4276c6c3 100644 --- a/internal/tool/skill.go +++ b/internal/tool/skill.go @@ -20,21 +20,36 @@ func (SkillTool) Description() string { return "Load instructions from a local Rho skill. Use without a skill name to list available skills." } -func (SkillTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "skill": map[string]interface{}{"type": "string", "description": "Skill name to load"}, +// SkillInput is the typed input for SkillTool. +type SkillInput struct { + Skill string `json:"skill"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (SkillTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "skill": {Type: "string", Description: "Skill name to load"}, }, } } +func (SkillTool) Parameters() map[string]interface{} { + return skillSchema.ToJSONSchema() +} + +// skillSchema is the single source of truth for Skill's input schema. +var skillSchema = SkillTool{}.Schema() + func (SkillTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Skill string `json:"skill"` - } + // Empty input lists available skills; only decode when present. + var p SkillInput if len(input) > 0 { - if err := json.Unmarshal(input, &p); err != nil { + var err error + p, err = DecodeInput[SkillInput]("Skill", input) + if err != nil { return "", err } }