Skip to content

Commit 2a4e122

Browse files
CopilotpelikhanCopilotgithub-actions[bot]
authored
Replace inlined Go builtin engine definitions with embedded shared agentic workflow files (#20500)
* Initial plan * feat: replace inlined Go builtin engine definitions with embedded YAML resources - Add YAML struct tags (kebab-case) to EngineDefinition and related types - Create data/engines/{claude,codex,copilot,gemini}.yml with engine metadata - Add engine_definition_loader.go with embed.FS and YAML parsing logic - Update NewEngineCatalog() to load definitions from embedded YAML files Adding a new built-in engine now requires only a new .yml file in data/engines/ — no Go code changes needed. Closes #20416 (Phase 1) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: add missing auth bindings, engine key wrapper, and schema validation - Add COPILOT_GITHUB_TOKEN auth binding to copilot.yml - Add GEMINI_API_KEY auth binding to gemini.yml - Move all engine data under top-level "engine:" key in YAML files - Add schemas/engine_definition_schema.json to validate engine YAML files - Validate YAML against schema before parsing in loader - Update TestBuiltInEngineAuthUnchanged for copilot and gemini Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: allow inline engine definition (engine.runtime) to be imported from shared workflow Two bugs prevented custom inline engine keys from being importable: 1. validateSingleEngineSpecification only recognized engine.id in imported engines, not the engine.runtime.id form used by inline definitions. 2. setupEngineAndImports did not validate or register inline engine definitions extracted from imported files, causing catalog.Resolve to fail. Added regression test TestImportedInlineEngineDefinition. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * feat: convert engine yml files to shared agentic workflow md files with builtin import injection Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: xml-comment engine md body to avoid polluting prompt; fix CLI engine override ID propagation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Update pkg/workflow/engine_definition.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: address automated review comments (virtual FS safety, CRLF frontmatter, imports shadowing, error message) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: validate builtin engine files against shared workflow schema Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: remove engine_definition_schema.json, validation via main workflow schema Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: resolve lint errors and update test fixtures for action version upgrades Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Add changeset [skip-ci] --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 9122b7f commit 2a4e122

20 files changed

Lines changed: 789 additions & 84 deletions

.changeset/patch-embed-engine-markdown.md

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/parser/import_field_extractor.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,15 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import
7878

7979
// Track import path for runtime-import macro generation (only if no inputs).
8080
// Imports with inputs must be inlined for compile-time substitution.
81+
// Builtin paths (@builtin:…) are pure configuration — they carry no user-visible
82+
// prompt content and must not generate runtime-import macros.
8183
importRelPath := computeImportRelPath(item.fullPath, item.importPath)
8284

83-
if len(item.inputs) == 0 {
84-
// No inputs - use runtime-import macro
85+
if len(item.inputs) == 0 && !strings.HasPrefix(importRelPath, BuiltinPathPrefix) {
86+
// No inputs and not a builtin - use runtime-import macro
8587
acc.importPaths = append(acc.importPaths, importRelPath)
8688
log.Printf("Added import path for runtime-import: %s", importRelPath)
87-
} else {
89+
} else if len(item.inputs) > 0 {
8890
// Has inputs - must inline for compile-time substitution
8991
log.Printf("Import %s has inputs - will be inlined for compile-time substitution", importRelPath)
9092
markdownContent, err := processIncludedFileWithVisited(item.fullPath, item.sectionName, false, visited)

pkg/parser/remote_fetch.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ func isRepositoryImport(importPath string) bool {
116116
func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, error) {
117117
remoteLog.Printf("Resolving include path: file_path=%s, base_dir=%s", filePath, baseDir)
118118

119+
// Handle builtin paths - these are embedded files that bypass filesystem resolution.
120+
// No security check is needed since the content is compiled into the binary.
121+
if strings.HasPrefix(filePath, BuiltinPathPrefix) {
122+
if !BuiltinVirtualFileExists(filePath) {
123+
return "", fmt.Errorf("builtin file not found: %s", filePath)
124+
}
125+
remoteLog.Printf("Resolved builtin path: %s", filePath)
126+
return filePath, nil
127+
}
128+
119129
// Check if this is a workflowspec (contains owner/repo/path format)
120130
// Format: owner/repo/path@ref or owner/repo/path@ref#section
121131
if isWorkflowSpec(filePath) {

pkg/parser/remote_fetch_wasm.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ func isRepositoryImport(importPath string) bool {
5656
}
5757

5858
func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, error) {
59+
// Handle builtin paths - these are embedded files that bypass filesystem resolution.
60+
if strings.HasPrefix(filePath, BuiltinPathPrefix) {
61+
if !BuiltinVirtualFileExists(filePath) {
62+
return "", fmt.Errorf("builtin file not found: %s", filePath)
63+
}
64+
return filePath, nil
65+
}
66+
5967
if isWorkflowSpec(filePath) {
6068
return "", fmt.Errorf("remote imports not available in Wasm: %s", filePath)
6169
}

pkg/parser/schemas/main_workflow_schema.json

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8076,6 +8076,138 @@
80768076
},
80778077
"required": ["runtime"],
80788078
"additionalProperties": false
8079+
},
8080+
{
8081+
"type": "object",
8082+
"description": "Engine definition: full declarative metadata for a named engine entry (used in builtin engine shared workflow files such as @builtin:engines/*.md)",
8083+
"properties": {
8084+
"id": {
8085+
"type": "string",
8086+
"description": "Unique engine identifier (e.g. 'copilot', 'claude', 'codex', 'gemini')"
8087+
},
8088+
"display-name": {
8089+
"type": "string",
8090+
"description": "Human-readable display name for the engine"
8091+
},
8092+
"description": {
8093+
"type": "string",
8094+
"description": "Human-readable description of the engine"
8095+
},
8096+
"runtime-id": {
8097+
"type": "string",
8098+
"description": "Runtime adapter identifier. Maps to the CodingAgentEngine registered in the engine registry. Defaults to id when omitted."
8099+
},
8100+
"provider": {
8101+
"type": "object",
8102+
"description": "Provider metadata for the engine",
8103+
"properties": {
8104+
"name": {
8105+
"type": "string",
8106+
"description": "Provider name (e.g. 'anthropic', 'github', 'google', 'openai')"
8107+
},
8108+
"auth": {
8109+
"type": "object",
8110+
"description": "Default authentication configuration for the provider",
8111+
"properties": {
8112+
"secret": {
8113+
"type": "string",
8114+
"description": "Name of the GitHub Actions secret that contains the API key"
8115+
},
8116+
"strategy": {
8117+
"type": "string",
8118+
"enum": ["api-key", "oauth-client-credentials", "bearer"],
8119+
"description": "Authentication strategy"
8120+
},
8121+
"token-url": {
8122+
"type": "string",
8123+
"description": "OAuth 2.0 token endpoint URL"
8124+
},
8125+
"client-id": {
8126+
"type": "string",
8127+
"description": "GitHub Actions secret name for the OAuth client ID"
8128+
},
8129+
"client-secret": {
8130+
"type": "string",
8131+
"description": "GitHub Actions secret name for the OAuth client secret"
8132+
},
8133+
"token-field": {
8134+
"type": "string",
8135+
"description": "JSON field name in the token response containing the access token"
8136+
},
8137+
"header-name": {
8138+
"type": "string",
8139+
"description": "HTTP header name to inject the API key or token into"
8140+
}
8141+
},
8142+
"additionalProperties": false
8143+
},
8144+
"request": {
8145+
"type": "object",
8146+
"description": "Request shaping configuration",
8147+
"properties": {
8148+
"path-template": {
8149+
"type": "string",
8150+
"description": "URL path template with variable placeholders"
8151+
},
8152+
"query": {
8153+
"type": "object",
8154+
"description": "Static query parameters",
8155+
"additionalProperties": { "type": "string" }
8156+
},
8157+
"body-inject": {
8158+
"type": "object",
8159+
"description": "Key/value pairs injected into the JSON request body",
8160+
"additionalProperties": { "type": "string" }
8161+
}
8162+
},
8163+
"additionalProperties": false
8164+
}
8165+
},
8166+
"additionalProperties": false
8167+
},
8168+
"models": {
8169+
"type": "object",
8170+
"description": "Model selection configuration for the engine",
8171+
"properties": {
8172+
"default": {
8173+
"type": "string",
8174+
"description": "Default model identifier"
8175+
},
8176+
"supported": {
8177+
"type": "array",
8178+
"items": { "type": "string" },
8179+
"description": "List of supported model identifiers"
8180+
}
8181+
},
8182+
"additionalProperties": false
8183+
},
8184+
"auth": {
8185+
"type": "array",
8186+
"description": "Authentication bindings — maps logical roles (e.g. 'api-key') to GitHub Actions secret names",
8187+
"items": {
8188+
"type": "object",
8189+
"properties": {
8190+
"role": {
8191+
"type": "string",
8192+
"description": "Logical authentication role (e.g. 'api-key', 'token')"
8193+
},
8194+
"secret": {
8195+
"type": "string",
8196+
"description": "Name of the GitHub Actions secret that provides credentials for this role"
8197+
}
8198+
},
8199+
"required": ["role", "secret"],
8200+
"additionalProperties": false
8201+
}
8202+
},
8203+
"options": {
8204+
"type": "object",
8205+
"description": "Additional engine-specific options",
8206+
"additionalProperties": true
8207+
}
8208+
},
8209+
"required": ["id", "display-name"],
8210+
"additionalProperties": false
80798211
}
80808212
]
80818213
},

pkg/parser/virtual_fs.go

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,68 @@
11
package parser
22

3-
import "os"
3+
import (
4+
"fmt"
5+
"os"
6+
"strings"
7+
"sync"
8+
)
9+
10+
// builtinVirtualFiles holds embedded built-in files registered at startup.
11+
// Keys use the "@builtin:" path prefix (e.g. "@builtin:engines/copilot.md").
12+
// The map is populated once and then read-only; concurrent reads are safe.
13+
var (
14+
builtinVirtualFiles map[string][]byte
15+
builtinVirtualFilesMu sync.RWMutex
16+
)
17+
18+
// RegisterBuiltinVirtualFile registers an embedded file under a canonical builtin path.
19+
// Paths must start with BuiltinPathPrefix ("@builtin:"); it panics if they do not.
20+
// If the same path is registered twice with identical content the call is a no-op.
21+
// Registering the same path with different content panics to surface configuration errors early.
22+
// This function is safe for concurrent use.
23+
func RegisterBuiltinVirtualFile(path string, content []byte) {
24+
if !strings.HasPrefix(path, BuiltinPathPrefix) {
25+
panic(fmt.Sprintf("RegisterBuiltinVirtualFile: path %q does not start with %q", path, BuiltinPathPrefix))
26+
}
27+
builtinVirtualFilesMu.Lock()
28+
defer builtinVirtualFilesMu.Unlock()
29+
if builtinVirtualFiles == nil {
30+
builtinVirtualFiles = make(map[string][]byte)
31+
}
32+
if existing, ok := builtinVirtualFiles[path]; ok {
33+
if string(existing) != string(content) {
34+
panic(fmt.Sprintf("RegisterBuiltinVirtualFile: path %q already registered with different content", path))
35+
}
36+
return // idempotent: same content, no-op
37+
}
38+
builtinVirtualFiles[path] = content
39+
}
40+
41+
// BuiltinVirtualFileExists returns true if the given path is registered as a builtin virtual file.
42+
func BuiltinVirtualFileExists(path string) bool {
43+
builtinVirtualFilesMu.RLock()
44+
defer builtinVirtualFilesMu.RUnlock()
45+
_, ok := builtinVirtualFiles[path]
46+
return ok
47+
}
48+
49+
// BuiltinPathPrefix is the path prefix used for embedded builtin files.
50+
// Paths with this prefix bypass filesystem resolution and security checks.
51+
const BuiltinPathPrefix = "@builtin:"
452

553
// readFileFunc is the function used to read file contents throughout the parser.
654
// In wasm builds, this is overridden to read from a virtual filesystem
755
// populated by the browser via SetVirtualFiles.
8-
var readFileFunc = os.ReadFile
56+
// In native builds, builtin virtual files are checked first, then os.ReadFile.
57+
var readFileFunc = func(path string) ([]byte, error) {
58+
builtinVirtualFilesMu.RLock()
59+
content, ok := builtinVirtualFiles[path]
60+
builtinVirtualFilesMu.RUnlock()
61+
if ok {
62+
return content, nil
63+
}
64+
return os.ReadFile(path)
65+
}
966

1067
// ReadFile reads a file using the parser's file reading function, which
1168
// checks the virtual filesystem first in wasm builds. Use this instead of

pkg/parser/virtual_fs_wasm.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ func VirtualFileExists(path string) bool {
3333
func init() {
3434
// Override readFileFunc in wasm builds to check virtual files first.
3535
readFileFunc = func(path string) ([]byte, error) {
36+
// Check builtin virtual files first (embedded engine .md files etc.)
37+
builtinVirtualFilesMu.RLock()
38+
builtinContent, builtinOK := builtinVirtualFiles[path]
39+
builtinVirtualFilesMu.RUnlock()
40+
if builtinOK {
41+
return builtinContent, nil
42+
}
3643
if virtualFiles != nil {
3744
if content, ok := virtualFiles[path]; ok {
3845
return content, nil

pkg/workflow/compiler_orchestrator_engine.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,27 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean
9898
c.IncrementWarningCount()
9999
}
100100
engineSetting = c.engineOverride
101+
// Update engineConfig.ID so that downstream code (e.g. generateCreateAwInfo) uses
102+
// the override engine ID, not the one parsed from the frontmatter.
103+
if engineConfig != nil {
104+
engineConfig.ID = c.engineOverride
105+
}
106+
}
107+
108+
// When the engine is specified in short/string form ("engine: copilot") and no CLI
109+
// override is active, inject the corresponding builtin shared-workflow .md as an
110+
// import. This makes "engine: copilot" syntactic sugar for importing the builtin
111+
// copilot.md, which carries the full engine definition. The engine field is removed
112+
// from the frontmatter so the definition comes entirely from the import.
113+
if c.engineOverride == "" && isStringFormEngine(result.Frontmatter) && engineSetting != "" {
114+
builtinPath := builtinEnginePath(engineSetting)
115+
if parser.BuiltinVirtualFileExists(builtinPath) {
116+
orchestratorEngineLog.Printf("Injecting builtin engine import: %s", builtinPath)
117+
addImportToFrontmatter(result.Frontmatter, builtinPath)
118+
delete(result.Frontmatter, "engine")
119+
engineSetting = ""
120+
engineConfig = nil
121+
}
101122
}
102123

103124
// Process imports from frontmatter first (before @include directives)
@@ -197,6 +218,19 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean
197218
return nil, fmt.Errorf("failed to extract engine config from included file: %w", err)
198219
}
199220
engineConfig = extractedConfig
221+
222+
// If the imported engine is an inline definition (engine.runtime sub-object),
223+
// validate and register it in the catalog. This mirrors the handling for inline
224+
// definitions declared directly in the main workflow (above).
225+
if engineConfig != nil && engineConfig.IsInlineDefinition {
226+
if err := c.validateEngineInlineDefinition(engineConfig); err != nil {
227+
return nil, err
228+
}
229+
if err := c.validateEngineAuthDefinition(engineConfig); err != nil {
230+
return nil, err
231+
}
232+
c.registerInlineEngineDefinition(engineConfig)
233+
}
200234
}
201235

202236
// Apply the default AI engine setting if not specified
@@ -299,3 +333,43 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean
299333
configSteps: configSteps,
300334
}, nil
301335
}
336+
337+
// isStringFormEngine reports whether the "engine" field in the given frontmatter is a
338+
// plain string (e.g. "engine: copilot"), as opposed to an object with an "id" or
339+
// "runtime" sub-key.
340+
func isStringFormEngine(frontmatter map[string]any) bool {
341+
engine, exists := frontmatter["engine"]
342+
if !exists {
343+
return false
344+
}
345+
_, isString := engine.(string)
346+
return isString
347+
}
348+
349+
// addImportToFrontmatter appends importPath to the "imports" slice in frontmatter.
350+
// It handles the case where "imports" may be absent, a []any, a []string, or a
351+
// single string (which is converted to a two-element slice preserving the original value).
352+
// Any other unexpected type is left unchanged and importPath is not injected.
353+
func addImportToFrontmatter(frontmatter map[string]any, importPath string) {
354+
existing, hasImports := frontmatter["imports"]
355+
if !hasImports {
356+
frontmatter["imports"] = []any{importPath}
357+
return
358+
}
359+
switch v := existing.(type) {
360+
case []any:
361+
frontmatter["imports"] = append(v, importPath)
362+
case []string:
363+
newSlice := make([]any, len(v)+1)
364+
for i, s := range v {
365+
newSlice[i] = s
366+
}
367+
newSlice[len(v)] = importPath
368+
frontmatter["imports"] = newSlice
369+
case string:
370+
// Single string import — preserve it and append the new one.
371+
frontmatter["imports"] = []any{v, importPath}
372+
// For any other unexpected type, leave the field untouched so the
373+
// downstream parser can still report its own error for the invalid value.
374+
}
375+
}

pkg/workflow/compiler_yaml.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,22 @@ func (c *Compiler) generateWorkflowHeader(yaml *strings.Builder, data *WorkflowD
101101
}
102102

103103
// Add manifest of imported/included files if any exist
104-
if len(data.ImportedFiles) > 0 || len(data.IncludedFiles) > 0 {
104+
// Build a user-visible imports list by filtering out internal builtin engine paths
105+
// (e.g. "@builtin:engines/copilot.md") which are implementation details.
106+
var visibleImports []string
107+
for _, file := range data.ImportedFiles {
108+
if !strings.HasPrefix(file, parser.BuiltinPathPrefix) {
109+
visibleImports = append(visibleImports, file)
110+
}
111+
}
112+
113+
if len(visibleImports) > 0 || len(data.IncludedFiles) > 0 {
105114
yaml.WriteString("#\n")
106115
yaml.WriteString("# Resolved workflow manifest:\n")
107116

108-
if len(data.ImportedFiles) > 0 {
117+
if len(visibleImports) > 0 {
109118
yaml.WriteString("# Imports:\n")
110-
for _, file := range data.ImportedFiles {
119+
for _, file := range visibleImports {
111120
cleanFile := stringutil.StripANSI(file)
112121
// Normalize to Unix paths (forward slashes) for cross-platform compatibility
113122
cleanFile = filepath.ToSlash(cleanFile)

0 commit comments

Comments
 (0)