feat: support env field in shared imports#26113
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/14d973cd-001e-452b-bdad-c0583eab8e72 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/14d973cd-001e-452b-bdad-c0583eab8e72 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
env field in shared imports
|
@copilot lock file header should list the env variable and their source (main vs imported). If an imported works flows tries to override another env, raise error. Only main workflow is allowed to override env |
…on in lock file header Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d60942a3-f7ad-40af-99a5-dad4c23d4919 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…idance Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d60942a3-f7ad-40af-99a5-dad4c23d4919 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Adds first-class support for env: in shared imported workflows, merges imported env vars into compiled output with main-workflow precedence, and surfaces env provenance in lock file headers.
Changes:
- Allow and extract
env:from shared imports, carrying merged env + per-key source attribution through the import pipeline. - Merge imported env into the workflow’s
env:block during compilation and emit an “Env variables” section in the generated lock header. - Update docs and add unit tests validating env extraction/merging and header rendering.
Show a summary per file
| File | Description |
|---|---|
pkg/constants/constants.go |
Removes env from the shared-workflow forbidden field list. |
pkg/parser/import_processor.go |
Extends ImportsResult to include merged env data and env source attribution. |
pkg/parser/import_field_extractor.go |
Extracts env from imports into an accumulator + tracks per-key sources. |
pkg/parser/include_processor.go |
Treats env as a valid relaxed frontmatter field (suppresses warnings). |
pkg/parser/import_field_extractor_test.go |
Adds tests for env extraction and import/import env-key conflict behavior. |
pkg/workflow/imports.go |
Adds mergeEnv to combine imported env JSON with top-level env. |
pkg/workflow/compiler_orchestrator_workflow.go |
Wires env merging and populates WorkflowData.EnvSources. |
pkg/workflow/compiler_types.go |
Adds EnvSources to WorkflowData for header rendering. |
pkg/workflow/compiler_yaml.go |
Emits a lock-header “Env variables” section based on EnvSources. |
pkg/workflow/compiler_yaml_helpers_test.go |
Tests deterministic header output including env sources. |
pkg/workflow/imports_env_test.go |
Adds unit tests for mergeEnv. |
docs/src/content/docs/reference/imports.md |
Documents env: as an allowed import field and its merge semantics. |
.github/workflows/stale-repo-identifier.lock.yml |
Regenerated lock header to include env variables section. |
.github/workflows/contribution-check.lock.yml |
Regenerated lock header to include env variables section. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 14/14 changed files
- Comments generated: 6
| // 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") | ||
| } |
| // For conflicts between imports, later imports take precedence. | ||
| func mergeEnv(topEnv map[string]any, importedEnvJSON string) (map[string]any, error) { | ||
| importsLog.Printf("Merging env: topEnv=%d", len(topEnv)) | ||
| result := make(map[string]any) | ||
|
|
||
| // Merge imported env vars first (newline-separated JSON objects, later overrides earlier) |
| | `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. Imported values fill in unspecified keys. Later imports override earlier for conflicts. | |
| // Two imports with distinct keys — import-to-import conflicts are caught earlier in the | ||
| // importAccumulator before mergeEnv is called (see TestEnvFieldConflictBetweenImports in | ||
| // pkg/parser/import_field_extractor_test.go). The JSON passed to mergeEnv is therefore | ||
| // guaranteed to have no duplicate keys across different import lines. |
| // Add env vars listing with source attribution when any env var is present | ||
| if len(data.EnvSources) > 0 { | ||
| yaml.WriteString("#\n") | ||
| yaml.WriteString("# Env variables:\n") | ||
| // Sort keys for deterministic output | ||
| keys := make([]string, 0, len(data.EnvSources)) | ||
| for k := range data.EnvSources { | ||
| keys = append(keys, k) | ||
| } | ||
| sort.Strings(keys) | ||
| for _, k := range keys { | ||
| fmt.Fprintf(yaml, "# - %s: %s\n", k, data.EnvSources[k]) | ||
| } | ||
| } |
| // 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) | ||
| } |
🧪 Test Quality Sentinel ReportTest Quality Score: 77/100
Test Classification DetailsAll 9 tests analyzed
Test Inflation NoteTwo test files show a line-ratio above the 2:1 threshold versus their primary corresponding production file:
Suggestions (non-blocking)
|
|
@copilot conflicting imports are errors, only main can override. Review all comments. |
Generated by the Design Decision Gate workflow to capture the architectural decisions behind lifting the env restriction from shared workflow imports, the conflict-error semantics for import-import collisions, and the lock file header source-attribution mechanism. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (317 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft covers the three key design decisions captured in this PR:
What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
Note 🔒 Integrity filter blocked 1 itemThe following item were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
… for env imports Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a430bbb1-ce07-4954-acfe-71113f298028 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot merge main and recompile |
…ield-in-shared-imports # Conflicts: # docs/src/content/docs/reference/imports.md Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The
envfield was explicitly forbidden in shared workflow files, causing it to be silently ignored with a warning. This adds full support forenvin shared imports, merging imported env vars into the compiled lock file with main workflow values taking precedence.Changes
pkg/constants/constants.go— Remove"env"fromSharedWorkflowForbiddenFieldspkg/parser/import_processor.go— AddMergedEnv stringandMergedEnvSources map[string]stringtoImportsResultpkg/parser/import_field_extractor.go— Extractenvfield from each imported file; detect and error on import-import conflictspkg/parser/include_processor.go— Add"env"to the valid non-workflow frontmatter fields (suppresses spurious warning)pkg/workflow/imports.go— AddmergeEnv(): main workflow env vars override imports; conflicts between imports are caught before mergingpkg/workflow/compiler_types.go— AddEnvSources map[string]stringtoWorkflowDatafor source attributionpkg/workflow/compiler_orchestrator_workflow.go— Wire up env merging and source attribution after YAML section extractionpkg/workflow/compiler_yaml.go— Add frontmatter env vars listing to lock file header with source attributiondocs/src/content/docs/reference/imports.md— Documentenv:as an allowed import field with merge semanticsMerge Semantics
envvars override any imported value, no error raised(main workflow)or the import file path); programmatically-injected vars (e.g.OTEL_*) are not listedExample
Compiled
envblock:Lock file header:
If two imports define the same env var, compilation fails: