refactor(workflow): extract helpers to fix function-length lint violations

docs/adr/44008-extract-helpers-comply-largefunc-lint-limit.md

@@ -0,0 +1,48 @@
+# ADR-44008: Extract Package-Private Helpers to Comply with the `largefunc` Lint Limit
+
+**Date**: 2026-07-07
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+The repository enforces a `largefunc` lint rule capping function bodies at 60 lines. Four functions in `pkg/workflow` exceeded this limit after organic growth: `ResolveSHA` (67 lines), `computeAntigravityToolsCore` (65 lines), `buildInputSchema` (69 lines), and `getShellEnvironmentPolicyVars` (61 lines). All four contained self-contained logical sub-problems—semver pin lookup, bash tool expansion, JSON Schema property construction, and MCP env-var mapping—that were implemented as inline blocks. Leaving these violations unaddressed blocks the lint gate and defers the structural clarity cost indefinitely.
+
+### Decision
+
+We will extract each over-limit inline block into a dedicated, package-private helper function co-located in the same source file, with no changes to observable behavior. The helpers (`lookupEmbeddedActionPin`, `appendBashTools`, `buildChoiceInputProperty`, `buildInputProperty`, `addMCPToolEnvVars`) own exactly one sub-problem each and are named after their responsibility rather than their caller. This satisfies the `largefunc` lint limit without raising the threshold or suppressing the check.
+
+### Alternatives Considered
+
+#### Alternative 1: Raise or Suppress the `largefunc` Threshold
+
+The lint threshold could be increased (e.g., from 60 to 80 lines) or suppressed with a per-file `//nolint` comment for the four affected files. This avoids any code restructuring and is trivially fast to apply.
+
+Rejected because it signals that the lint rule is optional and sets a precedent for future exemptions. The four functions each contained a genuinely separable concern; the lint violation was a signal worth acting on rather than silencing.
+
+#### Alternative 2: Promote Helpers to a New Sub-Package
+
+The extracted logic (semver pin matching, bash tool expansion, MCP env-var mapping) could be placed in separate sub-packages (e.g., `pkg/workflow/actionpin`, `pkg/workflow/bashtools`) to give them stronger encapsulation and independent test files.
+
+Rejected because the helpers are not reused outside `pkg/workflow` and the sub-package boundary would add import indirection with no practical benefit. Package-private functions in the same file achieve the same readability improvement with less structural overhead.
+
+### Consequences
+
+#### Positive
+- All four functions now comply with the 60-line `largefunc` lint rule, unblocking the lint gate.
+- Each extracted helper is independently unit-testable without constructing the full parent receiver.
+- Callers of the parent functions are shorter and read at a higher level of abstraction.
+
+#### Negative
+- The package now has five additional unexported functions, slightly increasing its exported-symbol footprint as seen by IDEs and documentation tools.
+- Readers who want to understand a full call path must navigate one extra indirection for each extracted helper.
+
+#### Neutral
+- All helpers are package-private (`lowercase` names) and co-located with their caller; no public API surface changes.
+- The refactoring carries zero behavior change risk, but reviewers still need to verify the extraction did not alter edge-case handling (e.g., the early-`continue` logic in `buildInputSchema`'s choice branch).
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/workflow/action_resolver.go

@@ -113,29 +113,12 @@ func (r *ActionResolver) ResolveSHA(ctx context.Context, repo, version string) (
 	resolverLog.Printf("Cache miss for %s@%s, checking embedded action pins", repo, version)
 
 	// Check embedded action pins for a semver-compatible version before making
-	// a network call. The embedded pins are the source-of-truth for known versions
-	// and are always available without network access. This avoids a ~1s gh-api
-	// subprocess for any action that is already covered by the embedded pin set.
-	requested := semverutil.EnsureVPrefix(version)
-	requestedVer := semverutil.ParseVersion(requested)
-	requestedIsPrecise := requestedVer != nil && requestedVer.IsPreciseVersion()
-
-	for _, pin := range actionpins.GetActionPinsByRepo(repo) {
-		pinVersion := semverutil.EnsureVPrefix(pin.Version)
-		if requestedIsPrecise {
-			if pinVersion != requested {
-				continue
-			}
-		} else if !semverutil.IsCompatible(pinVersion, requested) {
-			continue
-		}
-
-		resolverLog.Printf("Embedded pin hit for %s@%s → %s (%s)", repo, version, pin.SHA, pin.Version)
-		// Note: we intentionally do NOT call r.cache.Set() here. The embedded pins
-		// are always available in memory so there is nothing to persist, and writing
-		// to the on-disk cache would create root-owned files when compiling inside
-		// Docker containers (e.g. the Alpine CI test), preventing cleanup by the host.
-		return pin.SHA, nil
+	// a network call. Note: we intentionally do NOT call r.cache.Set() for
+	// embedded pins — they are always available in memory, and writing to the
+	// on-disk cache would create root-owned files inside Docker containers
+	// (e.g. the Alpine CI test), preventing cleanup by the host.
+	if sha, found := lookupEmbeddedActionPin(repo, version); found {
+		return sha, nil
 	}
 
 	resolverLog.Printf("No embedded pin for %s@%s, querying GitHub API", repo, version)
@@ -160,6 +143,33 @@ func (r *ActionResolver) ResolveSHA(ctx context.Context, repo, version string) (
 	return sha, nil
 }
 
+// lookupEmbeddedActionPin returns the pinned SHA for repo@version from the
+// embedded action pin set. It returns ("", false) if no matching pin is found.
+// The embedded pins are the source-of-truth for known versions and are always
+// available without network access, avoiding a ~1s gh-api subprocess for any
+// action covered by the embedded pin set.
+func lookupEmbeddedActionPin(repo, version string) (string, bool) {
+	requested := semverutil.EnsureVPrefix(version)
+	requestedVer := semverutil.ParseVersion(requested)
+	requestedIsPrecise := requestedVer != nil && requestedVer.IsPreciseVersion()
+
+	for _, pin := range actionpins.GetActionPinsByRepo(repo) {
+		pinVersion := semverutil.EnsureVPrefix(pin.Version)
+		if requestedIsPrecise {
+			if pinVersion != requested {
+				continue
+			}
+		} else if !semverutil.IsCompatible(pinVersion, requested) {
+			continue
+		}
+
+		resolverLog.Printf("Embedded pin hit for %s@%s → %s (%s)", repo, version, pin.SHA, pin.Version)
+		return pin.SHA, true
+	}
+
+	return "", false
+}
+
 // ParseTagRefTSV parses the tab-separated output from the GitHub API
 // `[.object.sha, .object.type] | @tsv` jq expression.
 // It returns the object SHA and type, or an error if the output is malformed.

pkg/workflow/action_resolver_test.go

@@ -115,6 +115,49 @@ func TestActionResolverFailedResolutionCache(t *testing.T) {
 // Note: Testing the actual GitHub API resolution requires network access
 // and is tested in integration tests or with network-dependent test tags
 
+// TestLookupEmbeddedActionPin exercises the semver matching logic inside
+// lookupEmbeddedActionPin directly — independently of the cache and network
+// layers. It covers the three reachable code paths: precise-version match,
+// range (major/minor) match, and no match.
+func TestLookupEmbeddedActionPin(t *testing.T) {
+	t.Run("precise version returns SHA", func(t *testing.T) {
+		// actions/checkout@v7.0.0 is in the embedded pin set.
+		sha, found := lookupEmbeddedActionPin("actions/checkout", "v7.0.0")
+		if !found {
+			t.Fatal("expected embedded pin hit for actions/checkout@v7.0.0, got not-found")
+		}
+		if sha == "" {
+			t.Error("expected non-empty SHA for actions/checkout@v7.0.0")
+		}
+	})
+
+	t.Run("semver range returns SHA for compatible pin", func(t *testing.T) {
+		// v7 is compatible with the pinned v7.0.0.
+		sha, found := lookupEmbeddedActionPin("actions/checkout", "v7")
+		if !found {
+			t.Fatal("expected embedded pin hit for actions/checkout@v7 (compatible with v7.0.0), got not-found")
+		}
+		if sha == "" {
+			t.Error("expected non-empty SHA for actions/checkout@v7")
+		}
+	})
+
+	t.Run("unknown repo returns not-found", func(t *testing.T) {
+		_, found := lookupEmbeddedActionPin("nonexistent/action", "v1")
+		if found {
+			t.Error("expected not-found for nonexistent/action@v1")
+		}
+	})
+
+	t.Run("known repo but incompatible version returns not-found", func(t *testing.T) {
+		// actions/checkout has no v1.x.x in the embedded pin set.
+		_, found := lookupEmbeddedActionPin("actions/checkout", "v1")
+		if found {
+			t.Error("expected not-found for actions/checkout@v1 (no such pin)")
+		}
+	})
+}
+
 // TestParseTagRefTSV verifies that ParseTagRefTSV correctly parses the tab-separated
 // output produced by the GitHub API jq expression `[.object.sha, .object.type] | @tsv`.
 // This is the core parsing step used when resolving action tags to SHAs; it must

pkg/workflow/antigravity_tools.go

@@ -56,38 +56,7 @@ func computeAntigravityToolsCore(tools map[string]any) []string {
 
 	// Map bash neutral tool to run_shell_command
 	if bashConfig, hasBash := tools["bash"]; hasBash {
-		bashCommands, ok := bashConfig.([]any)
-		if !ok || len(bashCommands) == 0 {
-			// bash with no specific commands - allow all shell commands
-			antigravityToolsLog.Print("bash (no specific commands) → run_shell_command")
-			toolsCore = append(toolsCore, "run_shell_command")
-		} else {
-			// Check for wildcard (* or :*)
-			hasWildcard := false
-			for _, cmd := range bashCommands {
-				if cmdStr, ok := cmd.(string); ok && (cmdStr == "*" || cmdStr == ":*") {
-					hasWildcard = true
-					break
-				}
-			}
-			if hasWildcard {
-				antigravityToolsLog.Print("bash wildcard → run_shell_command")
-				toolsCore = append(toolsCore, "run_shell_command")
-			} else {
-				// Add an entry for each specific command: run_shell_command(cmd)
-				for _, cmd := range bashCommands {
-					if cmdStr, ok := cmd.(string); ok {
-						// Normalize trailing " *" wildcard (e.g. "jq *" → "jq") so that
-						// all engines emit the canonical prefix form (run_shell_command(jq))
-						// regardless of whether the command was written with or without the wildcard.
-						normalized, _ := normalizeBashCommand(cmdStr)
-						entry := fmt.Sprintf("run_shell_command(%s)", normalized)
-						antigravityToolsLog.Printf("bash %q → %s", cmdStr, entry)
-						toolsCore = append(toolsCore, entry)
-					}
-				}
-			}
-		}
+		toolsCore = appendBashTools(toolsCore, bashConfig)
 	}
 
 	// Map edit neutral tool to write_file and replace (Antigravity's file write tools)
@@ -108,6 +77,43 @@ func computeAntigravityToolsCore(tools map[string]any) []string {
 	return toolsCore
 }
 
+// appendBashTools maps the bash neutral tool configuration to run_shell_command
+// entries and appends them to toolsCore.
+func appendBashTools(toolsCore []string, bashConfig any) []string {
+	bashCommands, ok := bashConfig.([]any)
+	if !ok || len(bashCommands) == 0 {
+		// bash with no specific commands - allow all shell commands
+		antigravityToolsLog.Print("bash (no specific commands) → run_shell_command")
+		return append(toolsCore, "run_shell_command")
+	}
+
+	// Single pass over bashCommands. A separate accumulator (specific) collects
+	// per-command entries so that if a wildcard ("*" or ":*") is found anywhere
+	// in the list — even after specific commands — only "run_shell_command" is
+	// appended and the pre-wildcard entries are discarded. This preserves the
+	// semantics of "any wildcard means allow all shell commands" regardless of
+	// command ordering.
+	var specific []string
+	for _, cmd := range bashCommands {
+		cmdStr, ok := cmd.(string)
+		if !ok {
+			continue
+		}
+		if cmdStr == "*" || cmdStr == ":*" {
+			antigravityToolsLog.Print("bash wildcard → run_shell_command")
+			return append(toolsCore, "run_shell_command")
+		}
+		// Normalize trailing " *" wildcard (e.g. "jq *" → "jq") so that
+		// all engines emit the canonical prefix form (run_shell_command(jq))
+		// regardless of whether the command was written with or without the wildcard.
+		normalized, _ := normalizeBashCommand(cmdStr)
+		entry := fmt.Sprintf("run_shell_command(%s)", normalized)
+		antigravityToolsLog.Printf("bash %q → %s", cmdStr, entry)
+		specific = append(specific, entry)
+	}
+	return append(toolsCore, specific...)
+}
+
 // generateAntigravitySettingsStep creates a GitHub Actions step that writes the
 // Antigravity CLI project settings file (.antigravity/settings.json) before execution.
 //

pkg/workflow/build_input_schema.go

@@ -46,34 +46,20 @@ func buildInputSchema(inputs map[string]any, descriptionFn func(inputName string
 			case "boolean":
 				inputType = "boolean"
 			case "choice":
-				inputType = "string"
-				if options, ok := inputDefMap["options"].([]any); ok && len(options) > 0 {
-					prop := map[string]any{
-						"type":        inputType,
-						"description": inputDescription,
-						"enum":        options,
-					}
-					if defaultVal, ok := inputDefMap["default"]; ok {
-						prop["default"] = defaultVal
-					}
+				if prop, isChoice := buildChoiceInputProperty(inputDefMap, inputDescription); isChoice {
 					properties[inputName] = prop
 					if inputRequired {
 						required = append(required, inputName)
 					}
 					continue
 				}
+				inputType = "string"
 			case "environment":
 				inputType = "string"
 			}
 		}
 
-		prop := map[string]any{
-			"type":        inputType,
-			"description": inputDescription,
-		}
-		if defaultVal, ok := inputDefMap["default"]; ok {
-			prop["default"] = defaultVal
-		}
+		prop := buildInputProperty(inputType, inputDescription, inputDefMap)
 		buildInputSchemaLog.Printf("Input %q: type=%s, required=%v", inputName, inputType, inputRequired)
 		properties[inputName] = prop
 
@@ -85,3 +71,34 @@ func buildInputSchema(inputs map[string]any, descriptionFn func(inputName string
 	buildInputSchemaLog.Printf("Built input schema: %d properties, %d required", len(properties), len(required))
 	return properties, required
 }
+
+// buildChoiceInputProperty builds a JSON Schema property for a "choice" input type.
+// It returns (property, true) when options are available, or (nil, false) otherwise.
+func buildChoiceInputProperty(inputDefMap map[string]any, inputDescription string) (map[string]any, bool) {
+	options, ok := inputDefMap["options"].([]any)
+	if !ok || len(options) == 0 {
+		return nil, false
+	}
+	prop := map[string]any{
+		"type":        "string",
+		"description": inputDescription,
+		"enum":        options,
+	}
+	if defaultVal, ok := inputDefMap["default"]; ok {
+		prop["default"] = defaultVal
+	}
+	return prop, true
+}
+
+// buildInputProperty builds a JSON Schema property map for a scalar input (string,
+// number, or boolean). Use buildChoiceInputProperty for "choice" inputs instead.
+func buildInputProperty(inputType, inputDescription string, inputDefMap map[string]any) map[string]any {
+	prop := map[string]any{
+		"type":        inputType,
+		"description": inputDescription,
+	}
+	if defaultVal, ok := inputDefMap["default"]; ok {
+		prop["default"] = defaultVal
+	}
+	return prop
+}