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
4 changes: 3 additions & 1 deletion pkg/workflow/claude_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,9 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str
stepLines = append(stepLines, " id: agentic_execution")

// Add allowed tools comment before the run section
allowedToolsComment := e.generateAllowedToolsComment(e.computeAllowedClaudeToolsString(toolsWithMountedCLIs, workflowData.SafeOutputs, workflowData.CacheMemoryConfig, workflowData.MCPScripts), " ")
// Reuse the already-computed allowedTools string (computed earlier for --allowed-tools flag)
// to avoid redundant allocations from calling computeAllowedClaudeToolsString twice.
allowedToolsComment := e.generateAllowedToolsComment(allowedTools, " ")
if allowedToolsComment != "" {
// Split the comment into lines and add each line
commentLines := strings.Split(strings.TrimSuffix(allowedToolsComment, "\n"), "\n")
Expand Down
21 changes: 19 additions & 2 deletions pkg/workflow/claude_tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,27 @@ func (e *ClaudeEngine) generateAllowedToolsComment(allowedToolsStr string, inden
return ""
}

// Pre-size the builder using the exact output size:
// - header line: indent + "# Allowed tools (sorted):\n"
// - per tool: indent + "# - " + toolName + "\n"
// allowedToolsStr is comma-separated, so subtracting (len(tools)-1) gives the
// total bytes contributed by tool names alone.
toolNameBytes := len(allowedToolsStr) - (len(tools) - 1)
var comment strings.Builder
comment.WriteString(indent + "# Allowed tools (sorted):\n")
comment.Grow(
len(indent) +
len("# Allowed tools (sorted):\n") +
len(tools)*len(indent) +
len(tools)*len("# - \n") +
toolNameBytes,
)
comment.WriteString(indent)
comment.WriteString("# Allowed tools (sorted):\n")
for _, tool := range tools {
fmt.Fprintf(&comment, "%s# - %s\n", indent, tool)
comment.WriteString(indent)
comment.WriteString("# - ")
comment.WriteString(tool)
comment.WriteByte('\n')
}

return comment.String()
Expand Down
37 changes: 26 additions & 11 deletions pkg/workflow/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,51 @@ var featuresLog = logger.New("workflow:features")
// If workflowData is nil or has no features, it falls back to checking the environment variable only.
func isFeatureEnabled(flag constants.FeatureFlag, workflowData *WorkflowData) bool {
flagLower := strings.ToLower(strings.TrimSpace(string(flag)))
featuresLog.Printf("Checking if feature is enabled: %s", flagLower)
logEnabled := featuresLog.Enabled()
if logEnabled {
featuresLog.Printf("Checking if feature is enabled: %s", flagLower)
}
Comment on lines 19 to +23
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

featuresLog.Enabled() is called repeatedly within the same function (and within getFeatureValueFromFrontmatter). Since it just returns a bool, consider capturing it once in a local logEnabled := featuresLog.Enabled() and reusing it to reduce repetition and keep the logging guards easier to scan/maintain.

Copilot uses AI. Check for mistakes.

// First, check if the feature is explicitly set in frontmatter.
// Frontmatter values always take precedence.
if enabled, found := getFeatureValueFromFrontmatter(flagLower, workflowData); found {
if enabled, found := getFeatureValueFromFrontmatter(flagLower, workflowData, logEnabled); found {
return enabled
}

// Fall back to checking the environment variable
if isFeatureInEnvironment(flagLower) {
featuresLog.Printf("Feature found in GH_AW_FEATURES: %s=true", flagLower)
if isFeatureInEnvironment(flagLower, logEnabled) {
if logEnabled {
featuresLog.Printf("Feature found in GH_AW_FEATURES: %s=true", flagLower)
}
return true
}

featuresLog.Printf("Feature not found: %s=false", flagLower)
if logEnabled {
featuresLog.Printf("Feature not found: %s=false", flagLower)
}
return false
}

func getFeatureValueFromFrontmatter(flagLower string, workflowData *WorkflowData) (bool, bool) {
func getFeatureValueFromFrontmatter(flagLower string, workflowData *WorkflowData, logEnabled bool) (bool, bool) {
if workflowData == nil || workflowData.Features == nil {
return false, false
}

if value, exists := workflowData.Features[flagLower]; exists {
if enabled, found := parseFeatureValue(value); found {
featuresLog.Printf("Feature found in frontmatter: %s=%v", flagLower, enabled)
if logEnabled {
featuresLog.Printf("Feature found in frontmatter: %s=%v", flagLower, enabled)
}
return enabled, true
}
}

for key, value := range workflowData.Features {
if strings.ToLower(key) == flagLower {
if enabled, found := parseFeatureValue(value); found {
featuresLog.Printf("Feature found in frontmatter (case-insensitive): %s=%v", flagLower, enabled)
if logEnabled {
featuresLog.Printf("Feature found in frontmatter (case-insensitive): %s=%v", flagLower, enabled)
}
return enabled, true
}
}
Expand All @@ -69,14 +80,18 @@ func parseFeatureValue(value any) (bool, bool) {
return false, false
}

func isFeatureInEnvironment(flagLower string) bool {
func isFeatureInEnvironment(flagLower string, logEnabled bool) bool {
features := os.Getenv("GH_AW_FEATURES")
if features == "" {
featuresLog.Printf("Feature not found, GH_AW_FEATURES empty: %s=false", flagLower)
if logEnabled {
featuresLog.Printf("Feature not found, GH_AW_FEATURES empty: %s=false", flagLower)
}
return false
}

featuresLog.Printf("Checking GH_AW_FEATURES environment variable: %s", features)
if logEnabled {
featuresLog.Printf("Checking GH_AW_FEATURES environment variable: %s", features)
}
for feature := range strings.SplitSeq(features, ",") {
if strings.ToLower(strings.TrimSpace(feature)) == flagLower {
return true
Expand Down
Loading