From fa72b90d4072d640e3dda734424c9913b44f45d2 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 16 Sep 2026 06:44:43 +0530 Subject: [PATCH] refactor(tool): migrate Download/WebFetch/FileWrite/FileEdit to typed schemas Follow the Schema()+Parameters() single-source pattern with wire-shape parity tests (including alias-marker preservation for file_path/old_str/ new_str, which the input validator discovers from descriptions). Execute parses via DecodeInput[T]. --- internal/tool/download.go | 34 ++++++++++++------ internal/tool/file_edit.go | 54 +++++++++++++++++----------- internal/tool/file_write.go | 41 +++++++++++++-------- internal/tool/schema_batch_test.go | 57 ++++++++++++++++++++++++++++++ internal/tool/web_fetch.go | 32 +++++++++++------ 5 files changed, 163 insertions(+), 55 deletions(-) diff --git a/internal/tool/download.go b/internal/tool/download.go index 89d14004..68921093 100644 --- a/internal/tool/download.go +++ b/internal/tool/download.go @@ -21,24 +21,36 @@ func (DownloadTool) Description() string { return "Download a file from a URL and save it to a local path." } -func (DownloadTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "url": map[string]interface{}{"type": "string", "description": "URL to download from"}, - "destination": map[string]interface{}{"type": "string", "description": "Local file path to save to"}, +// DownloadInput is the typed input for DownloadTool. +type DownloadInput struct { + URL string `json:"url"` + Destination string `json:"destination"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (DownloadTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "url": {Type: "string", Description: "URL to download from"}, + "destination": {Type: "string", Description: "Local file path to save to"}, }, } } +func (DownloadTool) Parameters() map[string]interface{} { + return downloadSchema.ToJSONSchema() +} + +// downloadSchema is the single source of truth for Download's input schema. +var downloadSchema = DownloadTool{}.Schema() + const maxDownloadSize = 50 * 1024 * 1024 // 50MB func (DownloadTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - URL string `json:"url"` - Destination string `json:"destination"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[DownloadInput]("Download", input) + if err != nil { return "", err } if p.URL == "" || p.Destination == "" { diff --git a/internal/tool/file_edit.go b/internal/tool/file_edit.go index d7445be8..20de6f64 100644 --- a/internal/tool/file_edit.go +++ b/internal/tool/file_edit.go @@ -18,31 +18,45 @@ func (FileEditTool) Description() string { return "Edit a file by replacing an exact string match with new content." } -func (FileEditTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "path": map[string]interface{}{"type": "string", "description": "File path to edit"}, - "file_path": map[string]interface{}{"type": "string", "description": "Archive-compatible alias for path"}, - "old_str": map[string]interface{}{"type": "string", "description": "Exact string to find and replace"}, - "old_string": map[string]interface{}{"type": "string", "description": "Archive-compatible alias for old_str"}, - "new_str": map[string]interface{}{"type": "string", "description": "Replacement string"}, - "new_string": map[string]interface{}{"type": "string", "description": "Archive-compatible alias for new_str"}, +// FileEditInput is the typed input for FileEditTool. NewStr/NewString are +// pointers so an explicit empty replacement is distinguishable from absent. +type FileEditInput struct { + Path string `json:"path"` + FilePath string `json:"file_path"` + OldStr string `json:"old_str"` + OldString string `json:"old_string"` + NewStr *string `json:"new_str"` + NewString *string `json:"new_string"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. Alias descriptions are preserved verbatim: the input +// validator discovers aliases from them. +func (FileEditTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "path": {Type: "string", Description: "File path to edit"}, + "file_path": {Type: "string", Description: "Archive-compatible alias for path"}, + "old_str": {Type: "string", Description: "Exact string to find and replace"}, + "old_string": {Type: "string", Description: "Archive-compatible alias for old_str"}, + "new_str": {Type: "string", Description: "Replacement string"}, + "new_string": {Type: "string", Description: "Archive-compatible alias for new_str"}, }, - "required": []string{"path", "old_str"}, + Required: []string{"path", "old_str"}, } } +func (FileEditTool) Parameters() map[string]interface{} { + return fileEditSchema.ToJSONSchema() +} + +// fileEditSchema is the single source of truth for FileEdit's input schema. +var fileEditSchema = FileEditTool{}.Schema() + func (FileEditTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Path string `json:"path"` - FilePath string `json:"file_path"` - OldStr string `json:"old_str"` - OldString string `json:"old_string"` - NewStr *string `json:"new_str"` - NewString *string `json:"new_string"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[FileEditInput]("FileEdit", input) + if err != nil { return "", err } path := p.Path diff --git a/internal/tool/file_write.go b/internal/tool/file_write.go index 9818f7b5..8719af24 100644 --- a/internal/tool/file_write.go +++ b/internal/tool/file_write.go @@ -18,25 +18,38 @@ func (FileWriteTool) Description() string { return "Create or overwrite a file with the given content." } -func (FileWriteTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "path": map[string]interface{}{"type": "string", "description": "File path to write"}, - "file_path": map[string]interface{}{"type": "string", "description": "Archive-compatible alias for path"}, - "content": map[string]interface{}{"type": "string", "description": "File content"}, +// FileWriteInput is the typed input for FileWriteTool. +type FileWriteInput struct { + Path string `json:"path"` + FilePath string `json:"file_path"` + Content string `json:"content"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. Alias descriptions are preserved verbatim: the input +// validator discovers aliases from them. +func (FileWriteTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "path": {Type: "string", Description: "File path to write"}, + "file_path": {Type: "string", Description: "Archive-compatible alias for path"}, + "content": {Type: "string", Description: "File content"}, }, - "required": []string{"path", "content"}, + Required: []string{"path", "content"}, } } +func (FileWriteTool) Parameters() map[string]interface{} { + return fileWriteSchema.ToJSONSchema() +} + +// fileWriteSchema is the single source of truth for FileWrite's input schema. +var fileWriteSchema = FileWriteTool{}.Schema() + func (FileWriteTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - Path string `json:"path"` - FilePath string `json:"file_path"` - Content string `json:"content"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[FileWriteInput]("FileWrite", input) + if err != nil { return "", err } path := p.Path diff --git a/internal/tool/schema_batch_test.go b/internal/tool/schema_batch_test.go index 92c5d0e5..12fed7cc 100644 --- a/internal/tool/schema_batch_test.go +++ b/internal/tool/schema_batch_test.go @@ -118,3 +118,60 @@ func TestMatchesRange(t *testing.T) { t.Fatal("no bounds should pass") } } + +func TestDownloadSchemaProvider(t *testing.T) { + var _ SchemaProvider = DownloadTool{} + props := schemaProps(t, DownloadTool{}.Parameters()) + for _, f := range []string{"url", "destination"} { + if props[f].(map[string]interface{})["type"] != "string" { + t.Fatalf("%s type wrong", f) + } + } + _, hasRequired := DownloadTool{}.Parameters()["required"] + if hasRequired { + t.Fatalf("Download has no required fields, got %v", DownloadTool{}.Parameters()["required"]) + } +} + +func TestWebFetchSchemaProvider(t *testing.T) { + var _ SchemaProvider = WebFetchTool{} + props := schemaProps(t, WebFetchTool{}.Parameters()) + if props["url"].(map[string]interface{})["type"] != "string" { + t.Fatal("url type wrong") + } + req, _ := WebFetchTool{}.Parameters()["required"].([]string) + if len(req) != 1 || req[0] != "url" { + t.Fatalf("required = %v, want [url]", WebFetchTool{}.Parameters()["required"]) + } +} + +func TestFileWriteSchemaProvider(t *testing.T) { + var _ SchemaProvider = FileWriteTool{} + props := schemaProps(t, FileWriteTool{}.Parameters()) + // The file_path alias marker must survive: the validator discovers + // aliases from descriptions. + if desc := props["file_path"].(map[string]interface{})["description"]; desc != "Archive-compatible alias for path" { + t.Fatalf("file_path description = %v, alias marker lost", desc) + } + req, _ := FileWriteTool{}.Parameters()["required"].([]string) + if len(req) != 2 || req[0] != "path" || req[1] != "content" { + t.Fatalf("required = %v, want [path content]", FileWriteTool{}.Parameters()["required"]) + } +} + +func TestFileEditSchemaProvider(t *testing.T) { + var _ SchemaProvider = FileEditTool{} + props := schemaProps(t, FileEditTool{}.Parameters()) + for _, f := range []string{"path", "old_str", "new_str"} { + if props[f].(map[string]interface{})["type"] != "string" { + t.Fatalf("%s type wrong", f) + } + } + if desc := props["old_string"].(map[string]interface{})["description"]; desc != "Archive-compatible alias for old_str" { + t.Fatalf("old_string description = %v, alias marker lost", desc) + } + req, _ := FileEditTool{}.Parameters()["required"].([]string) + if len(req) != 2 || req[0] != "path" || req[1] != "old_str" { + t.Fatalf("required = %v, want [path old_str]", FileEditTool{}.Parameters()["required"]) + } +} diff --git a/internal/tool/web_fetch.go b/internal/tool/web_fetch.go index 1fe5ff21..436ff09f 100644 --- a/internal/tool/web_fetch.go +++ b/internal/tool/web_fetch.go @@ -19,16 +19,30 @@ func (WebFetchTool) Description() string { return "Fetch a URL and return its content as text. HTML is converted to plain text." } -func (WebFetchTool) Parameters() map[string]interface{} { - return map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "url": map[string]interface{}{"type": "string", "description": "URL to fetch"}, +// WebFetchInput is the typed input for WebFetchTool. +type WebFetchInput struct { + URL string `json:"url"` +} + +// Schema returns the typed input schema. Parameters() delegates to it so the +// two cannot diverge. +func (WebFetchTool) Schema() ToolSchema { + return ToolSchema{ + Type: "object", + Properties: map[string]SchemaProperty{ + "url": {Type: "string", Description: "URL to fetch"}, }, - "required": []string{"url"}, + Required: []string{"url"}, } } +func (WebFetchTool) Parameters() map[string]interface{} { + return webFetchSchema.ToJSONSchema() +} + +// webFetchSchema is the single source of truth for WebFetch's input schema. +var webFetchSchema = WebFetchTool{}.Schema() + // Timeout declares WebFetch's execution budget so the engine's dispatch // deadline matches the tool's own 30s HTTP deadline. Declared budgets win // over the name-based fallback (DSH tool-declared timeout policy). @@ -40,10 +54,8 @@ var ( ) func (WebFetchTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { - var p struct { - URL string `json:"url"` - } - if err := json.Unmarshal(input, &p); err != nil { + p, err := DecodeInput[WebFetchInput]("WebFetch", input) + if err != nil { return "", err } if p.URL == "" {