Skip to content

Add fully-typed Tools struct and parser for tools section#1941

Merged
pelikhan merged 9 commits intomainfrom
copilot/add-tools-go-struct
Oct 18, 2025
Merged

Add fully-typed Tools struct and parser for tools section#1941
pelikhan merged 9 commits intomainfrom
copilot/add-tools-go-struct

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 18, 2025

Overview

This PR adds a structured Tools type to represent the parsed tools section from workflow frontmatter, providing fully type-safe access to tool configurations with zero any types and minimal API surface.

Problem

Currently, tools are accessed throughout the codebase using direct map access with type assertions:

// Current approach - no type safety, verbose type assertions
githubTool := tools["github"]
if githubConfig, ok := githubTool.(map[string]any); ok {
    if allowed, hasAllowed := githubConfig["allowed"].([]any); hasAllowed {
        // ... process allowed tools
    }
}

This approach:

  • Requires verbose type assertions at every access point
  • Provides no IDE autocomplete or type hints
  • Makes it easy to introduce bugs through typos or incorrect type assertions
  • Lacks distinction between "tool not configured" and "tool configured with nil value"

Solution

Introduced a structured Tools type with fully-typed fields and direct field access:

1. Fully-Typed Tools Struct

type Tools struct {
    GitHub           *GitHubToolConfig
    Bash             *BashToolConfig
    Edit             *EditToolConfig
    WebFetch         *WebFetchToolConfig
    WebSearch        *WebSearchToolConfig
    Playwright       *PlaywrightToolConfig
    AgenticWorkflows *AgenticWorkflowsToolConfig
    CacheMemory      *CacheMemoryToolConfig
    SafetyPrompt     *bool
    Timeout          *int
    StartupTimeout   *int
    Custom           map[string]any  // For custom MCP tools
}

2. All Config Structs Are Fully Typed

type GitHubToolConfig struct {
    Allowed     []string
    Mode        string
    Version     string
    Args        []string
    ReadOnly    bool
    GitHubToken string
    Toolset     []string
}

type PlaywrightToolConfig struct {
    Version        string
    AllowedDomains []string  // Handles both string and array from YAML
    Args           []string
}

type BashToolConfig struct {
    AllowedCommands []string  // List of allowed bash commands
}

3. Direct Type-Safe Field Access

// New approach - fully type-safe, direct field access
if data.ParsedTools.GitHub != nil {
    config := data.ParsedTools.GitHub  // Direct access to *GitHubToolConfig
    if config.Mode == "remote" {
        // Work with typed fields - full autocomplete support
    }
}

4. Backwards Compatibility

// Existing code continues to work
tools := data.Tools  // Still map[string]any

// New code can use ParsedTools
parsedTools := data.ParsedTools  // Fully typed Tools struct

Changes Made

Tools Type System (pkg/workflow/tools_types.go)

  • Fully-typed Tools struct - All fields properly typed, no any types in tool configurations
  • Minimal API surface - Only HasTool() and GetToolNames() methods, encouraging direct field access
  • Idiomatic nil checks - Uses pointer nil checks instead of separate tracking flags
  • Dedicated parser functions - Each tool type has its own parser (parseGitHubTool, parseBashTool, etc.)
  • Deprecated tools removed - Claude tool removed from Go code as it is no longer supported

Schema Updates

  • Removed claude tool from JSON schemas:
    • pkg/parser/schemas/main_workflow_schema.json - Removed claude tool definition (lines 1660-1702)
    • pkg/parser/schemas/included_file_schema.json - Removed claude tool definition (lines 155-185)
  • Preserved claude engine - All "claude" engine references remain intact (as requested)
  • Updated tests - Removed "invalid claude tools" test, updated complex tools test to use bash

WorkflowData Integration

  • Added ParsedTools *Tools field to WorkflowData
  • Automatically populated during workflow parsing
  • Zero changes required to existing code

Shared Workflow Fix

  • Fixed genaiscript.md - Added mkdir -p /tmp/gh-aw/aw-prompts to ensure directory exists before writing files

Benefits

  • Full Type Safety: Zero any types - all tool configurations are properly typed
  • Better IDE Support: Complete autocomplete for all tool fields
  • Compile-Time Validation: Type errors caught at compile time, not runtime
  • Cleaner Code: Direct field access instead of type assertions and map lookups
  • Simple API: No wrapper functions or converters - just direct field access
  • Idiomatic Go: Uses standard pointer nil checks instead of separate tracking booleans
  • Backwards Compatible: No breaking changes, existing code works unchanged
  • Easy Migration: New code can directly use ParsedTools with full type safety
  • Extensible: Simple to add new tool types and typed configuration structs
  • Smaller footprint: Eliminated redundant tracking fields for cleaner struct definition

Testing

  • Added comprehensive test coverage (200+ lines of tests)
  • All existing tests pass
  • New test suites for:
    • Tools struct creation and methods
    • Parser functions for each tool type
    • Direct field access validation
  • Linter passes with no warnings
  • Build successful

Example Usage

// Before (with any and type assertions):
githubTool := tools["github"]
if githubConfig, ok := githubTool.(map[string]any); ok {
    if mode, ok := githubConfig["mode"].(string); ok {
        // use mode
    }
}

// After (fully typed, direct access):
if data.ParsedTools.GitHub != nil {
    mode := data.ParsedTools.GitHub.Mode  // Direct field access, full autocomplete
    if mode == "remote" {
        // Work with typed config
    }
}

// Check if a tool is configured (idiomatic nil check)
if data.ParsedTools.HasTool("github") {
    config := data.ParsedTools.GitHub  // *GitHubToolConfig
    for _, allowed := range config.Allowed {
        // Process with type safety
    }
}

// List all tools
for _, toolName := range data.ParsedTools.GetToolNames() {
    // Access tools directly through typed fields
    switch toolName {
    case "github":
        config := data.ParsedTools.GitHub
        // Use config
    }
}

Future Work

This infrastructure enables:

  • Gradual migration of existing map access to direct typed field access
  • Adding more typed configuration structs for other tools
  • Improved error messages when tools are misconfigured
  • Better validation of tool configurations at parse time
Original prompt
  • Add a Tools go struct type in the complete that represents the parsed "tools" section.
  • Add parser for the tools section that uses the new type
  • add tools parsed object to workflow data.
  • use parsed tool information in tools helpers

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 2 commits October 18, 2025 12:32
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add tools go struct and parser for workflow data Add Tools struct type and parser for tools section Oct 18, 2025
Comment thread pkg/workflow/tools_types.go
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Add Tools struct type and parser for tools section Add fully-typed Tools struct and parser for tools section Oct 18, 2025
Copilot AI requested a review from pelikhan October 18, 2025 13:06
Comment thread pkg/workflow/tools_helpers.go Outdated

// getToolFromWorkflowData is a helper to get a tool configuration from WorkflowData
// It checks ParsedTools first, and falls back to the raw Tools map for backwards compatibility
func getToolFromWorkflowData(data *WorkflowData, toolName string) any {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot remove this helper and untyped data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit b81aaa7. Removed tools_helpers.go file containing untyped helper functions. Users should now access tools directly via the typed data.ParsedTools field.

Comment thread pkg/workflow/tools_helpers.go Outdated
// getPlaywrightConfigFromWorkflowData is a helper to get Playwright tool configuration from WorkflowData
// Returns the configuration as a typed struct when ParsedTools is available, or the raw value otherwise
// nolint:unused // Public API for accessing parsed tool configuration
func getPlaywrightConfigFromWorkflowData(data *WorkflowData) any {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Copilt avoid untype functions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit b81aaa7. Removed all untyped functions that returned any. Tools should be accessed directly via typed fields like data.ParsedTools.GitHub instead.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan October 18, 2025 13:19
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot format lint and remove dead code

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 18, 2025

@copilot format lint and remove dead code

Done in commit 70a1c52.

Formatted code with make fmt, ran linter with make lint, and removed dead code:

  • Removed ToMap() and all converter helper functions (not used)
  • Removed GetTool() method (returns any, not type-safe, not used)
  • Removed redundant accessor methods GetGitHubConfig(), GetPlaywrightConfig(), GetClaudeConfig() (fields are directly accessible)

All tests passing ✓

Comment thread pkg/workflow/tools_types.go Outdated
raw map[string]any

// Track which known tools are explicitly set (even if nil)
hasGitHub bool
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot can we infer this field by checking for nil instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit 2b8fd8e. Removed all has* tracking fields and simplified to use direct nil checks instead (e.g., t.GitHub != nil instead of t.hasGitHub).

Comment thread pkg/workflow/tools_types.go Outdated
// Extract custom MCP tools (anything not in the known list)
knownTools := map[string]bool{
"github": true,
"claude": true,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot remove the "claude" tool, it is deprecated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit 2b8fd8e. Removed the deprecated Claude tool including ClaudeToolConfig type, parseClaudeTool function, and related tests.

… has* flags

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan October 18, 2025 13:58
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot remove "claude" tool from jsom schema (do not modify agentic engine)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 18, 2025

@copilot remove "claude" tool from jsom schema (do not modify agentic engine)

Done in commit 31dee9f. Removed claude tool from both JSON schemas (main_workflow_schema.json and included_file_schema.json) while preserving all claude engine references. Updated related tests.

@pelikhan pelikhan marked this pull request as ready for review October 18, 2025 16:09
@github-actions
Copy link
Copy Markdown
Contributor

Agentic Changeset Generator triggered by this pull request.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot up the jqschema shared agentic workflow to update the step that creates the script to create the /tmp/gh-aw/ folder before writing to it

…ript shared workflow

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 18, 2025

@copilot up the jqschema shared agentic workflow to update the step that creates the script to create the /tmp/gh-aw/ folder before writing to it

Done in commit 7c4b8ed. Added mkdir -p /tmp/gh-aw/aw-prompts before writing to the prompt file in the genaiscript shared workflow.

@pelikhan pelikhan merged commit 832ebc2 into main Oct 18, 2025
4 checks passed
@pelikhan pelikhan deleted the copilot/add-tools-go-struct branch October 18, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants