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
5 changes: 4 additions & 1 deletion pkg/workflow/agent_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,10 @@ func (c *Compiler) validateBareModeSupport(frontmatter map[string]any, engine Co
// validateWorkflowRunBranches validates that workflow_run triggers include branch restrictions
// This is a security best practice to avoid running on all branches
func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markdownPath string) error {
if workflowData.On == "" {
// Fast path: skip expensive YAML parsing when the On field cannot possibly contain
// a workflow_run trigger (including when it is empty). This avoids yaml.Unmarshal
// on every validateWorkflowData call for the common case of non-workflow_run workflows.
if !strings.Contains(workflowData.On, "workflow_run") {
return nil
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/workflow/compiler_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,8 @@ type WorkflowData struct {
EngineConfigSteps []map[string]any // steps returned by engine.RenderConfig — prepended before execution steps
ServicePortExpressions string // comma-separated ${{ job.services['<id>'].ports['<port>'] }} expressions for AWF --allow-host-service-ports
RunInstallScripts bool // true when run-install-scripts: true is set (globally or per node runtime); disables --ignore-scripts on generated npm install steps
CachedPermissions *Permissions // cached parsed Permissions object (for performance optimization); populated by applyDefaults after all permission mutations
ConcurrencyGroupExpr string // cached concurrency group expression extracted from Concurrency YAML (for performance optimization); populated by applyDefaults
}

// PinContext returns an actionpins.PinContext backed by this WorkflowData.
Expand Down
21 changes: 13 additions & 8 deletions pkg/workflow/compiler_validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,16 @@ func (c *Compiler) validateFeatureConfig(workflowData *WorkflowData, markdownPat
// branch security, GitHub MCP toolset permissions, and the id-token write warning.
// It returns the parsed *Permissions for reuse in subsequent validation steps.
func (c *Compiler) validatePermissions(workflowData *WorkflowData, markdownPath string) (*Permissions, error) {
// Parse permissions once for all permission-related validation checks below.
// WorkflowData.Permissions contains the raw YAML string (including "permissions:" prefix).
// Parsing once here avoids redundant YAML parsing in each validator.
workflowPermissions := NewPermissionsParser(workflowData.Permissions).ToPermissions()
// Use the cached *Permissions object when available to avoid repeated YAML parsing.
// CachedPermissions is populated by applyDefaults after all permission mutations are applied.
// Fall back to parsing from the raw string for code paths that bypass applyDefaults
// (e.g., tests that construct WorkflowData directly).
var workflowPermissions *Permissions
if workflowData.CachedPermissions != nil {
workflowPermissions = workflowData.CachedPermissions
} else {
workflowPermissions = NewPermissionsParser(workflowData.Permissions).ToPermissions()
}

// Validate dangerous permissions
log.Printf("Validating dangerous permissions")
Expand Down Expand Up @@ -248,10 +254,9 @@ func (c *Compiler) validateToolConfiguration(workflowData *WorkflowData, markdow
// Validate workflow-level concurrency group expression
log.Printf("Validating workflow-level concurrency configuration")
if workflowData.Concurrency != "" {
// Extract the group expression from the concurrency YAML
// The Concurrency field contains the full YAML (e.g., "concurrency:\n group: \"...\"")
// We need to extract just the group value
groupExpr := extractConcurrencyGroupFromYAML(workflowData.Concurrency)
// Use the cached concurrency group expression extracted during applyDefaults to avoid
// repeated regex-based extraction on every validateWorkflowData call.
groupExpr := workflowData.ConcurrencyGroupExpr
if groupExpr != "" {
if err := validateConcurrencyGroupExpression(groupExpr); err != nil {
return formatCompilerError(markdownPath, "error", "workflow-level concurrency validation failed: "+err.Error(), err)
Expand Down
11 changes: 11 additions & 0 deletions pkg/workflow/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ var toolsLog = logger.New("workflow:tools")
func (c *Compiler) applyDefaults(data *WorkflowData, markdownPath string) error {
toolsLog.Printf("Applying defaults to workflow: name=%s, path=%s", data.Name, markdownPath)

// Populate cached values after all mutations to Permissions and Concurrency have been applied.
// Using defer ensures the cache is always set on every return path, including early returns.
// applyDefaults is the final stage that mutates data.Permissions (setting defaults and
// injecting feature-flag permissions), so the values computed here represent the stable,
// final state that validateWorkflowData will use. These caches eliminate repeated
// YAML parsing and regex extraction in the hot validateWorkflowData loop.
defer func() {
data.CachedPermissions = NewPermissionsParser(data.Permissions).ToPermissions()
data.ConcurrencyGroupExpr = extractConcurrencyGroupFromYAML(data.Concurrency)
}()

// Check if this is a command trigger workflow (by checking if user specified "on.command")
isCommandTrigger := false
isLabelCommandTrigger := false
Expand Down
Loading