From 60a6bb09de73c9fdef51734259a0f94e8fe1a8cf Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 29 Mar 2026 21:32:02 +0000 Subject: [PATCH] Add debug logging to workflow pkg files lacking coverage Add logger declarations and meaningful debug log calls to five workflow package files that had no or minimal logging: - cache_integrity.go: new logger, log policy hash computation, cache key generation, and integrity level ordering - safe_outputs_tools_computation.go: new logger, log enabled tool count - validation_helpers.go: new logger, log mount format parsing and trigger lookup results - mcp_rendering.go: log renderer factory creation and standard renderer construction with key parameters - expression_nodes.go: log inline disjunction rendering term count Co-Authored-By: Claude Sonnet 4.6 --- pkg/workflow/cache_integrity.go | 24 ++++++++++++++----- pkg/workflow/expression_nodes.go | 1 + pkg/workflow/mcp_rendering.go | 2 ++ .../safe_outputs_tools_computation.go | 6 +++++ pkg/workflow/validation_helpers.go | 17 +++++++++---- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/pkg/workflow/cache_integrity.go b/pkg/workflow/cache_integrity.go index f67657e45fd..ec482770a90 100644 --- a/pkg/workflow/cache_integrity.go +++ b/pkg/workflow/cache_integrity.go @@ -6,8 +6,12 @@ import ( "fmt" "sort" "strings" + + "github.com/github/gh-aw/pkg/logger" ) +var cacheIntegrityLog = logger.New("workflow:cache_integrity") + // integrityLevelOrder defines integrity levels from highest to lowest. // Used to determine which branches to merge down from when setting up cache. var integrityLevelOrder = []string{"merged", "approved", "unapproved", "none"} @@ -27,12 +31,15 @@ const noPolicySentinel = "nopolicy" // - Workflows without policy → sentinel value "nopolicy" (consistent key format) func computePolicyHash(github *GitHubToolConfig) string { if github == nil || github.MinIntegrity == "" { + cacheIntegrityLog.Print("No guard policy configured, using nopolicy sentinel") return noPolicySentinel } canonical := buildCanonicalPolicy(github) hash := sha256.Sum256([]byte(canonical)) - return hex.EncodeToString(hash[:])[:8] + result := hex.EncodeToString(hash[:])[:8] + cacheIntegrityLog.Printf("Computed policy hash: %s (min-integrity=%s)", result, github.MinIntegrity) + return result } // buildCanonicalPolicy builds the normalized string representation of the allow-only policy. @@ -183,16 +190,20 @@ func cacheIntegrityLevel(github *GitHubToolConfig) string { // memory-unapproved-7e4d9f12-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }} // memory-none-nopolicy-session-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }} func generateIntegrityAwareCacheKey(cacheID, integrityLevel, policyHash string) string { + var key string if cacheID == "default" || cacheID == "" { - return fmt.Sprintf( + key = fmt.Sprintf( "memory-%s-%s-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}", integrityLevel, policyHash, ) + } else { + key = fmt.Sprintf( + "memory-%s-%s-%s-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}", + integrityLevel, policyHash, cacheID, + ) } - return fmt.Sprintf( - "memory-%s-%s-%s-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}", - integrityLevel, policyHash, cacheID, - ) + cacheIntegrityLog.Printf("Generated integrity-aware cache key: cacheID=%s, integrityLevel=%s, policyHash=%s", cacheID, integrityLevel, policyHash) + return key } // higherIntegrityLevels returns the integrity levels that are higher than the given level, @@ -206,5 +217,6 @@ func higherIntegrityLevels(level string) []string { } result = append(result, l) } + cacheIntegrityLog.Printf("Higher integrity levels than %q: %v", level, result) return result } diff --git a/pkg/workflow/expression_nodes.go b/pkg/workflow/expression_nodes.go index 01ae034a56f..cbf5b916a97 100644 --- a/pkg/workflow/expression_nodes.go +++ b/pkg/workflow/expression_nodes.go @@ -98,6 +98,7 @@ func (d *DisjunctionNode) Render() string { return d.RenderMultiline() } + expressionNodesLog.Printf("Rendering inline disjunction with %d terms", len(d.Terms)) var parts []string for _, term := range d.Terms { parts = append(parts, term.Render()) diff --git a/pkg/workflow/mcp_rendering.go b/pkg/workflow/mcp_rendering.go index dc72e33be3f..1fb469d01c1 100644 --- a/pkg/workflow/mcp_rendering.go +++ b/pkg/workflow/mcp_rendering.go @@ -118,6 +118,7 @@ func renderStandardJSONMCPConfig( // The returned function accepts isLast as a parameter and creates a renderer with engine-specific // options derived from the provided parameters and workflowData at call time. func buildMCPRendererFactory(workflowData *WorkflowData, format string, includeCopilotFields, inlineArgs bool) func(bool) *MCPConfigRendererUnified { + mcpRenderingLog.Printf("Building MCP renderer factory: format=%s, copilotFields=%t, inlineArgs=%t", format, includeCopilotFields, inlineArgs) return func(isLast bool) *MCPConfigRendererUnified { return NewMCPConfigRenderer(MCPRendererOptions{ IncludeCopilotFields: includeCopilotFields, @@ -147,6 +148,7 @@ func buildStandardJSONMCPRenderers( webFetchIncludeTools bool, renderCustom RenderCustomMCPToolConfigHandler, ) MCPToolRenderers { + mcpRenderingLog.Printf("Building standard JSON MCP renderers: webFetchIncludeTools=%t", webFetchIncludeTools) return MCPToolRenderers{ RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) { createRenderer(isLast).RenderGitHubMCP(yaml, githubTool, workflowData) diff --git a/pkg/workflow/safe_outputs_tools_computation.go b/pkg/workflow/safe_outputs_tools_computation.go index 8c4183b9050..4502e1e2b73 100644 --- a/pkg/workflow/safe_outputs_tools_computation.go +++ b/pkg/workflow/safe_outputs_tools_computation.go @@ -1,11 +1,16 @@ package workflow +import "github.com/github/gh-aw/pkg/logger" + +var safeOutputsToolsComputationLog = logger.New("workflow:safe_outputs_tools_computation") + // computeEnabledToolNames returns the set of predefined tool names that are enabled // by the workflow's SafeOutputsConfig. Dynamic tools (dispatch-workflow, custom jobs, // call-workflow) are excluded because they are generated separately. func computeEnabledToolNames(data *WorkflowData) map[string]bool { enabledTools := make(map[string]bool) if data.SafeOutputs == nil { + safeOutputsToolsComputationLog.Print("No safe outputs configuration, returning empty tool set") return enabledTools } @@ -126,5 +131,6 @@ func computeEnabledToolNames(data *WorkflowData) map[string]bool { enabledTools["push_repo_memory"] = true } + safeOutputsToolsComputationLog.Printf("Computed %d enabled safe output tool names", len(enabledTools)) return enabledTools } diff --git a/pkg/workflow/validation_helpers.go b/pkg/workflow/validation_helpers.go index 5acb66e809b..ee57487de24 100644 --- a/pkg/workflow/validation_helpers.go +++ b/pkg/workflow/validation_helpers.go @@ -33,6 +33,8 @@ import ( "github.com/github/gh-aw/pkg/logger" ) +var validationHelpersLog = logger.New("workflow:validation_helpers") + // newValidationLogger creates a standardized logger for a validation domain. // It follows the naming convention "workflow:_validation" used across // all *_validation.go files. @@ -81,12 +83,15 @@ func validateIntRange(value, min, max int, fieldName string) error { func validateMountStringFormat(mount string) (source, dest, mode string, err error) { parts := strings.Split(mount, ":") if len(parts) != 3 { + validationHelpersLog.Printf("Invalid mount format: %q (expected 3 colon-separated parts, got %d)", mount, len(parts)) return "", "", "", errors.New("must follow 'source:destination:mode' format with exactly 3 colon-separated parts") } mode = parts[2] if mode != "ro" && mode != "rw" { + validationHelpersLog.Printf("Invalid mount mode: %q in %q (must be 'ro' or 'rw')", mode, mount) return parts[0], parts[1], parts[2], fmt.Errorf("mode must be 'ro' or 'rw', got %q", mode) } + validationHelpersLog.Printf("Valid mount: source=%s, dest=%s, mode=%s", parts[0], parts[1], mode) return parts[0], parts[1], parts[2], nil } @@ -131,18 +136,20 @@ func validateNoDuplicateIDs[T any](items []T, idFunc func(T) string, onDuplicate // - []any: "on: [push, ]" // - map[string]any: "on:\n : ..." func containsTrigger(onSection any, triggerName string) bool { + found := false switch on := onSection.(type) { case string: - return on == triggerName + found = on == triggerName case []any: for _, trigger := range on { if triggerStr, ok := trigger.(string); ok && triggerStr == triggerName { - return true + found = true + break } } case map[string]any: - _, ok := on[triggerName] - return ok + _, found = on[triggerName] } - return false + validationHelpersLog.Printf("containsTrigger: trigger=%s, found=%t", triggerName, found) + return found }