feat: support env field in shared imports

docs/adr/26113-env-field-support-in-shared-workflow-imports.md

@@ -0,0 +1,92 @@
+# ADR-26113: `env` Field Support in Shared Workflow Imports with Conflict-Error Semantics
+
+**Date**: 2026-04-14
+**Status**: Draft
+**Deciders**: pelikhan, Copilot
+
+---
+
+## Part 1 — Narrative (Human-Friendly)
+
+### Context
+
+The GitHub Agentic Workflows (gh-aw) compiler supports shared workflow files that are imported by main workflows. These shared files allow teams to extract reusable steps, tools, permissions, and other configuration into composable fragments. Prior to this change, the `env` field was explicitly listed in `SharedWorkflowForbiddenFields`, meaning any `env:` block in a shared import was silently dropped with a warning. This prevented shared workflows from declaring workflow-level environment variables (e.g., `TARGET_REPOSITORY`, `SHARED_CONFIG`), forcing workflow authors to duplicate those declarations in every consuming main workflow — violating the DRY principle and making shared workflows less self-contained.
+
+### Decision
+
+We will lift the `env` restriction from shared workflow imports and implement a three-tier precedence model: (1) the main workflow's `env` vars always win over any imported value, (2) import-import conflicts on the same variable name are a hard compilation error with an actionable message, and (3) when no conflict exists, imported env vars are merged into the compiled lock file. Additionally, the lock file header will list all env vars with source attribution (`(main workflow)` or the import file path) to aid auditability. This model makes the main workflow the single authoritative override point while enforcing explicit conflict resolution between imports.
+
+### Alternatives Considered
+
+#### Alternative 1: Last-Write-Wins Between Imports
+
+Import order could determine precedence when two shared files define the same env var (breadth-first topological order of the import graph). This would avoid surfacing an error but would make behavior silently dependent on the import declaration order, creating subtle, hard-to-debug bugs when shared files are reordered or when a new shared import is added that happens to define the same variable.
+
+#### Alternative 2: First-Write-Wins Between Imports
+
+Similar to last-write-wins but more predictable in practice (the first declared import "owns" the variable). Still suffers from the same silent ordering dependency; teams would have no visible signal that two imports conflict, leading to unexpected behavior when the import list is edited.
+
+#### Alternative 3: Keep `env` Forbidden in Shared Imports (Status Quo)
+
+Maintaining the current restriction is simple and avoids the complexity of conflict resolution entirely. However, it forces every consumer of a shared workflow to manually redeclare env vars that logically belong to the shared concern, making shared workflows less reusable and increasing the risk of drift between copies.
+
+#### Alternative 4: Allow All Overrides Without Source Attribution
+
+Merging could be done without tracking which import contributed which variable. This simplifies the implementation but sacrifices transparency: when a compiled workflow contains an unexpected env var value, there is no way to determine where it came from without re-reading every imported file.
+
+### Consequences
+
+#### Positive
+- Shared workflow files are more self-contained; env vars that logically belong to a reusable concern can be co-located with the rest of the shared configuration.
+- Import-import conflicts fail loudly at compile time with a clear, actionable error message instead of silently producing incorrect behavior.
+- Lock file headers now list all env vars with source attribution, improving auditability of compiled workflows.
+- The main workflow retains unconditional override authority, preserving the "main workflow is the source of truth" invariant already established for other merged fields.
+
+#### Negative
+- Workflow authors who accidentally duplicated the same env var across two shared imports will now get a compilation error they must resolve before their workflow compiles.
+- The `importAccumulator` struct gains two new fields (`envBuilder`, `envSources`) and the `ImportsResult` and `WorkflowData` types each gain new fields, increasing structural complexity.
+- The lock file header grows by the number of env vars, which may slightly increase generated file size.
+
+#### Neutral
+- The env merging approach (newline-separated JSON objects) follows the same internal serialisation convention already used for other merged fields (e.g., `MergedJobs`).
+- Existing workflows without `env:` in their shared imports are entirely unaffected; no migration is required.
+- The `include_processor.go` suppression-list update (adding `"env"` to valid non-workflow frontmatter fields) removes a spurious warning that users would have seen if they had `env:` in an included file.
+
+---
+
+## Part 2 — Normative Specification (RFC 2119)
+
+> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).
+
+### Env Field Allowance
+
+1. Shared workflow imports **MUST** be permitted to declare an `env:` field; the compiler **MUST NOT** treat `env` as a forbidden field in shared workflow files.
+2. The `env` key **MUST NOT** appear in `SharedWorkflowForbiddenFields`.
+3. The `env` field **MUST** be listed as a valid non-workflow frontmatter field in the include processor to suppress spurious unknown-field warnings.
+
+### Env Merge Precedence
+
+1. Env vars declared in the main workflow **MUST** take precedence over any env var from an imported file with the same key; the compiled output **MUST** use the main workflow's value.
+2. When two different imported files both declare the same env var key, the compiler **MUST** return a compilation error before producing any output.
+3. The compilation error for import-import conflicts **MUST** identify the conflicting variable name and both import file paths, and **SHOULD** include guidance on how to resolve the conflict (e.g., move the variable to the main workflow or remove it from one import).
+4. Imported env vars that do not conflict with each other and are not overridden by the main workflow **MUST** be merged into the compiled workflow's `env:` block.
+
+### Source Attribution
+
+1. The compiled lock file header **MUST** include an `# Env variables:` section listing every env var that is present in the merged env block.
+2. Each entry in the env variables section **MUST** be annotated with its source: `(main workflow)` if the variable originates from the main workflow file, or the import file path (relative to the repo root) if it originates from a shared import.
+3. Keys in the env variables header section **MUST** be emitted in sorted (lexicographic ascending) order for deterministic output.
+
+### Data Model
+
+1. `ImportsResult` **MUST** expose a `MergedEnv string` field containing the accumulated env var JSON from all imports, and a `MergedEnvSources map[string]string` field mapping each env var key to its originating import path.
+2. `WorkflowData` **MUST** expose an `EnvSources map[string]string` field mapping each env var key to its final source label (`(main workflow)` or import path) for use in lock file header generation.
+3. Implementations **MUST NOT** store the merged env blob in any format other than newline-separated JSON objects, consistent with the existing convention for other multi-import merged fields.
+
+### Conformance
+
+An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Specifically: the `env` field is accepted in shared imports without warning, import-import conflicts cause a hard compilation error with an informative message, main-workflow env vars always override imported values, and the compiled lock file header lists all env vars with source attribution in sorted order. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24374798953) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

docs/src/content/docs/reference/imports.md

@@ -319,6 +319,7 @@ Shared workflow files (without `on:` field) can define:
 - `permissions:` - GitHub Actions permissions (validated, not merged)
 - `runtimes:` - Runtime version overrides (node, python, go, etc.)
 - `secret-masking:` - Secret masking steps
+- `env:` - Workflow-level environment variables
 - `github-app:` - GitHub App credentials for token minting (centralize shared app config)
 
 Agent files (`.github/agents/*.md`) can additionally define:
@@ -345,6 +346,7 @@ Imports are processed using breadth-first traversal: direct imports first, then
 | `steps:` | Imported steps prepended to main; concatenated in import order. |
 | `jobs:` | Not merged — define only in the main workflow. Use `safe-outputs.jobs` for importable jobs. |
 | `safe-outputs.jobs` | Names must be unique; duplicates fail. Order determined by `needs:` dependencies. |
+| `env:` | Main workflow env vars take precedence over imports. Duplicate keys across different imports fail compilation — move to the main workflow to override imported values. |
 
 Example — `tools.bash.allowed` merging:
 

pkg/constants/constants.go

@@ -245,7 +245,7 @@ var IgnoredFrontmatterFields = []string{"user-invokable"}
 //   - Workflow triggers: on (defines it as a main workflow)
 //   - Workflow execution: command, run-name, runs-on, concurrency, if, timeout-minutes, timeout_minutes
 //   - Workflow metadata: name, tracker-id, strict
-//   - Workflow features: container, env, environment, sandbox, features
+//   - Workflow features: container, environment, sandbox, features
 //   - Access control: roles, github-token
 //
 // All other fields defined in main_workflow_schema.json can be used in shared workflows
@@ -255,7 +255,6 @@ var SharedWorkflowForbiddenFields = []string{
 	"command",         // Command for workflow execution
 	"concurrency",     // Concurrency control
 	"container",       // Container configuration
-	"env",             // Environment variables
 	"environment",     // Deployment environment
 	"features",        // Feature flags
 	"github-token",    // GitHub token configuration

pkg/parser/import_field_extractor.go

@@ -30,8 +30,10 @@ type importAccumulator struct {
 	permissionsBuilder       strings.Builder
 	secretMaskingBuilder     strings.Builder
 	postStepsBuilder         strings.Builder
-	jobsBuilder              strings.Builder // Jobs from imported YAML workflows
-	observabilityBuilder     strings.Builder // observability config (first-wins for OTLP endpoint)
+	jobsBuilder              strings.Builder   // Jobs from imported YAML workflows
+	envBuilder               strings.Builder   // env vars from imported workflows (JSON, one object per line)
+	envSources               map[string]string // env var name → source import path (for conflict detection and header listing)
+	observabilityBuilder     strings.Builder   // observability config (first-wins for OTLP endpoint)
 	engines                  []string
 	safeOutputs              []string
 	mcpScripts               []string
@@ -67,6 +69,7 @@ func newImportAccumulator() *importAccumulator {
 		skipRolesSet: make(map[string]bool),
 		skipBotsSet:  make(map[string]bool),
 		importInputs: make(map[string]any),
+		envSources:   make(map[string]string),
 	}
 }
 
@@ -339,6 +342,22 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import
 		acc.jobsBuilder.WriteString(jobsContent + "\n")
 	}
 
+	// Extract env from imported file (append in order; main workflow env takes precedence).
+	// Conflicts between two imports are disallowed — only the main workflow may override imported vars.
+	envContent, err := extractFieldJSONFromMap(fm, "env", "{}")
+	if err == nil && envContent != "" && envContent != "{}" {
+		var envMap map[string]any
+		if jsonErr := json.Unmarshal([]byte(envContent), &envMap); jsonErr == nil {
+			for key := range envMap {
+				if existingSource, exists := acc.envSources[key]; exists {
+					return fmt.Errorf("env variable %q is defined in multiple imports: %q and %q; remove the duplicate definition from one of the imports, or move it to the main workflow to override imported values", key, existingSource, item.importPath)
+				}
+				acc.envSources[key] = item.importPath
+			}
+			acc.envBuilder.WriteString(envContent + "\n")
+		}
+	}
+
 	// Extract labels from imported file (merge into set to avoid duplicates)
 	labelsContent, err := extractFieldJSONFromMap(fm, "labels", "[]")
 	if err == nil && labelsContent != "" && labelsContent != "[]" {
@@ -439,6 +458,8 @@ func (acc *importAccumulator) toImportsResult(topologicalOrder []string) *Import
 		MergedLabels:                acc.labels,
 		MergedCaches:                acc.caches,
 		MergedJobs:                  acc.jobsBuilder.String(),
+		MergedEnv:                   acc.envBuilder.String(),
+		MergedEnvSources:            acc.envSources,
 		MergedFeatures:              acc.features,
 		MergedObservability:         acc.observabilityBuilder.String(),
 		ImportedFiles:               topologicalOrder,

pkg/parser/import_field_extractor_test.go

@@ -202,6 +202,92 @@ imports:
 	assert.Contains(t, importsResult.MergedJobs, "ubuntu-slim", "MergedJobs should contain the job runner")
 }
 
+// TestEnvFieldExtractedFromMdImport verifies that env: in a shared .md workflow's
+// frontmatter is captured into ImportsResult.MergedEnv and merged correctly.
+func TestEnvFieldExtractedFromMdImport(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create a shared .md workflow with an env: section
+	sharedContent := `---
+env:
+  TARGET_REPOSITORY: owner/repo
+  SHARED_VAR: shared-value
+---
+
+# Shared workflow with env vars
+`
+	sharedDir := filepath.Join(tmpDir, "shared")
+	require.NoError(t, os.MkdirAll(sharedDir, 0755), "Failed to create shared dir")
+	require.NoError(t, os.WriteFile(filepath.Join(sharedDir, "target.md"), []byte(sharedContent), 0644), "Failed to write shared file")
+
+	// Create a main .md workflow that imports the shared workflow
+	mainContent := `---
+name: Main Workflow
+on: issue_comment
+imports:
+  - shared/target.md
+---
+
+# Main Workflow
+`
+	result, err := ExtractFrontmatterFromContent(mainContent)
+	require.NoError(t, err, "ExtractFrontmatterFromContent should succeed")
+
+	importsResult, err := ProcessImportsFromFrontmatterWithSource(result.Frontmatter, tmpDir, nil, "", "")
+	require.NoError(t, err, "ProcessImportsFromFrontmatterWithSource should succeed")
+
+	assert.NotEmpty(t, importsResult.MergedEnv, "MergedEnv should be populated from shared .md import")
+	assert.Contains(t, importsResult.MergedEnv, "TARGET_REPOSITORY", "MergedEnv should contain TARGET_REPOSITORY")
+	assert.Contains(t, importsResult.MergedEnv, "owner/repo", "MergedEnv should contain the repository value")
+	assert.Contains(t, importsResult.MergedEnv, "SHARED_VAR", "MergedEnv should contain SHARED_VAR")
+	assert.Equal(t, "shared/target.md", importsResult.MergedEnvSources["TARGET_REPOSITORY"], "MergedEnvSources should track the import path for TARGET_REPOSITORY")
+	assert.Equal(t, "shared/target.md", importsResult.MergedEnvSources["SHARED_VAR"], "MergedEnvSources should track the import path for SHARED_VAR")
+}
+
+// TestEnvFieldConflictBetweenImports verifies that defining the same env var in two different
+// imports produces a compilation error.
+func TestEnvFieldConflictBetweenImports(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	sharedDir := filepath.Join(tmpDir, "shared")
+	require.NoError(t, os.MkdirAll(sharedDir, 0755), "Failed to create shared dir")
+
+	// First import defines SHARED_KEY
+	require.NoError(t, os.WriteFile(filepath.Join(sharedDir, "first.md"), []byte(`---
+env:
+  SHARED_KEY: value-from-first
+---
+
+# First shared workflow
+`), 0644))
+
+	// Second import also defines SHARED_KEY (conflict)
+	require.NoError(t, os.WriteFile(filepath.Join(sharedDir, "second.md"), []byte(`---
+env:
+  SHARED_KEY: value-from-second
+---
+
+# Second shared workflow
+`), 0644))
+
+	mainContent := `---
+name: Main Workflow
+on: issue_comment
+imports:
+  - shared/first.md
+  - shared/second.md
+---
+
+# Main Workflow
+`
+	result, err := ExtractFrontmatterFromContent(mainContent)
+	require.NoError(t, err, "ExtractFrontmatterFromContent should succeed")
+
+	_, err = ProcessImportsFromFrontmatterWithSource(result.Frontmatter, tmpDir, nil, "", "")
+	require.Error(t, err, "Should error when two imports define the same env var")
+	assert.Contains(t, err.Error(), "SHARED_KEY", "Error should mention the conflicting variable name")
+}
+
 // TestExtractAllImportFields_BuiltinCacheHit verifies that extractAllImportFields uses the
 // process-level builtin frontmatter cache for builtin files without inputs.
 func TestExtractAllImportFields_BuiltinCacheHit(t *testing.T) {