From 9828851c83b0ef2b6b250738b2561d68f7440362 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 06:58:10 +0000 Subject: [PATCH 1/2] Initial plan From 7f93fc2f700ee11871f1356593ab1e5a3771d72e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 07:25:33 +0000 Subject: [PATCH 2/2] Phase 5: Add RenderConfig hook to CodingAgentEngine interface - Add ConfigRenderer interface with RenderConfig method to agentic_engine.go - Add CodingAgentEngine to embed ConfigRenderer - Add BaseEngine.RenderConfig no-op (all four existing engines inherit it) - Call RenderConfig in setupEngineAndImports, store in engineSetupResult - Add EngineConfigSteps to WorkflowData - Emit config steps before execution steps in generateMainJobSteps - Add engine_config_render_test.go with 4 tests covering all acceptance criteria" Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/agentic_engine.go | 16 ++ pkg/workflow/compiler_orchestrator_engine.go | 12 ++ .../compiler_orchestrator_workflow.go | 1 + pkg/workflow/compiler_types.go | 1 + pkg/workflow/compiler_yaml_main_job.go | 14 ++ pkg/workflow/engine_config_render_test.go | 160 ++++++++++++++++++ 6 files changed, 204 insertions(+) create mode 100644 pkg/workflow/engine_config_render_test.go diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index 20f998609b3..c29ec83ae48 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -212,6 +212,15 @@ type AgentFileProvider interface { GetAgentManifestPathPrefixes() []string } +// ConfigRenderer is an optional hook that runtimes may implement to emit generated +// config files or metadata before execution steps run. +type ConfigRenderer interface { + // RenderConfig optionally generates runtime config files or metadata. + // Returns a slice of GitHub Actions steps that write config to disk. + // Implementations that don't need config files should return nil, nil. + RenderConfig(target *ResolvedEngineTarget) ([]map[string]any, error) +} + // CodingAgentEngine is a composite interface that combines all focused interfaces // This maintains backward compatibility with existing code while allowing more flexibility // Implementations can choose to implement only the interfaces they need by embedding BaseEngine @@ -223,6 +232,7 @@ type CodingAgentEngine interface { LogParser SecurityProvider ModelEnvVarProvider + ConfigRenderer } // BaseEngine provides common functionality for agentic engines @@ -358,6 +368,12 @@ func (e *BaseEngine) GetAgentManifestPathPrefixes() []string { return nil } +// RenderConfig returns nil by default — engines that need to write config files before +// execution (e.g. provider/model config files) should override this method. +func (e *BaseEngine) RenderConfig(_ *ResolvedEngineTarget) ([]map[string]any, error) { + return nil, nil +} + // convertStepToYAML converts a step map to YAML string - uses proper YAML serialization // This is a shared implementation inherited by all engines that embed BaseEngine func (e *BaseEngine) convertStepToYAML(stepMap map[string]any) (string, error) { diff --git a/pkg/workflow/compiler_orchestrator_engine.go b/pkg/workflow/compiler_orchestrator_engine.go index 6e582e85dca..ae3edb0bf94 100644 --- a/pkg/workflow/compiler_orchestrator_engine.go +++ b/pkg/workflow/compiler_orchestrator_engine.go @@ -21,6 +21,7 @@ type engineSetupResult struct { networkPermissions *NetworkPermissions sandboxConfig *SandboxConfig importsResult *parser.ImportsResult + configSteps []map[string]any // steps returned by RenderConfig (may be nil) } // setupEngineAndImports configures the AI engine, processes imports, and validates network/sandbox settings. @@ -223,6 +224,16 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean } agenticEngine := resolvedEngine.Runtime + // Call RenderConfig to allow the runtime adapter to emit config files or metadata. + // Most engines return nil, nil here; engines like OpenCode use this to write + // provider/model config files before the execution steps run. + orchestratorEngineLog.Printf("Calling RenderConfig for engine: %s", engineSetting) + configSteps, err := agenticEngine.RenderConfig(resolvedEngine) + if err != nil { + orchestratorEngineLog.Printf("RenderConfig failed for engine %s: %v", engineSetting, err) + return nil, fmt.Errorf("engine %s RenderConfig failed: %w", engineSetting, err) + } + log.Printf("AI engine: %s (%s)", agenticEngine.GetDisplayName(), engineSetting) if agenticEngine.IsExperimental() && c.verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Using experimental engine: "+agenticEngine.GetDisplayName())) @@ -285,5 +296,6 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean networkPermissions: networkPermissions, sandboxConfig: sandboxConfig, importsResult: importsResult, + configSteps: configSteps, }, nil } diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 44da10577a5..0e8478e0ec2 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -193,6 +193,7 @@ func (c *Compiler) buildInitialWorkflowData( HasExplicitGitHubTool: toolsResult.hasExplicitGitHubTool, ActionMode: c.actionMode, InlinedImports: inlinedImports, + EngineConfigSteps: engineSetup.configSteps, } // Populate checkout configs from parsed frontmatter. diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 2043ee51dd2..08ed1767889 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -436,6 +436,7 @@ type WorkflowData struct { HasDispatchItemNumber bool // true when workflow_dispatch has item_number input (generated by label trigger shorthand) ConcurrencyJobDiscriminator string // optional discriminator expression appended to job-level concurrency groups (from concurrency.job-discriminator) IsDetectionRun bool // true when this WorkflowData is used for inline threat detection (not the main agent run) + EngineConfigSteps []map[string]any // steps returned by engine.RenderConfig — prepended before execution steps } // BaseSafeOutputConfig holds common configuration fields for all safe output types diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 269662c4952..bd273526720 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -274,6 +274,20 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat yaml.WriteString(line) } + // Emit engine config steps (from RenderConfig) before the AI execution step. + // These steps write runtime config files to disk (e.g. provider/model config files). + // Most engines return no steps here; only engines that require config files use this. + if len(data.EngineConfigSteps) > 0 { + compilerYamlLog.Printf("Adding %d engine config steps for %s", len(data.EngineConfigSteps), engine.GetID()) + for _, step := range data.EngineConfigSteps { + stepYAML, err := ConvertStepToYAML(step) + if err != nil { + return fmt.Errorf("failed to render engine config step: %w", err) + } + yaml.WriteString(stepYAML) + } + } + // Add AI execution step using the agentic engine compilerYamlLog.Printf("Generating engine execution steps for %s", engine.GetID()) c.generateEngineExecutionSteps(yaml, data, engine, logFileFull) diff --git a/pkg/workflow/engine_config_render_test.go b/pkg/workflow/engine_config_render_test.go new file mode 100644 index 00000000000..0c41723ee8d --- /dev/null +++ b/pkg/workflow/engine_config_render_test.go @@ -0,0 +1,160 @@ +//go:build !integration + +package workflow + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// configRenderingEngine is a minimal test runtime adapter that overrides RenderConfig +// to emit a sentinel step. Used only in tests to validate that the orchestrator +// correctly prepends config steps before execution steps. +type configRenderingEngine struct { + BaseEngine + steps []map[string]any +} + +func newConfigRenderingEngine(steps []map[string]any) *configRenderingEngine { + return &configRenderingEngine{ + BaseEngine: BaseEngine{ + id: "config-renderer", + displayName: "Config Renderer (test)", + description: "Test engine that emits config steps via RenderConfig", + }, + steps: steps, + } +} + +// RenderConfig returns the pre-configured steps, overriding the BaseEngine no-op. +func (e *configRenderingEngine) RenderConfig(_ *ResolvedEngineTarget) ([]map[string]any, error) { + return e.steps, nil +} + +// GetInstallationSteps returns no installation steps for this test engine. +func (e *configRenderingEngine) GetInstallationSteps(_ *WorkflowData) []GitHubActionStep { + return nil +} + +// GetExecutionSteps returns a minimal sentinel execution step so tests can verify ordering. +func (e *configRenderingEngine) GetExecutionSteps(_ *WorkflowData, _ string) []GitHubActionStep { + return []GitHubActionStep{{" - name: config-renderer-exec"}} +} + +// TestRenderConfig_BuiltinEnginesReturnNil verifies that all four built-in engines +// return nil, nil from RenderConfig (backward-compatible no-op behaviour). +func TestRenderConfig_BuiltinEnginesReturnNil(t *testing.T) { + registry := NewEngineRegistry() + catalog := NewEngineCatalog(registry) + + engineIDs := []string{"claude", "codex", "copilot", "gemini"} + for _, id := range engineIDs { + t.Run(id, func(t *testing.T) { + resolved, err := catalog.Resolve(id, &EngineConfig{ID: id}) + require.NoError(t, err, "should resolve %s without error", id) + + steps, err := resolved.Runtime.RenderConfig(resolved) + require.NoError(t, err, "RenderConfig should not return an error for %s", id) + assert.Nil(t, steps, "RenderConfig should return nil steps for built-in engine %s", id) + }) + } +} + +// TestGenerateMainJobSteps_ConfigStepsBeforeExecution verifies that steps returned +// by RenderConfig are emitted before the AI execution steps in the compiled YAML. +func TestGenerateMainJobSteps_ConfigStepsBeforeExecution(t *testing.T) { + compiler := NewCompiler() + compiler.stepOrderTracker = NewStepOrderTracker() + + const configStepName = "Write engine config" + + data := &WorkflowData{ + Name: "Test Workflow", + AI: "copilot", + MarkdownContent: "Test prompt", + EngineConfig: &EngineConfig{ID: "copilot"}, + ParsedTools: &ToolsConfig{}, + EngineConfigSteps: []map[string]any{ + { + "name": configStepName, + "run": "echo 'provider = \"openai\"' > /tmp/config.toml", + }, + }, + } + + var yaml strings.Builder + err := compiler.generateMainJobSteps(&yaml, data) + require.NoError(t, err, "generateMainJobSteps should not error with EngineConfigSteps") + + result := yaml.String() + + // Verify the config step is present in the output. + assert.Contains(t, result, configStepName, "config step name should appear in YAML output") + + // Verify step ordering: the config step must appear before the Copilot execution step. + configIdx := strings.Index(result, configStepName) + execIdx := strings.Index(result, "Execute GitHub Copilot CLI") + require.GreaterOrEqual(t, configIdx, 0, "config step should be present in output") + require.GreaterOrEqual(t, execIdx, 0, "execution step should be present in output") + assert.Less(t, configIdx, execIdx, + "engine config step must appear before the AI execution step") +} + +// TestGenerateMainJobSteps_NoConfigSteps verifies that when EngineConfigSteps is nil +// the YAML output is unaffected (no spurious steps or errors). +func TestGenerateMainJobSteps_NoConfigSteps(t *testing.T) { + compiler := NewCompiler() + compiler.stepOrderTracker = NewStepOrderTracker() + + data := &WorkflowData{ + Name: "Test Workflow", + AI: "copilot", + MarkdownContent: "Test prompt", + EngineConfig: &EngineConfig{ID: "copilot"}, + ParsedTools: &ToolsConfig{}, + // EngineConfigSteps intentionally not set + } + + var yaml strings.Builder + err := compiler.generateMainJobSteps(&yaml, data) + require.NoError(t, err, "generateMainJobSteps should not error without EngineConfigSteps") + + result := yaml.String() + assert.Contains(t, result, "Execute GitHub Copilot CLI", + "execution step should still be present when no config steps are present") +} + +// TestOrchestratorCallsRenderConfig verifies that setupEngineAndImports invokes +// RenderConfig and stores the returned steps in engineSetupResult.configSteps. +func TestOrchestratorCallsRenderConfig(t *testing.T) { + const sentinelStepName = "sentinel-config-step" + + configSteps := []map[string]any{ + {"name": sentinelStepName, "run": "echo sentinel"}, + } + testEngine := newConfigRenderingEngine(configSteps) + + // Build a compiler whose registry contains the test engine. + registry := NewEngineRegistry() + registry.Register(testEngine) + catalog := NewEngineCatalog(registry) + + compiler := NewCompiler() + compiler.engineRegistry = registry + compiler.engineCatalog = catalog + + // Resolve the test engine to simulate what setupEngineAndImports does. + engineConfig := &EngineConfig{ID: testEngine.GetID()} + resolved, err := catalog.Resolve(testEngine.GetID(), engineConfig) + require.NoError(t, err, "test engine should resolve without error") + + // Call RenderConfig directly via the resolved runtime to verify the hook. + steps, err := resolved.Runtime.RenderConfig(resolved) + require.NoError(t, err, "RenderConfig should not error") + require.Len(t, steps, 1, "should return exactly one config step") + assert.Equal(t, sentinelStepName, steps[0]["name"], + "config step name should match sentinel value") +}