Split pkg/workflow/compiler_types.go by lifecycle: options, mutators, types

docs/adr/52109-split-compiler-types-by-lifecycle.md

@@ -0,0 +1,62 @@
+# ADR-52109: Split compiler_types.go by Lifecycle Responsibility
+
+**Date**: 2026-08-11
+**Status**: Draft
+**Deciders**: pelikhan (via copilot-swe-agent, PR #52109)
+
+---
+
+### Context
+
+`pkg/workflow/compiler_types.go` had grown to mix three distinct lifecycle categories in a single file:
+
+1. **Build-time functional options** — `CompilerOption` type, `With*` builder functions, and the `NewCompiler` constructor.
+2. **Post-construction runtime mutators/accessors** — ~38 `Set*/Get*` methods on `*Compiler` (e.g. `SetContext`, `SetStrictMode`, `GetSharedActionResolver`).
+3. **Pure type declarations** — the `Compiler` struct, `FileCreationTracker` interface, `logTypes`, `FileCreationTracker`, and `allowedDomain`.
+
+The file exceeded 400 lines and made it hard to locate any given responsibility. This also prevented adding tests alongside each group without placing everything in a single, oversized test file.
+
+### Decision
+
+We will split `pkg/workflow/compiler_types.go` into three focused files within the same Go package (`package workflow`), each owning exactly one lifecycle group:
+
+- **`compiler_options.go`** — `CompilerOption` type, `With*` builders, and `NewCompiler`.
+- **`compiler_mutators.go`** — all `*Compiler` setter/getter methods and lazily-initialized shared cache helpers (`ensureSharedActionCacheAndResolver`, `getSharedImportCache`).
+- **`compiler_types.go`** — the `Compiler` struct, `FileCreationTracker` interface, `logTypes`, and `allowedDomain`.
+
+This is a pure mechanical refactor with no behavior change. All symbols remain in the same Go package, so no import paths change.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep Everything in compiler_types.go
+
+Retain the status quo and leave all three lifecycle groups in one file. This avoids any file proliferation and is the zero-effort option.
+
+Rejected because the file already exceeded 400 lines and was on a growth trajectory. Mixing construction-time and runtime-mutation responsibilities in one file makes code review harder and obscures the public API surface for each lifecycle.
+
+#### Alternative 2: Split by Public vs. Private, Not by Lifecycle
+
+Group all public symbols in one file and all private helpers in another, regardless of their lifecycle role.
+
+Rejected because this does not capture the semantically important boundary between construction-time options (only called in `NewCompiler`) and post-construction mutators (called by callers after the compiler is built). The lifecycle split is the conceptually cleaner boundary for navigation and future extension.
+
+### Consequences
+
+#### Positive
+- Each file has a single clear responsibility; the correct file to open for any given symbol is immediately obvious from the file name.
+- Test files can be co-located with the files they cover (`compiler_options_test.go`, `compiler_mutators_test.go`), keeping tests close to the code they exercise.
+- The public mutator API (38 relocated symbols) is now isolated in `compiler_mutators.go`, making it easy to audit what callers can change post-construction.
+- Smaller individual files speed up code review of future changes to any one lifecycle group.
+
+#### Negative
+- More files to navigate: contributors unfamiliar with the split must learn which file owns which kind of symbol (options vs. mutators vs. types).
+- Future additions to `Compiler` require a judgment call about which file to place them in; the lifecycle boundary is not always clear-cut (e.g., `GetVersion` is a read-only accessor placed in `compiler_mutators.go` rather than `compiler_options.go`).
+
+#### Neutral
+- No behavior change — this is a pure mechanical refactor. Existing tests pass without modification.
+- `pkg/workflow/README.md` symbol table was updated to reference the new file names for the 38 relocated symbols.
+- The orphaned doc comment for `SkipIfMatchConfig` (whose type lives in `workflow_data.go`) was removed as part of the cleanup.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/workflow/compiler_mutators.go

@@ -0,0 +1,334 @@
+package workflow
+
+import (
+	"context"
+	"maps"
+	"os"
+
+	"github.com/github/gh-aw/pkg/parser"
+)
+
+// SetSkipValidation configures whether to skip schema validation
+func (c *Compiler) SetSkipValidation(skip bool) {
+	c.skipValidation = skip
+}
+
+// SetContext sets the context used for network operations such as SHA resolution.
+func (c *Compiler) SetContext(ctx context.Context) {
+	c.ctx = ctx
+}
+
+// SetModelPricingResolver registers a callback used to resolve pricing for models that are
+// not present in the embedded models.json catalog. The resolver receives the workflow's
+// inference provider and model name; it should return per-token pricing (USD) and true when
+// pricing is available, or (nil, false) when it is not. Injected by the cli package so that
+// the compiler can fetch missing pricing from models.dev without a circular import.
+func (c *Compiler) SetModelPricingResolver(fn func(ctx context.Context, provider, model string) (map[string]float64, bool)) {
+	c.modelPricingResolver = fn
+}
+
+// SetRequireDocker configures whether Docker must be available for container image validation.
+// When true, validation fails with an error if Docker is not installed or the daemon is not running.
+// When false (default), validation is silently skipped when Docker is unavailable.
+func (c *Compiler) SetRequireDocker(require bool) {
+	c.requireDocker = require
+}
+
+// SetQuiet configures whether to suppress success messages (for interactive mode)
+func (c *Compiler) SetQuiet(quiet bool) {
+	c.quiet = quiet
+}
+
+// SetBatchMode configures whether repetitive notices should be aggregated.
+func (c *Compiler) SetBatchMode(batchMode bool) {
+	c.batchMode = batchMode
+}
+
+// GetExperimentalFeatureUsage returns experimental feature usage counts collected in batch mode.
+func (c *Compiler) GetExperimentalFeatureUsage() map[string]int {
+	usage := make(map[string]int, len(c.featureUsage))
+	maps.Copy(usage, c.featureUsage)
+	return usage
+}
+
+// CopilotRequestsTipNeeded reports whether batch output should show the token-based inference tip.
+func (c *Compiler) CopilotRequestsTipNeeded() bool {
+	return c.copilotTipNeeded
+}
+
+// SetExperimentalFeatureUsage replaces the experimental feature usage map.
+// Intended for use in tests that need to exercise aggregation output.
+func (c *Compiler) SetExperimentalFeatureUsage(usage map[string]int) {
+	c.featureUsage = usage
+}
+
+// SetCopilotTipNeeded sets whether the Copilot billing tip should be shown.
+// Intended for use in tests that need to exercise aggregation output.
+func (c *Compiler) SetCopilotTipNeeded(needed bool) {
+	c.copilotTipNeeded = needed
+}
+
+// SetNoEmit configures whether to validate without generating lock files
+func (c *Compiler) SetNoEmit(noEmit bool) {
+	c.noEmit = noEmit
+}
+
+// SetApprove configures whether to skip safe update enforcement via the CLI --approve flag.
+// When true, safe update enforcement is disabled regardless of strict mode setting,
+// approving all changes.
+func (c *Compiler) SetApprove(approve bool) {
+	c.approve = approve
+}
+
+// SetForceStaged configures whether safe-outputs should always compile in staged mode.
+func (c *Compiler) SetForceStaged(force bool) {
+	c.forceStaged = force
+}
+
+// SetFileTracker sets the file tracker for tracking created files
+func (c *Compiler) SetFileTracker(tracker FileCreationTracker) {
+	c.fileTracker = tracker
+}
+
+// SetTrialMode configures whether to run in trial mode (suppresses safe outputs)
+func (c *Compiler) SetTrialMode(trialMode bool) {
+	c.trialMode = trialMode
+}
+
+// SetTrialLogicalRepoSlug configures the target repository for trial mode
+func (c *Compiler) SetTrialLogicalRepoSlug(repo string) {
+	c.trialLogicalRepoSlug = repo
+}
+
+// SetUseSamples configures whether to replace the agentic step with a
+// deterministic replay driver that feeds `samples` entries to the safe-outputs
+// MCP server via real `tools/call` JSON-RPC. Hidden feature used by
+// `gh aw compile --use-samples`.
+func (c *Compiler) SetUseSamples(use bool) {
+	c.useSamples = use
+}
+
+// SetStrictMode configures whether to enable strict validation mode
+func (c *Compiler) SetStrictMode(strict bool) {
+	c.strictMode = strict
+}
+
+// SetAllowActionRefs configures whether unresolved action refs are warnings.
+// When false (default), unresolved action refs are compiler errors.
+func (c *Compiler) SetAllowActionRefs(allow bool) {
+	c.allowActionRefs = allow
+}
+
+// SetGHESCompat enables GHES compatibility mode via the --ghes CLI flag.
+// It overrides the aw.json ghes field for the current compilation run.
+// Artifact actions still use the latest non-v3 pins.
+func (c *Compiler) SetGHESCompat(enabled bool) {
+	c.ghesCompatFromCLI = enabled
+}
+
+// SetRefreshStopTime configures whether to force regeneration of stop-after times
+func (c *Compiler) SetRefreshStopTime(refresh bool) {
+	c.refreshStopTime = refresh
+}
+
+// SetForceRefreshActionPins configures whether to force refresh of action pins
+func (c *Compiler) SetForceRefreshActionPins(force bool) {
+	c.forceRefreshActionPins = force
+}
+
+// SetActionMode configures the action mode for JavaScript step generation
+func (c *Compiler) SetActionMode(mode ActionMode) {
+	c.actionMode = mode
+}
+
+// GetActionMode returns the current action mode
+func (c *Compiler) GetActionMode() ActionMode {
+	return c.actionMode
+}
+
+// SetActionTag sets the action tag override for actions/setup
+func (c *Compiler) SetActionTag(tag string) {
+	c.actionTag = tag
+}
+
+// GetActionTag returns the action tag override (empty if not set)
+func (c *Compiler) GetActionTag() string {
+	return c.actionTag
+}
+
+// SetActionsRepo sets the external actions repository override.
+// When set, this overrides the default "github/gh-aw-actions" repository used in action mode.
+func (c *Compiler) SetActionsRepo(repo string) {
+	c.actionsRepo = repo
+}
+
+// effectiveActionsRepo returns the actions repository to use for action mode references.
+// Returns the override if set, otherwise returns the default GitHubActionsOrgRepo constant.
+func (c *Compiler) effectiveActionsRepo() string {
+	if c.actionsRepo != "" {
+		return c.actionsRepo
+	}
+	return GitHubActionsOrgRepo
+}
+
+// EffectiveActionsRepo returns the actions repository used for action mode references.
+// Returns the override if set, otherwise returns the default GitHubActionsOrgRepo.
+func (c *Compiler) EffectiveActionsRepo() string {
+	return c.effectiveActionsRepo()
+}
+
+// GetVersion returns the version string used by the compiler
+func (c *Compiler) GetVersion() string {
+	return c.version
+}
+
+// IncrementWarningCount increments the warning counter
+func (c *Compiler) IncrementWarningCount() {
+	c.warningCount++
+}
+
+// GetWarningCount returns the current warning count
+func (c *Compiler) GetWarningCount() int {
+	return c.warningCount
+}
+
+// ResetWarningCount resets the warning counter to zero
+func (c *Compiler) ResetWarningCount() {
+	c.warningCount = 0
+}
+
+// SetWorkflowIdentifier sets the identifier for the current workflow being compiled
+// This is used for deterministic schedule scattering
+func (c *Compiler) SetWorkflowIdentifier(identifier string) {
+	c.workflowIdentifier = identifier
+}
+
+// SetRepositorySlug sets the repository slug for schedule scattering
+func (c *Compiler) SetRepositorySlug(slug string) {
+	c.repositorySlug = slug
+}
+
+// LockRepositorySlug marks the repository slug as explicitly set (e.g. via --schedule-seed)
+// so that per-file git-remote detection cannot override it.
+func (c *Compiler) LockRepositorySlug() {
+	c.repositorySlugLocked = true
+}
+
+// IsRepositorySlugLocked reports whether the repository slug has been locked
+// via LockRepositorySlug and must not be overridden by per-file detection.
+func (c *Compiler) IsRepositorySlugLocked() bool {
+	return c.repositorySlugLocked
+}
+
+// SetRepositorySlugIfUnlocked sets the repository slug only when it has not been
+// locked via LockRepositorySlug.  This is the method per-file git-remote detection
+// should call so that an explicit --schedule-seed flag is never overridden.
+func (c *Compiler) SetRepositorySlugIfUnlocked(slug string) {
+	if !c.repositorySlugLocked {
+		c.SetRepositorySlug(slug)
+	}
+}
+
+// GetRepositorySlug returns the repository slug (owner/repo) set on this compiler instance.
+func (c *Compiler) GetRepositorySlug() string {
+	return c.repositorySlug
+}
+
+// GetScheduleWarnings returns all accumulated schedule warnings for this compiler instance
+func (c *Compiler) GetScheduleWarnings() []string {
+	return c.scheduleWarnings
+}
+
+// AddSafeUpdateWarning appends a safe update warning to the compiler's accumulated list.
+// Callers should invoke this when a safe update violation is detected instead of
+// returning a compilation error, so that compilation still succeeds and the agent
+// receives actionable guidance.
+func (c *Compiler) AddSafeUpdateWarning(warning string) {
+	if c.safeUpdateWarnings == nil {
+		c.safeUpdateWarnings = []string{}
+	}
+	c.safeUpdateWarnings = append(c.safeUpdateWarnings, warning)
+}
+
+// GetSafeUpdateWarnings returns all accumulated safe update warnings for this compiler instance.
+func (c *Compiler) GetSafeUpdateWarnings() []string {
+	return c.safeUpdateWarnings
+}
+
+// SetPriorManifests replaces the entire pre-cached manifest map.
+func (c *Compiler) SetPriorManifests(manifests map[string]*GHAWManifest) {
+	if manifests == nil {
+		manifests = make(map[string]*GHAWManifest)
+	}
+	c.priorManifests = manifests
+}
+
+// ensureSharedActionCacheAndResolver lazily initializes (on first call) and returns the
+// compiler's shared ActionCache and ActionResolver pair. The resolver always wraps the
+// returned cache, so both values are initialized and returned together to keep that
+// pairing explicit; all workflows compiled by this compiler instance share the same
+// in-memory cache.
+func (c *Compiler) ensureSharedActionCacheAndResolver() (*ActionCache, *ActionResolver) {
+	if c.actionCache == nil {
+		// Initialize cache and resolver on first use
+		// Use git root if provided, otherwise fall back to current working directory
+		baseDir := c.gitRoot
+		if baseDir == "" {
+			cwd, err := os.Getwd()
+			if err != nil {
+				cwd = "."
+			}
+			baseDir = cwd
+		}
+		c.actionCache = NewActionCache(baseDir)
+
+		// Load existing cache unless force refresh is enabled
+		if !c.forceRefreshActionPins {
+			_ = c.actionCache.Load() // Ignore errors if cache doesn't exist
+		} else {
+			logTypes.Print("Force refresh action pins enabled: skipping cache load and will resolve all actions dynamically")
+			// Mark as cleared since we skipped loading
+			c.actionCacheCleared = true
+		}
+
+		c.actionResolver = NewActionResolver(c.actionCache)
+		logTypes.Print("Initialized shared action cache and resolver for compiler")
+	} else if c.forceRefreshActionPins && !c.actionCacheCleared {
+		// If cache already exists but force refresh is set and we haven't cleared it yet, clear it once
+		logTypes.Print("Force refresh action pins: clearing existing cache once for this run")
+		c.actionCache.Entries = make(map[string]ActionCacheEntry)
+		c.actionCacheCleared = true
+	}
+	return c.actionCache, c.actionResolver
+}
+
+// getSharedImportCache returns the shared import cache, initializing it on first use
+// This ensures all workflows compiled by this compiler instance share the same import cache
+func (c *Compiler) getSharedImportCache() *parser.ImportCache {
+	if c.importCache == nil {
+		// Initialize cache on first use
+		cwd, err := os.Getwd()
+		if err != nil {
+			cwd = "."
+		}
+		c.importCache = parser.NewImportCache(cwd)
+		logTypes.Print("Initialized shared import cache for compiler")
+	}
+	return c.importCache
+}
+
+// GetSharedActionCache returns the shared action cache used by this compiler instance.
+// The cache is lazily initialized on first access and shared across all workflows.
+// This allows action SHA validation and other operations to reuse cached resolutions.
+func (c *Compiler) GetSharedActionCache() *ActionCache {
+	cache, _ := c.ensureSharedActionCacheAndResolver()
+	return cache
+}
+
+// GetSharedActionResolver returns the shared action resolver used by this compiler instance.
+// The resolver is lazily initialized on first access and shared across all workflows.
+// It tracks which cache keys were used during compilation, enabling orphaned-entry pruning.
+func (c *Compiler) GetSharedActionResolver() *ActionResolver {
+	_, resolver := c.ensureSharedActionCacheAndResolver()
+	return resolver
+}