From 57fe30753f6e3bae566dbe7f9a953ecebd8205bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 25 Apr 2026 06:08:25 +0000 Subject: [PATCH 1/2] Initial plan From 5d32ae94f0e8e2afecee1e9c78f8ac6205437730 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 25 Apr 2026 06:35:38 +0000 Subject: [PATCH 2/2] fix: eliminate repeated YAML parsing in validateWorkflowData to resolve BenchmarkValidation regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add CachedPermissions *Permissions and ConcurrencyGroupExpr string to WorkflowData - Populate both caches in applyDefaults via defer (safe because applyDefaults is the final stage that mutates Permissions and Concurrency) - Use CachedPermissions in validatePermissions instead of re-parsing YAML every call - Use ConcurrencyGroupExpr in validateToolConfiguration instead of regex extraction - Add strings.Contains fast-path in validateWorkflowRunBranches to skip yaml.Unmarshal when the On field cannot contain a workflow_run trigger BenchmarkValidation: 35,423 ns/op (337 allocs) → 6,722 ns/op (40 allocs): -81% On CI (6x slower machine): ~227,548 ns/op → ~40,000 ns/op (below 60,542 ns/op historical) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e12d8c4d-acfe-471b-8d70-c3a062083c37 Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/agent_validation.go | 5 ++++- pkg/workflow/compiler_types.go | 2 ++ pkg/workflow/compiler_validators.go | 21 +++++++++++++-------- pkg/workflow/tools.go | 11 +++++++++++ 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index fedab12239e..886b1c325bf 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -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 } diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 8e77ecf56cc..da0e78c36a9 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -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[''].ports[''] }} 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. diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go index 5167c9636dd..820163b6e4a 100644 --- a/pkg/workflow/compiler_validators.go +++ b/pkg/workflow/compiler_validators.go @@ -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") @@ -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) diff --git a/pkg/workflow/tools.go b/pkg/workflow/tools.go index 49eca12e32d..229e0709e5a 100644 --- a/pkg/workflow/tools.go +++ b/pkg/workflow/tools.go @@ -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