Conversation
…ted import discovery
Allow ${{ github.aw.import-inputs.* }} expressions in the imports: section
of shared workflows by applying expression substitution before parsing the
frontmatter for nested import discovery in the BFS traversal.
Previously, substituteImportInputsInContent was only called inside
extractAllImportFields (after nested imports had already been discovered),
so with: values containing ${{ github.aw.import-inputs.* }} were passed
as literal strings to nested imports. The fix applies the substitution
earlier, before ExtractFrontmatterFromContent is called for nested import
discovery, so that forwarded inputs are correctly resolved.
Adds TestImportInputsForwardedToNestedImports to verify the fix.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/50450d68-2884-4f6d-91d1-7b2afe54671c
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
${{ github.aw.import-inputs.* }} expressions in imports: frontmatter before nested import discovery
There was a problem hiding this comment.
Pull request overview
Fixes multi-level workflow composition by ensuring ${{ github.aw.import-inputs.* }} expressions inside an imported file’s imports: frontmatter are substituted before BFS nested-import discovery, so nested imports receive resolved concrete with: values instead of literal expression strings.
Changes:
- Update BFS import processing to apply
import-schemadefaults + import-input substitution prior to frontmatter parsing for nested import discovery when the current import has inputs. - Add a regression test that composes consumer → intermediate → leaf workflows and verifies forwarded inputs are fully substituted in the compiled lock output.
Show a summary per file
| File | Description |
|---|---|
| pkg/parser/import_bfs.go | Substitutes import inputs before frontmatter extraction so nested imports get resolved with: values during BFS discovery. |
| pkg/workflow/imports_inputs_test.go | Adds test coverage for forwarding import-inputs through nested imports and ensuring no unresolved expressions remain. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
pkg/parser/import_bfs.go
Outdated
| contentForFrontmatter := string(content) | ||
| if len(item.inputs) > 0 { | ||
| inputsWithDefaults := applyImportSchemaDefaults(contentForFrontmatter, item.inputs) | ||
| contentForFrontmatter = substituteImportInputsInContent(contentForFrontmatter, inputsWithDefaults) | ||
| } |
There was a problem hiding this comment.
applyImportSchemaDefaults parses the file’s YAML frontmatter, and ExtractFrontmatterFromContent(contentForFrontmatter) parses it again immediately afterward. For large import graphs this adds avoidable YAML parsing overhead on every import-with-inputs. Consider refactoring to reuse a single parsed frontmatter/schema (e.g., extract import-schema once, compute defaults, substitute, then parse only what’s needed for nested import discovery), or adjust applyImportSchemaDefaults to accept an already-parsed frontmatter result.
|
@copilot review comments, add integration tests |
… import-inputs forwarding - Refactor applyImportSchemaDefaults to delegate to a new applyImportSchemaDefaultsFromFrontmatter helper that works with an already-parsed frontmatter map, avoiding a redundant YAML parse. - In the BFS loop, parse the original content once, then reuse that result to compute import-schema defaults before substitution (instead of calling applyImportSchemaDefaults which would parse again). Only the second parse (of substituted content) is necessary to see resolved nested imports. - Add two integration tests in pkg/parser testing forwarding of single and multiple inputs through an intermediate shared workflow. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2f21b3cc-a5e9-4e11-b5e7-21d18f0b3c87 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit 21f82e1: Review comment (double YAML parse): Refactored Integration tests: Added
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:
|
|
🩺 Examination complete! CI Failure Doctor has delivered the diagnosis. Prescription issued! 💊 |
🩺 CI Doctor DiagnosisChecked SummaryTwo checks are failing — Failing Checks
Detailed AnalysisBoth jobs fail in --- golden
+++ run
@@ -51,8 +51,8 @@
GH_AW_INFO_ENGINE_ID: "copilot"
GH_AW_INFO_ENGINE_NAME: "GitHub Copilot CLI"
GH_AW_INFO_MODEL: $\{\{ vars.GH_AW_MODEL_AGENT_COPILOT || 'auto' }}
- GH_AW_INFO_VERSION: "1.0.20"
- GH_AW_INFO_AGENT_VERSION: "1.0.20"
+ GH_AW_INFO_VERSION: "1.0.21"
+ GH_AW_INFO_AGENT_VERSION: "1.0.21"- run: \$\{RUNNER_TEMP}/gh-aw/actions/install_copilot_cli.sh 1.0.20
+ run: \$\{RUNNER_TEMP}/gh-aw/actions/install_copilot_cli.sh 1.0.21Why the mismatch?
Affected golden files:
This is not caused by the PR's changes (which only touch parser files: Recommended Fixes
Prevention TipsWhen golden file tests cover version-stamped output, a rebase onto main after a version bump always requires regenerating the golden files. Consider adding a note in the PR checklist or a Analysis Steps
|
|
@copilot apply comments |
All review comments were already applied in commit 21f82e1:
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 apply #25241 (comment) |
…thub-aw-import-inputs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…(1.0.20 → 1.0.21)" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6324e02e-a20b-442a-9182-5475820588da Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Applied in commit a2dddf6: merged 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:
|
Shared workflows can use
${{ github.aw.import-inputs.* }}intools:,safe-outputs:, etc., but expressions in theimports:section'swith:values were silently passed as literal strings to nested imports — making multi-level workflow composition impossible.Root cause
In
import_bfs.go, the BFS loop parsed a file's frontmatter to discover nested imports from the raw content, beforesubstituteImportInputsInContentwas ever called. By the time substitution happened (insideextractAllImportFields), the nested import queue was already populated with unresolved expressions as literal input values.Fix
pkg/parser/import_bfs.go— Parse the original content once, reuse that result to computeimport-schemadefaults via the newapplyImportSchemaDefaultsFromFrontmatterhelper, apply substitution, then re-parse only the substituted content so that nested-import discovery sees resolvedwith:values. This avoids the redundant YAML parse that the previous approach incurred.pkg/parser/import_field_extractor.go— Extracted the schema-lookup logic fromapplyImportSchemaDefaultsinto a newapplyImportSchemaDefaultsFromFrontmatter(frontmatter map[string]any, inputs map[string]any)helper that accepts an already-parsed frontmatter map, allowing callers to avoid a second YAML parse.pkg/workflow/imports_inputs_test.go— AddTestImportInputsForwardedToNestedImportscovering the multi-level composition scenario.pkg/parser/import_inputs_integration_test.go— Add two//go:build integrationtests (TestImportInputsForwardedToNestedImports_IntegrationandTestImportInputsMultipleForwardedToNestedImports_Integration) that exercise the parser API directly, verifying single-input and multi-input forwarding through an intermediate shared workflow.Example (now works)
A consumer passing
branch-name: "memory/my-workflow"will have that value correctly forwarded toshared/repo-memory.md.