From 338c680c5215d86981a3b5ffd0ca61eeee477848 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 16:09:42 +0000 Subject: [PATCH 1/3] Initial plan From cd7813ca45860318fae6c16f6156ea63120d3e8a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 16:34:14 +0000 Subject: [PATCH 2/3] Fix import-schema default value resolution in engine and safe-outputs sections When no explicit with: inputs were provided to a shared importable workflow, ${{ github.aw.import-inputs.* }} expressions in engine and safe-outputs frontmatter fields were not resolved to their import-schema default values. Root cause: applyImportSchemaDefaults and substituteImportInputsInContent were only called when len(item.inputs) > 0 in both extractAllImportFields and the BFS nested-import discovery phase. Fix: Always apply import-schema defaults regardless of whether explicit inputs were provided. If defaults exist, perform substitution and update all downstream conditions to use content-modification checks instead of input-count checks. Adds TestImportSchemaDefaultsNoExplicitInputs covering engine section defaults, safe-outputs section defaults, and uses/with syntax with no with block. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4d1ffef5-9661-481c-9132-ce6071a2c82f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/import_bfs.go | 18 +-- pkg/parser/import_field_extractor.go | 62 ++++----- pkg/workflow/import_schema_test.go | 185 +++++++++++++++++++++++++++ 3 files changed, 228 insertions(+), 37 deletions(-) diff --git a/pkg/parser/import_bfs.go b/pkg/parser/import_bfs.go index b2dc7373870..a4f87fae550 100644 --- a/pkg/parser/import_bfs.go +++ b/pkg/parser/import_bfs.go @@ -302,19 +302,21 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a result, err = ExtractFrontmatterFromContent(string(content)) } - // When the import provides 'with' inputs, apply expression substitution before - // discovering nested imports. This resolves ${{ github.aw.import-inputs.* }} + // Apply import-schema defaults before discovering nested imports, even when no + // explicit 'with:' inputs were provided. This resolves ${{ github.aw.import-inputs.* }} // expressions that appear in the 'with' values of nested imports, enabling // multi-level workflow composition. // We reuse the already-parsed frontmatter to extract import-schema defaults, // avoiding a second YAML parse inside applyImportSchemaDefaults. - if err == nil && result != nil && len(item.inputs) > 0 { + if err == nil && result != nil { inputsWithDefaults := applyImportSchemaDefaultsFromFrontmatter(result.Frontmatter, item.inputs) - substituted := substituteImportInputsInContent(string(content), inputsWithDefaults) - // Re-parse the substituted content so that nested-import discovery sees - // the resolved 'with' values instead of literal expression strings. - if reparse, rerr := ExtractFrontmatterFromContent(substituted); rerr == nil { - result = reparse + if len(inputsWithDefaults) > 0 { + substituted := substituteImportInputsInContent(string(content), inputsWithDefaults) + // Re-parse the substituted content so that nested-import discovery sees + // the resolved 'with' values instead of literal expression strings. + if reparse, rerr := ExtractFrontmatterFromContent(substituted); rerr == nil { + result = reparse + } } } if err != nil { diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index aa417018dfd..3eada751164 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -19,7 +19,7 @@ import ( type importAccumulator struct { toolsBuilder strings.Builder mcpServersBuilder strings.Builder - markdownBuilder strings.Builder // Only used for imports WITH inputs (compile-time substitution) + markdownBuilder strings.Builder // imports with substituted inputs or schema defaults (compile-time substitution) importPaths []string // Import paths for runtime-import macro generation stepsBuilder strings.Builder copilotSetupStepsBuilder strings.Builder // Steps from copilot-setup-steps.yml (inserted at start) @@ -82,26 +82,28 @@ func newImportAccumulator() *importAccumulator { func (acc *importAccumulator) extractAllImportFields(content []byte, item importQueueItem, visited map[string]bool) error { log.Printf("Extracting all import fields: path=%s, section=%s, inputs=%d, content_size=%d bytes", item.fullPath, item.sectionName, len(item.inputs), len(content)) - // When the import provides 'with' inputs, apply expression substitution to the raw - // content before any YAML or markdown processing. This enables ${{ github.aw.import-inputs.* }} - // expressions in the imported workflow's frontmatter fields (tools, runtimes, etc.) - // as well as in the markdown body. Array and map values are serialized as JSON so they - // produce valid YAML inline syntax (e.g. ["go","typescript"]). + // Apply import-schema defaults before any YAML or markdown processing, even when no + // explicit 'with:' inputs were provided by the importing workflow. This enables + // ${{ github.aw.import-inputs.* }} expressions in the imported workflow's frontmatter + // fields (engine, safe-outputs, tools, runtimes, etc.) and markdown body to resolve + // to their declared default values rather than remaining as literal strings. + // Array and map values are serialized as JSON so they produce valid YAML inline syntax. rawContent := string(content) - if len(item.inputs) > 0 { - // Apply import-schema defaults for any optional parameters not supplied by the caller, - // so that ${{ github.aw.import-inputs. }} expressions for defaulted parameters - // are replaced with their declared default values rather than left as literal strings. - inputsWithDefaults := applyImportSchemaDefaults(rawContent, item.inputs) + // Apply import-schema defaults for any optional parameters not supplied by the caller. + inputsWithDefaults := applyImportSchemaDefaults(rawContent, item.inputs) + if len(inputsWithDefaults) > 0 { rawContent = substituteImportInputsInContent(rawContent, inputsWithDefaults) + // Add resolved defaults to acc.importInputs so the compile-time markdown + // substitution pass (generatePrompt) also has access to them. + maps.Copy(acc.importInputs, inputsWithDefaults) } // Extract tools from imported file. - // When inputs are present we use the already-substituted content (to pick up any - // ${{ github.aw.import-inputs.* }} expressions in the tools/mcp-servers frontmatter) - // rather than re-reading the original file from disk. + // When content was modified by substitution (either explicit inputs or schema defaults), + // we use the already-substituted content (to pick up any ${{ github.aw.import-inputs.* }} + // expressions in the tools/mcp-servers frontmatter) rather than re-reading the original file. var toolsContent string - if len(item.inputs) > 0 { + if string(content) != rawContent { var err error toolsContent, err = extractToolsFromContent(rawContent) if err != nil { @@ -116,21 +118,22 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import } acc.toolsBuilder.WriteString(toolsContent + "\n") - // Track import path for runtime-import macro generation (only if no inputs). - // Imports with inputs must be inlined for compile-time substitution. + // Track import path for runtime-import macro generation (only if no substitution happened). + // Imports with substituted inputs (explicit or via schema defaults) must be inlined for + // compile-time substitution so that ${{ github.aw.import-inputs.* }} expressions are resolved. // Builtin paths (@builtin:…) are pure configuration — they carry no user-visible // prompt content and must not generate runtime-import macros. importRelPath := computeImportRelPath(item.fullPath, item.importPath) - if len(item.inputs) == 0 && !strings.HasPrefix(importRelPath, BuiltinPathPrefix) { - // No inputs and not a builtin - use runtime-import macro + if string(content) == rawContent && !strings.HasPrefix(importRelPath, BuiltinPathPrefix) { + // No substitution happened and not a builtin - use runtime-import macro acc.importPaths = append(acc.importPaths, importRelPath) log.Printf("Added import path for runtime-import: %s", importRelPath) - } else if len(item.inputs) > 0 { - // Has inputs - must inline for compile-time substitution. + } else if string(content) != rawContent { + // Content was modified by substitution - inline for compile-time substitution. // Extract markdown from the already-substituted content so that import-inputs // expressions embedded in the markdown body are resolved here. - log.Printf("Import %s has inputs - will be inlined for compile-time substitution", importRelPath) + log.Printf("Import %s has substituted inputs - will be inlined for compile-time substitution", importRelPath) markdownContent, err := ExtractMarkdownContent(rawContent) if err != nil { return fmt.Errorf("failed to extract markdown from imported file '%s': %w", item.fullPath, err) @@ -150,15 +153,16 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import // Parse frontmatter once to avoid redundant YAML parsing for each field extraction. // All subsequent field extractions use the pre-parsed result. - // When inputs are present we parse the already-substituted content so that all - // frontmatter fields (runtimes, mcp-servers, etc.) reflect the resolved values. - // For builtin files without inputs, use the process-level cache to avoid redundant - // YAML re-parsing (processIncludedFileWithVisited already populated this cache). - // Builtin files WITH inputs must skip the cache because input substitution modifies - // the content, so the cached (unsubstituted) result would be stale. + // When content was modified by substitution, parse the already-substituted content so + // that all frontmatter fields (runtimes, mcp-servers, engine, safe-outputs, etc.) + // reflect the resolved values. + // For builtin files where no substitution happened, use the process-level cache to + // avoid redundant YAML re-parsing (processIncludedFileWithVisited already populated it). + // Builtin files where substitution modified the content must skip the cache because + // the cached (unsubstituted) result would be stale. var parsed *FrontmatterResult var err error - if strings.HasPrefix(item.fullPath, BuiltinPathPrefix) && len(item.inputs) == 0 { + if strings.HasPrefix(item.fullPath, BuiltinPathPrefix) && string(content) == rawContent { parsed, err = ExtractFrontmatterFromBuiltinFile(item.fullPath, content) } else { parsed, err = ExtractFrontmatterFromContent(rawContent) diff --git a/pkg/workflow/import_schema_test.go b/pkg/workflow/import_schema_test.go index e141b08211d..5965633bd73 100644 --- a/pkg/workflow/import_schema_test.go +++ b/pkg/workflow/import_schema_test.go @@ -658,3 +658,188 @@ imports: } }) } + +// TestImportSchemaDefaultsNoExplicitInputs tests that import-schema default values are +// applied when no explicit 'with:' inputs are provided by the importing workflow. +// This verifies the fix where ${{ github.aw.import-inputs.* }} expressions in 'engine' +// and 'safe-outputs' sections were not resolved to their default values when no inputs +// were passed. +func TestImportSchemaDefaultsNoExplicitInputs(t *testing.T) { + tempDir := testutil.TempDir(t, "test-import-defaults-no-inputs-*") + + sharedDir := filepath.Join(tempDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatalf("Failed to create shared directory: %v", err) + } + + t.Run("engine section with schema default", func(t *testing.T) { + sharedPath := filepath.Join(sharedDir, "engine-defaults.md") + sharedContent := `--- +import-schema: + claude-max-turns: + type: integer + default: 10000 + description: Maximum number of chat iterations per run + +engine: + id: claude + max-turns: ${{ github.aw.import-inputs.claude-max-turns }} +--- + +# Shared Engine Workflow +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main-engine.md") + workflowContent := `--- +on: issues +permissions: + contents: read + issues: read +imports: + - shared/engine-defaults.md +--- + +# Main Workflow +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContent := string(lockFileContent) + + // The engine max-turns should be resolved to the default value (10000) + if strings.Contains(lockContent, "github.aw.import-inputs.claude-max-turns") { + t.Error("Lock file should not contain unresolved github.aw.import-inputs.claude-max-turns expression") + } + if !strings.Contains(lockContent, "10000") { + t.Errorf("Lock file should contain the default max-turns value 10000; got:\n%s", lockContent) + } + }) + + t.Run("safe-outputs section with schema default", func(t *testing.T) { + sharedPath := filepath.Join(sharedDir, "safe-outputs-defaults.md") + sharedContent := `--- +import-schema: + expires: + type: string + default: "3d" + description: "How long to keep discussions before expiry" + +safe-outputs: + create-discussion: + expires: ${{ github.aw.import-inputs.expires }} +--- + +# Shared Safe Outputs Workflow +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main-safe-outputs.md") + workflowContent := `--- +on: issues +permissions: + contents: read + issues: read +engine: copilot +imports: + - shared/safe-outputs-defaults.md +--- + +# Main Workflow +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContent := string(lockFileContent) + + // The safe-outputs expires should be resolved to the default value ("3d") + if strings.Contains(lockContent, "github.aw.import-inputs.expires") { + t.Error("Lock file should not contain unresolved github.aw.import-inputs.expires expression") + } + if !strings.Contains(lockContent, "3d") { + t.Errorf("Lock file should contain the default expires value '3d'; got:\n%s", lockContent) + } + }) + + t.Run("uses/with syntax with no with block", func(t *testing.T) { + sharedPath := filepath.Join(sharedDir, "uses-defaults.md") + sharedContent := `--- +import-schema: + model: + type: string + default: "claude-sonnet-4-5" + description: "AI model to use" + +engine: + id: claude + model: ${{ github.aw.import-inputs.model }} +--- + +# Shared Uses Workflow +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main-uses.md") + workflowContent := `--- +on: issues +permissions: + contents: read + issues: read +imports: + - uses: shared/uses-defaults.md +--- + +# Main Workflow +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContent := string(lockFileContent) + + if strings.Contains(lockContent, "github.aw.import-inputs.model") { + t.Error("Lock file should not contain unresolved github.aw.import-inputs.model expression") + } + if !strings.Contains(lockContent, "claude-sonnet-4-5") { + t.Errorf("Lock file should contain the default model value 'claude-sonnet-4-5'; got:\n%s", lockContent) + } + }) +} From f727a13ff190bc09830c5e91eb98b842fcd9da32 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 17:09:06 +0000 Subject: [PATCH 3/3] Address reviewer feedback: avoid double YAML parse, skip BFS reparse when unchanged, specific test assertions - Replace applyImportSchemaDefaults (which re-parsed internally) with an up-front parse + applyImportSchemaDefaultsFromFrontmatter to eliminate the redundant YAML parse in extractAllImportFields. Remove now-unused applyImportSchemaDefaults. Add wasSubstituted bool to avoid repeated string(content) != rawContent comparisons and extra re-parses for schema validation. - In import_bfs.go only reparse frontmatter when substituteImportInputsInContent actually changed the content (substituted != origContent), avoiding wasted YAML parses when defaults exist but no expressions are present. - Replace generic substring assertions in TestImportSchemaDefaultsNoExplicitInputs with field-specific patterns (GH_AW_MAX_TURNS: 10000, "expires":72, ANTHROPIC_MODEL: claude-sonnet-4-5) that won't false-positive on unrelated content in the lock file. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4d735341-f013-4324-9e89-a7b67a82d9fd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/import_bfs.go | 14 ++-- pkg/parser/import_field_extractor.go | 98 ++++++++++++---------------- pkg/workflow/import_schema_test.go | 18 ++--- 3 files changed, 62 insertions(+), 68 deletions(-) diff --git a/pkg/parser/import_bfs.go b/pkg/parser/import_bfs.go index a4f87fae550..3667e996943 100644 --- a/pkg/parser/import_bfs.go +++ b/pkg/parser/import_bfs.go @@ -311,11 +311,15 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a if err == nil && result != nil { inputsWithDefaults := applyImportSchemaDefaultsFromFrontmatter(result.Frontmatter, item.inputs) if len(inputsWithDefaults) > 0 { - substituted := substituteImportInputsInContent(string(content), inputsWithDefaults) - // Re-parse the substituted content so that nested-import discovery sees - // the resolved 'with' values instead of literal expression strings. - if reparse, rerr := ExtractFrontmatterFromContent(substituted); rerr == nil { - result = reparse + origContent := string(content) + substituted := substituteImportInputsInContent(origContent, inputsWithDefaults) + // Only re-parse when substitution actually changed the content. + // If no ${{ github.aw.import-inputs.* }} expressions were present, + // the content is unchanged and a YAML reparse would be wasteful. + if substituted != origContent { + if reparse, rerr := ExtractFrontmatterFromContent(substituted); rerr == nil { + result = reparse + } } } } diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index 3eada751164..0bec0c6e6fc 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -82,28 +82,47 @@ func newImportAccumulator() *importAccumulator { func (acc *importAccumulator) extractAllImportFields(content []byte, item importQueueItem, visited map[string]bool) error { log.Printf("Extracting all import fields: path=%s, section=%s, inputs=%d, content_size=%d bytes", item.fullPath, item.sectionName, len(item.inputs), len(content)) + // Parse frontmatter once from the original content. This parse is reused for + // import-schema default extraction and schema validation, avoiding redundant YAML parsing. + // For builtin files we use the process-level cache. + origContent := string(content) + var origParsed *FrontmatterResult + var origParseErr error + if strings.HasPrefix(item.fullPath, BuiltinPathPrefix) { + origParsed, origParseErr = ExtractFrontmatterFromBuiltinFile(item.fullPath, content) + } else { + origParsed, origParseErr = ExtractFrontmatterFromContent(origContent) + } + var origFm map[string]any + if origParseErr == nil { + origFm = origParsed.Frontmatter + } else { + origFm = make(map[string]any) + } + // Apply import-schema defaults before any YAML or markdown processing, even when no // explicit 'with:' inputs were provided by the importing workflow. This enables // ${{ github.aw.import-inputs.* }} expressions in the imported workflow's frontmatter // fields (engine, safe-outputs, tools, runtimes, etc.) and markdown body to resolve // to their declared default values rather than remaining as literal strings. // Array and map values are serialized as JSON so they produce valid YAML inline syntax. - rawContent := string(content) - // Apply import-schema defaults for any optional parameters not supplied by the caller. - inputsWithDefaults := applyImportSchemaDefaults(rawContent, item.inputs) + // We reuse the already-parsed frontmatter to avoid a second YAML parse. + inputsWithDefaults := applyImportSchemaDefaultsFromFrontmatter(origFm, item.inputs) + rawContent := origContent if len(inputsWithDefaults) > 0 { - rawContent = substituteImportInputsInContent(rawContent, inputsWithDefaults) + rawContent = substituteImportInputsInContent(origContent, inputsWithDefaults) // Add resolved defaults to acc.importInputs so the compile-time markdown // substitution pass (generatePrompt) also has access to them. maps.Copy(acc.importInputs, inputsWithDefaults) } + wasSubstituted := rawContent != origContent // Extract tools from imported file. // When content was modified by substitution (either explicit inputs or schema defaults), // we use the already-substituted content (to pick up any ${{ github.aw.import-inputs.* }} // expressions in the tools/mcp-servers frontmatter) rather than re-reading the original file. var toolsContent string - if string(content) != rawContent { + if wasSubstituted { var err error toolsContent, err = extractToolsFromContent(rawContent) if err != nil { @@ -125,11 +144,11 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import // prompt content and must not generate runtime-import macros. importRelPath := computeImportRelPath(item.fullPath, item.importPath) - if string(content) == rawContent && !strings.HasPrefix(importRelPath, BuiltinPathPrefix) { + if !wasSubstituted && !strings.HasPrefix(importRelPath, BuiltinPathPrefix) { // No substitution happened and not a builtin - use runtime-import macro acc.importPaths = append(acc.importPaths, importRelPath) log.Printf("Added import path for runtime-import: %s", importRelPath) - } else if string(content) != rawContent { + } else if wasSubstituted { // Content was modified by substitution - inline for compile-time substitution. // Extract markdown from the already-substituted content so that import-inputs // expressions embedded in the markdown body are resolved here. @@ -151,48 +170,29 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import } } - // Parse frontmatter once to avoid redundant YAML parsing for each field extraction. - // All subsequent field extractions use the pre-parsed result. - // When content was modified by substitution, parse the already-substituted content so - // that all frontmatter fields (runtimes, mcp-servers, engine, safe-outputs, etc.) - // reflect the resolved values. - // For builtin files where no substitution happened, use the process-level cache to - // avoid redundant YAML re-parsing (processIncludedFileWithVisited already populated it). - // Builtin files where substitution modified the content must skip the cache because - // the cached (unsubstituted) result would be stale. - var parsed *FrontmatterResult - var err error - if strings.HasPrefix(item.fullPath, BuiltinPathPrefix) && string(content) == rawContent { - parsed, err = ExtractFrontmatterFromBuiltinFile(item.fullPath, content) - } else { - parsed, err = ExtractFrontmatterFromContent(rawContent) - } + // Parse frontmatter from the (possibly substituted) content for field extraction. + // All subsequent field extractions use this pre-parsed result. + // When substitution changed the content, reparse from rawContent so that all + // frontmatter fields (runtimes, mcp-servers, engine, safe-outputs, etc.) reflect + // the resolved values. When content is unchanged we reuse origFm, which was already + // parsed above — for builtin files the cache also applies. var fm map[string]any - if err == nil { - fm = parsed.Frontmatter + if wasSubstituted { + if reparsed, rerr := ExtractFrontmatterFromContent(rawContent); rerr == nil { + fm = reparsed.Frontmatter + } else { + fm = make(map[string]any) + } } else { - fm = make(map[string]any) + fm = origFm } // Validate 'with'/'inputs' values against the imported workflow's 'import-schema' (if present). - // Run validation even when inputs is nil/empty so required fields can be detected. - // Use the ORIGINAL (unsubstituted) frontmatter for schema lookup so the import-schema + // Always use the ORIGINAL (unsubstituted) frontmatter for schema lookup so the import-schema // declaration itself is not affected by expression substitution. - if len(item.inputs) > 0 || string(content) != rawContent { - // When substitution happened, reload the original frontmatter for schema validation. - origParsed, origErr := ExtractFrontmatterFromContent(string(content)) - if origErr == nil { - if _, hasSchema := origParsed.Frontmatter["import-schema"]; hasSchema { - if err := validateWithImportSchema(item.inputs, origParsed.Frontmatter, item.importPath); err != nil { - return err - } - } - } - } else { - if _, hasSchema := fm["import-schema"]; hasSchema { - if err := validateWithImportSchema(item.inputs, fm, item.importPath); err != nil { - return err - } + if _, hasSchema := origFm["import-schema"]; hasSchema { + if err := validateWithImportSchema(item.inputs, origFm, item.importPath); err != nil { + return err } } @@ -698,18 +698,6 @@ func validateImportInputType(name string, value any, declaredType string, paramD return nil } -// applyImportSchemaDefaults reads the import-schema from rawContent and returns a copy -// of inputs augmented with default values for any schema parameters that are declared -// with a "default" field but not present in the provided inputs map. Parameters that -// are already in inputs are left unchanged. -func applyImportSchemaDefaults(rawContent string, inputs map[string]any) map[string]any { - parsed, err := ExtractFrontmatterFromContent(rawContent) - if err != nil { - return inputs - } - return applyImportSchemaDefaultsFromFrontmatter(parsed.Frontmatter, inputs) -} - // applyImportSchemaDefaultsFromFrontmatter applies import-schema defaults from an // already-parsed frontmatter map, avoiding a redundant YAML parse when the caller // has already extracted the frontmatter. Returns a copy of inputs augmented with diff --git a/pkg/workflow/import_schema_test.go b/pkg/workflow/import_schema_test.go index 5965633bd73..d870733ab55 100644 --- a/pkg/workflow/import_schema_test.go +++ b/pkg/workflow/import_schema_test.go @@ -720,12 +720,13 @@ imports: } lockContent := string(lockFileContent) - // The engine max-turns should be resolved to the default value (10000) + // The engine max-turns should be resolved to the default value (10000). + // In the compiled lock file the value appears as the GH_AW_MAX_TURNS env var. if strings.Contains(lockContent, "github.aw.import-inputs.claude-max-turns") { t.Error("Lock file should not contain unresolved github.aw.import-inputs.claude-max-turns expression") } - if !strings.Contains(lockContent, "10000") { - t.Errorf("Lock file should contain the default max-turns value 10000; got:\n%s", lockContent) + if !strings.Contains(lockContent, "GH_AW_MAX_TURNS: 10000") { + t.Errorf("Lock file should contain GH_AW_MAX_TURNS: 10000 from the schema default; got:\n%s", lockContent) } }) @@ -778,12 +779,13 @@ imports: } lockContent := string(lockFileContent) - // The safe-outputs expires should be resolved to the default value ("3d") + // The safe-outputs expires should be resolved to the default value ("3d" = 72 hours). + // In the compiled lock file the value appears as JSON in the safe-outputs config. if strings.Contains(lockContent, "github.aw.import-inputs.expires") { t.Error("Lock file should not contain unresolved github.aw.import-inputs.expires expression") } - if !strings.Contains(lockContent, "3d") { - t.Errorf("Lock file should contain the default expires value '3d'; got:\n%s", lockContent) + if !strings.Contains(lockContent, `"expires":72`) { + t.Errorf("Lock file should contain expires:72 (3d = 72h) from the schema default; got:\n%s", lockContent) } }) @@ -838,8 +840,8 @@ imports: if strings.Contains(lockContent, "github.aw.import-inputs.model") { t.Error("Lock file should not contain unresolved github.aw.import-inputs.model expression") } - if !strings.Contains(lockContent, "claude-sonnet-4-5") { - t.Errorf("Lock file should contain the default model value 'claude-sonnet-4-5'; got:\n%s", lockContent) + if !strings.Contains(lockContent, "ANTHROPIC_MODEL: claude-sonnet-4-5") { + t.Errorf("Lock file should contain ANTHROPIC_MODEL: claude-sonnet-4-5 from the schema default; got:\n%s", lockContent) } }) }