Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,49 @@ DEBUG_COLORS=0 DEBUG=* gh aw compile
### Go Code Style
- **ALWAYS use `any` instead of `interface{}`** - Use the modern `any` type alias (Go 1.18+) for consistency across the codebase

### Type Patterns and Best Practices

Use appropriate type patterns to improve code clarity, maintainability, and type safety:

**Semantic Type Aliases** - Use for domain-specific primitives:
```go
// ✅ GOOD - Semantic meaning
type LineLength int
type Version string
type FeatureFlag string

const MaxExpressionLineLength LineLength = 120
const DefaultCopilotVersion Version = "0.0.369"
const MCPGatewayFeatureFlag FeatureFlag = "mcp-gateway"
```

**Dynamic Types** - Use `map[string]any` for truly dynamic data:
```go
// ✅ GOOD - Unknown structure at compile time
func ProcessFrontmatter(frontmatter map[string]any) error {
// YAML/JSON with dynamic structure
}

// ✅ GOOD - Document why any is needed
// githubTool uses any because tool configuration structure
// varies based on engine and toolsets
func ValidatePermissions(permissions *Permissions, githubTool any)
```

**When to use each pattern**:
- **Semantic type aliases**: Domain concepts (lengths, versions, durations)
- **`map[string]any`**: YAML/JSON parsing, dynamic configurations
- **Interfaces**: Multiple implementations, polymorphism, testing
- **Concrete types**: Known structure, type safety

**Avoid**:
- Using `any` when the type is known
- Creating unnecessary type aliases that don't add clarity
- Large "god" interfaces with many methods
- Type name collisions (use descriptive, domain-qualified names)

**See**: <a>specs/go-type-patterns.md</a> for detailed guidance and examples

### GitHub Actions Integration
For JavaScript files in `pkg/workflow/js/*.cjs`:
- Use `core.info`, `core.warning`, `core.error` (not console.log)
Expand Down
34 changes: 31 additions & 3 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,39 @@ import (
const CLIExtensionPrefix = "gh aw"

// Semantic types for measurements and identifiers

// LineLength represents a line length in characters for expression formatting
//
// These type aliases provide meaningful names for primitive types, improving code clarity
// and type safety. They follow the semantic type alias pattern where the type name
// indicates both what the value represents and how it should be used.
//
// Benefits of semantic type aliases:
// - Self-documenting: The type name explains the purpose
// - Type safety: Prevents mixing different concepts with the same underlying type
// - Clear intent: Signals to readers what the value represents
// - Easy refactoring: Can change implementation without affecting API
//
// See specs/go-type-patterns.md for detailed guidance on type patterns.

// LineLength represents a line length in characters for expression formatting.
// This semantic type distinguishes line lengths from arbitrary integers,
// making formatting code more readable and preventing accidental misuse.
//
// Example usage:
//
// if len(expression) > int(constants.MaxExpressionLineLength) {
// // Break into multiple lines
// }
type LineLength int

// Version represents a software version string
// Version represents a software version string.
// This semantic type distinguishes version strings from arbitrary strings,
// enabling future validation logic (e.g., semver parsing) and making
// version requirements explicit in function signatures.
//
// Example usage:
//
// const DefaultCopilotVersion Version = "0.0.369"
// func InstallTool(name string, version Version) error { ... }
type Version string

// MaxExpressionLineLength is the maximum length for a single line expression before breaking into multiline
Expand Down
45 changes: 28 additions & 17 deletions pkg/parser/frontmatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,25 @@ func ParseImportDirective(line string) *ImportDirectiveMatch {

// ImportsResult holds the result of processing imports from frontmatter
type ImportsResult struct {
MergedTools string // Merged tools configuration from all imports
MergedMCPServers string // Merged mcp-servers configuration from all imports
MergedEngines []string // Merged engine configurations from all imports
MergedSafeOutputs []string // Merged safe-outputs configurations from all imports
MergedSafeInputs []string // Merged safe-inputs configurations from all imports
MergedMarkdown string // Merged markdown content from all imports
MergedSteps string // Merged steps configuration from all imports
MergedRuntimes string // Merged runtimes configuration from all imports
MergedServices string // Merged services configuration from all imports
MergedNetwork string // Merged network configuration from all imports
MergedPermissions string // Merged permissions configuration from all imports
MergedSecretMasking string // Merged secret-masking steps from all imports
ImportedFiles []string // List of imported file paths (for manifest)
AgentFile string // Path to custom agent file (if imported)
ImportInputs map[string]any // Aggregated input values from all imports (key = input name, value = input value)
MergedTools string // Merged tools configuration from all imports
MergedMCPServers string // Merged mcp-servers configuration from all imports
MergedEngines []string // Merged engine configurations from all imports
MergedSafeOutputs []string // Merged safe-outputs configurations from all imports
MergedSafeInputs []string // Merged safe-inputs configurations from all imports
MergedMarkdown string // Merged markdown content from all imports
MergedSteps string // Merged steps configuration from all imports
MergedRuntimes string // Merged runtimes configuration from all imports
MergedServices string // Merged services configuration from all imports
MergedNetwork string // Merged network configuration from all imports
MergedPermissions string // Merged permissions configuration from all imports
MergedSecretMasking string // Merged secret-masking steps from all imports
ImportedFiles []string // List of imported file paths (for manifest)
AgentFile string // Path to custom agent file (if imported)
// ImportInputs uses map[string]any because input values can be different types (string, number, boolean).
// This is parsed from YAML frontmatter where the structure is dynamic and not known at compile time.
// This is an appropriate use of 'any' for dynamic YAML/JSON data.
// See specs/go-type-patterns.md for guidance on when to use map[string]any.
ImportInputs map[string]any // Aggregated input values from all imports (key = input name, value = input value)
}

// ImportInputDefinition defines an input parameter for a shared workflow import.
Expand All @@ -94,19 +98,26 @@ type ImportsResult struct {
type ImportInputDefinition struct {
Description string `yaml:"description,omitempty" json:"description,omitempty"`
Required bool `yaml:"required,omitempty" json:"required,omitempty"`
Default any `yaml:"default,omitempty" json:"default,omitempty"` // Can be string, number, or boolean
Default any `yaml:"default,omitempty" json:"default,omitempty"` // Can be string, number, or boolean (dynamic type from YAML)
Type string `yaml:"type,omitempty" json:"type,omitempty"` // "string", "choice", "boolean", "number"
Options []string `yaml:"options,omitempty" json:"options,omitempty"` // Options for choice type
}

// ImportSpec represents a single import specification (either a string path or an object with path and inputs)
type ImportSpec struct {
Path string // Import path (required)
Path string // Import path (required)
// Inputs uses map[string]any because input values can be different types (string, number, boolean).
// This is parsed from YAML frontmatter and validated against the imported workflow's input definitions.
// This is an appropriate use of 'any' for dynamic YAML data. See specs/go-type-patterns.md.
Inputs map[string]any // Optional input values to pass to the imported workflow (values are string, number, or boolean)
}

// ProcessImportsFromFrontmatter processes imports field from frontmatter
// Returns merged tools and engines from imported files
//
// Type Pattern Note: frontmatter uses map[string]any because it represents parsed YAML with
// dynamic structure that varies by workflow. This is the appropriate pattern for parsing
// user-provided configuration files. See specs/go-type-patterns.md for guidance.
func ProcessImportsFromFrontmatter(frontmatter map[string]any, baseDir string) (mergedTools string, mergedEngines []string, err error) {
result, err := ProcessImportsFromFrontmatterWithManifest(frontmatter, baseDir, nil)
if err != nil {
Expand Down
14 changes: 14 additions & 0 deletions pkg/workflow/permissions_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,20 @@ type PermissionsValidationResult struct {
// This is the general-purpose permission validator used during workflow compilation to check
// that the declared permissions are sufficient for the GitHub MCP toolsets being used.
//
// Parameters:
// - permissions: The workflow's declared permissions
// - githubTool: The GitHub tool configuration (uses any type because the structure varies
// based on the engine and toolsets being used. This is an appropriate use of any for
// dynamic configuration data that cannot be known at compile time.)
//
// Returns:
// - A validation result indicating any missing permissions and which toolsets require them
//
// Type Pattern Note: githubTool uses 'any' because tool configuration structure is dynamic
// and varies based on engine (copilot, claude, codex) and mode (local, remote). This is
// parsed from YAML frontmatter where the structure is not known until runtime.
// See specs/go-type-patterns.md for guidance on when to use 'any' types.
//
// Use ValidatePermissions (this function) for general permission validation against GitHub MCP toolsets.
// Use ValidateIncludedPermissions (in imports.go) when validating permissions from included/imported workflow files.
func ValidatePermissions(permissions *Permissions, githubTool any) *PermissionsValidationResult {
Expand Down
3 changes: 2 additions & 1 deletion specs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This directory contains design specifications and implementation documentation f
|----------|--------|----------------|
| [Code Organization Patterns](./code-organization.md) | ✅ Documented | Code organization guidelines and patterns |
| [Validation Architecture](./validation-architecture.md) | ✅ Documented | `pkg/workflow/validation.go` and domain-specific files |
| [Go Type Patterns and Best Practices](./go-type-patterns.md) | ✅ Documented | `pkg/constants/constants.go`, `pkg/workflow/permissions_validator.go`, `pkg/parser/frontmatter.go` |

## Specifications

Expand Down Expand Up @@ -43,4 +44,4 @@ When adding new specifications:

---

**Last Updated**: 2025-12-05
**Last Updated**: 2025-12-18
Loading
Loading