Add configurable agentic engine driver script support#27453
Merged
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aad415e2-3500-4787-bb47-608b50147cfd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This reverts commit b8f81ce. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aad415e2-3500-4787-bb47-608b50147cfd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aad415e2-3500-4787-bb47-608b50147cfd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
April 20, 2026 23:02
View session
Collaborator
|
@copilot driver is available for any agentic engine |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ffcc5d11-c0c2-4d16-b4cd-46f9b8b1655d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Updated in a4ac9bc: 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
AI
changed the title
Add configurable Copilot engine driver script support
Add configurable agentic engine driver script support
Apr 21, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for configuring a custom agentic engine driver script via engine.driver, enabling Copilot workflows to replace the built-in copilot_driver.cjs, with validation, schema updates, and runtime detection adjustments (Node 24).
Changes:
- Parse
engine.driverfrom workflow frontmatter intoEngineConfig.DriverScriptand validate it’s a Node.js script extension. - Use the configured driver script in Copilot execution step generation instead of the built-in driver.
- Update runtime requirement detection, schema, and tests to cover parsing/validation/execution/runtime behavior.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/runtime_setup_test.go | Adds test coverage for Node 24 runtime requirement when a custom driver is configured. |
| pkg/workflow/runtime_detection.go | Forces Node runtime setup (default version) when engine.driver is present. |
| pkg/workflow/engine_validation_test.go | Adds tests for engine.driver validation behavior. |
| pkg/workflow/engine_validation.go | Implements engine.driver extension validation. |
| pkg/workflow/engine_config_test.go | Extends engine config extraction tests to include driver. |
| pkg/workflow/engine.go | Adds DriverScript to EngineConfig and extracts engine.driver from frontmatter. |
| pkg/workflow/copilot_engine_test.go | Verifies Copilot execution uses the configured custom driver instead of the built-in one. |
| pkg/workflow/copilot_engine_execution.go | Switches the driver script used by Copilot execution to the configured EngineConfig.DriverScript. |
| pkg/workflow/compiler_orchestrator_workflow.go | Hooks driver validation into workflow parsing. |
| pkg/parser/schemas/main_workflow_schema.json | Adds engine.driver field to the workflow schema. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/workflow/copilot_engine_execution.go:200
DriverScriptcomes from workflow frontmatter and is interpolated directly into a shell command (%s/%s) without escaping. A value containing whitespace, shell metacharacters, or path separators could break the command or enable shell injection/path traversal in the generatedrun:block. Consider validatingengine.driveras a safe basename (no/,\, whitespace,$, etc.; e.g.^[A-Za-z0-9._-]+\.(js|cjs|mjs)$) and/or constructing the command so the driver path is safely quoted while still allowing${RUNNER_TEMP}expansion.
driverScriptName := e.GetDriverScriptName()
if workflowData.EngineConfig != nil && workflowData.EngineConfig.DriverScript != "" {
driverScriptName = workflowData.EngineConfig.DriverScript
}
var execPrefix string
if driverScriptName != "" {
// Driver wraps the copilot subprocess; ${RUNNER_TEMP} and ${GH_AW_NODE_BIN} expand in the shell context.
execPrefix = fmt.Sprintf(`%s %s/%s %s`, nodeRuntimeResolutionCommand, SetupActionDestinationShell, driverScriptName, commandName)
- Files reviewed: 10/10 changed files
- Comments generated: 4
Comment on lines
+79
to
+92
| // validateEngineDriverScript validates optional engine.driver configuration. | ||
| // engine.driver must point to a Node.js script. | ||
| func (c *Compiler) validateEngineDriverScript(workflowData *WorkflowData) error { | ||
| if workflowData == nil || workflowData.EngineConfig == nil || workflowData.EngineConfig.DriverScript == "" { | ||
| return nil | ||
| } | ||
|
|
||
| ext := strings.ToLower(filepath.Ext(workflowData.EngineConfig.DriverScript)) | ||
| switch ext { | ||
| case ".js", ".cjs", ".mjs": | ||
| return nil | ||
| default: | ||
| return fmt.Errorf("engine.driver must be a Node.js script ending with .js, .cjs, or .mjs (found: %s).\n\nSee: %s", workflowData.EngineConfig.DriverScript, constants.DocsEnginesURL) | ||
| } |
There was a problem hiding this comment.
The current validation only checks the file extension. Since engine.driver is later used to build shell commands and file paths, it should also reject unsafe values (path separators, .., absolute paths, whitespace, shell metacharacters) to prevent command breakage/injection and directory traversal. Tightening validation to a safe filename pattern (and possibly documenting that it must be a basename located under the setup actions directory) would make this feature safer and more predictable.
Comment on lines
+42
to
+49
| // When a custom driver script is configured, require Node.js runtime setup | ||
| // with the default version so workflows consistently execute the driver with Node 24. | ||
| if workflowData.EngineConfig != nil && workflowData.EngineConfig.DriverScript != "" { | ||
| nodeRuntime := findRuntimeByID("node") | ||
| if nodeRuntime != nil { | ||
| updateRequiredRuntime(nodeRuntime, string(constants.DefaultNodeVersion), requirements) | ||
| } | ||
| } |
There was a problem hiding this comment.
DetectRuntimeRequirements adds a Node.js requirement whenever EngineConfig.DriverScript is set, regardless of which engine is selected. Currently the only execution path that consumes DriverScript is the Copilot engine (copilot_engine_execution.go), so setting engine.driver for other engines would unexpectedly force Node 24 setup without affecting execution. Consider scoping this to engines that actually support a Node driver wrapper (e.g., engine.id == "copilot" for now, or a capability check via the engine registry) to avoid surprising runtime setup.
Comment on lines
+249
to
+254
| // Extract optional 'driver' field (string - copilot engine only) | ||
| if driver, hasDriver := engineObj["driver"]; hasDriver { | ||
| if driverStr, ok := driver.(string); ok { | ||
| config.DriverScript = driverStr | ||
| } | ||
| } |
There was a problem hiding this comment.
Comment says driver is "copilot engine only", but ExtractEngineConfig will populate DriverScript for any engine object and validation/tests currently allow non-copilot engines too. Either enforce the restriction in code/validation, or update the comment to match the actual supported behavior.
| "description": "Custom executable path for the AI engine CLI. When specified, the workflow will skip the standard installation steps and use this command instead. The command should be the full path to the executable or a command available in PATH." | ||
| }, | ||
| "driver": { | ||
| "type": "string", |
There was a problem hiding this comment.
Schema text says engine.driver "must end with .js, .cjs, or .mjs", but the schema itself doesn’t enforce that constraint (e.g., via pattern). Adding a regex pattern here (and ideally also restricting to a safe basename) would keep schema validation consistent with validateEngineDriverScript and help catch invalid/unsafe values earlier.
Suggested change
| "type": "string", | |
| "type": "string", | |
| "pattern": "^(?!\\.)(?!.*[\\\\/])[A-Za-z0-9._-]+\\.(?:js|cjs|mjs)$", |
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 75/100
Test Classification DetailsView All Test Classifications (9 cases across 4 functions)
Flagged Tests — Suggestions for ImprovementNo tests are flagged as requiring changes. The following are improvement suggestions: 💡
|
Collaborator
|
@copilot Review all comments |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/00ba7f2e-61c3-433d-a6a0-79ac7a1155f1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/00ba7f2e-61c3-433d-a6a0-79ac7a1155f1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/00ba7f2e-61c3-433d-a6a0-79ac7a1155f1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/00ba7f2e-61c3-433d-a6a0-79ac7a1155f1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Reviewed and addressed the review feedback across validation, schema, runtime detection scope, and tests in commits 2c70445, 09d19e5, 68a27e8, and 1c59766. 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:
|
This was referenced Apr 21, 2026
Contributor
|
Hey The PR looks solid overall — the only thing worth a quick look before merge is the unstable CI state. The PR body correctly identifies four pre-existing failing tests ( Everything else looks 🟢:
|
Collaborator
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c7600ed5-a45c-4092-9029-8c99de7f2403 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c7600ed5-a45c-4092-9029-8c99de7f2403 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c7600ed5-a45c-4092-9029-8c99de7f2403 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c7600ed5-a45c-4092-9029-8c99de7f2403 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c7600ed5-a45c-4092-9029-8c99de7f2403 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c7600ed5-a45c-4092-9029-8c99de7f2403 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Fixed in c0de1a7 (with the core change in e26ac51): the failing action pin tests now derive setup-node expectations dynamically instead of hardcoding a specific SHA/version, so they no longer break when setup-node pins are refreshed. No UI changes were made (test-only change), so there is no UI screenshot. 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:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
engine.driverfrontmatter support for agentic engine workflowsengine.driveris a safe script filename (basename only, no path traversal/separators/shell-like values) and ends with a Node.js extension (.js,.cjs, or.mjs)engine.driverpattern enforcement to align with runtime validationcopilot_driver.cjsduring Copilot executionValidation
go test -v -run 'TestExtractEngineConfig|TestValidateEngineDriverScript|TestCopilotEngineDriverScript|TestDetectRuntimeRequirements_CustomDriverAddsNode24|TestDetectRuntimeRequirements_CustomDriverDoesNotAddNodeForNonCopilotEngine' ./pkg/workflow/✅go test -v -run 'TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_EngineDriverPattern' ./pkg/parser/✅make agent-finishpkg/workflow:TestApplyActionPinToStepTestGetActionPinByRepoTestApplyActionPinToTypedStepTestMapToStepWithActionPinningparallel_validation✅ Code Review + ✅ CodeQL (no security alerts)