Inline agentic commands router JavaScript #52986
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package setupjs | ||
|
|
||
| import "embed" | ||
|
|
||
| // AgenticCommandsScripts contains the JavaScript modules used by the generated | ||
| // agentic commands router. | ||
| // | ||
| //go:embed route_slash_command.cjs add_reaction.cjs add_workflow_run_comment.cjs | ||
| //go:embed aw_context.cjs error_codes.cjs error_helpers.cjs experiment_helpers.cjs | ||
| //go:embed generate_footer.cjs github_api_helpers.cjs glob_pattern_helpers.cjs | ||
| //go:embed invocation_context_helpers.cjs markdown_code_region_balancer.cjs | ||
| //go:embed messages_core.cjs messages_run_status.cjs repo_helpers.cjs | ||
| //go:embed sanitize_content.cjs sanitize_content_core.cjs slash_command_matcher.cjs | ||
| //go:embed templatable.cjs threat_detection_warning.cjs workflow_metadata_helpers.cjs | ||
| var AgenticCommandsScripts embed.FS | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # ADR-52986: Inline Agentic Commands Router JavaScript | ||
|
|
||
| **Date**: 2026-08-15 | ||
| **Status**: Draft | ||
| **Deciders**: pelikhan, copilot-swe-agent | ||
|
|
||
| --- | ||
|
|
||
| ### Context | ||
|
|
||
| The agentic commands router is a set of CommonJS JavaScript modules that implement slash-command routing logic for generated GitHub Actions workflows. Previously, these modules were distributed as external files loaded at runtime: the generated workflow included a `checkout` step plus a `gh-aw-actions/setup` step that copied the modules to `${{ runner.temp }}/gh-aw/actions/`, and the router entry point was then `require`d from that path. This approach added two extra workflow steps to every generated agentic command workflow, introduced latency at job startup, and created a version coupling risk between the Go compiler binary that generates workflows and the JavaScript modules fetched at runtime. | ||
|
|
||
| ### Decision | ||
|
|
||
| We will embed all agentic commands JavaScript modules directly into the Go compiler binary using Go's `//go:embed` directive and generate a self-contained CommonJS bundle that is inlined into the `actions/github-script` step at workflow-generation time. The bundle uses a minimal module loader (`__ghAwRequire`) that resolves relative `require()` calls across the embedded module map without any filesystem access. The generated workflow no longer contains a `checkout` or `setup-action` step for the router. | ||
|
|
||
| ### Alternatives Considered | ||
|
|
||
| #### Alternative 1: Keep External File Distribution (Checkout + Setup Action) | ||
|
|
||
| The existing model: a `gh-aw-actions/setup` action copies JavaScript modules to a well-known temp path, and the generated workflow `require`s the entry point from there. This is the simplest individual-module update path (update JS without a Go rebuild), but it adds two mandatory workflow steps, increases job startup time, and couples the runtime module version to a separate action release cycle rather than the compiler binary. | ||
|
|
||
| #### Alternative 2: Pre-Built External Bundle (esbuild/rollup artifact) | ||
|
|
||
| An external JavaScript bundler (esbuild, rollup, or webpack) could produce a single minified file committed to the repo, which the setup action copies rather than many individual modules. This eliminates step count parity concerns but still requires a runtime download step, adds a build-tool dependency to the release pipeline, and leaves the committed artifact out of sync with the Go compiler unless an automated check enforces it. | ||
|
|
||
| ### Consequences | ||
|
|
||
| #### Positive | ||
| - Removes the `checkout` and `setup-action` steps from every generated agentic command workflow, reducing job startup time and step count. | ||
| - The JavaScript modules and the Go compiler binary share the same release artifact; there is no version skew between what generates the workflow and what runs inside it. | ||
| - Dependency resolution failures are caught at compile time (Go build error) rather than at workflow runtime. | ||
|
|
||
| #### Negative | ||
| - The Go compiler binary grows proportionally with the JavaScript module set; each new `.cjs` module added to the embed list increases binary size. | ||
| - JavaScript-only changes (bug fixes in a `.cjs` module) now require a full Go rebuild and release cycle; there is no way to patch the router without shipping a new binary. | ||
| - The generated workflow YAML file size increases substantially (~3,500 lines added to `agentic_commands.yml`) because the full module bundle is inlined; this makes the generated file harder to read directly. | ||
|
|
||
| #### Neutral | ||
| - The custom `__ghAwRequire` loader replicates a subset of Node.js `require` semantics (relative path resolution, module caching); it handles only the `.cjs`/`.js` extension set the agentic commands modules actually use. | ||
| - Tests for the bundler (`TestBundleAgenticCommandsScript`, `TestGetAgenticCommandsScript`) validate the Go-side bundling logic and embed correctness, but do not execute the inlined JavaScript end-to-end. | ||
|
|
||
| --- | ||
|
|
||
| *ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| package workflow | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "io/fs" | ||
| "path" | ||
| "regexp" | ||
| "sort" | ||
| "strconv" | ||
| "strings" | ||
| "sync" | ||
|
|
||
| setupjs "github.com/github/gh-aw/actions/setup/js" | ||
| ) | ||
|
|
||
| var ( | ||
| agenticCommandsLocalRequirePattern = regexp.MustCompile(`require\(\s*["'](\.{1,2}/[^"']+)["']\s*\)`) | ||
| agenticCommandsScriptOnce sync.Once | ||
| agenticCommandsScript string | ||
| agenticCommandsScriptErr error | ||
| ) | ||
|
|
||
| func getAgenticCommandsScript() (string, error) { | ||
| agenticCommandsScriptOnce.Do(func() { | ||
|
Contributor
There was a problem hiding this comment. [/codebase-design] 💡 Suggested fixEither use var (
agenticCommandsScriptOnce sync.Once
agenticCommandsScript string
agenticCommandsScriptErr error
)The current design is fine if the only failure mode is a broken binary (missing @copilot please address this. |
||
| agenticCommandsScript, agenticCommandsScriptErr = bundleAgenticCommandsScript( | ||
| setupjs.AgenticCommandsScripts, | ||
| "route_slash_command.cjs", | ||
| ) | ||
| }) | ||
| return agenticCommandsScript, agenticCommandsScriptErr | ||
| } | ||
|
|
||
| func bundleAgenticCommandsScript(scripts fs.FS, entrypoint string) (string, error) { | ||
| modules := make(map[string]string) | ||
| if err := collectAgenticCommandsModules(scripts, entrypoint, modules); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| moduleNames := make([]string, 0, len(modules)) | ||
| for name := range modules { | ||
| moduleNames = append(moduleNames, name) | ||
| } | ||
| sort.Strings(moduleNames) | ||
|
|
||
| var script strings.Builder | ||
| script.WriteString("const __ghAwNativeRequire = require;\n") | ||
| script.WriteString("const __ghAwPath = __ghAwNativeRequire('node:path').posix;\n") | ||
| script.WriteString("const __ghAwModules = {\n") | ||
| for _, name := range moduleNames { | ||
| fmt.Fprintf(&script, " %s: (module, exports, require) => {\n", strconv.Quote(name)) | ||
| for line := range strings.SplitSeq(modules[name], "\n") { | ||
| script.WriteString(" ") | ||
| script.WriteString(strings.TrimRight(line, " \t")) | ||
| script.WriteByte('\n') | ||
| } | ||
| script.WriteString(" },\n") | ||
| } | ||
| script.WriteString(`}; | ||
| const __ghAwModuleCache = Object.create(null); | ||
| function __ghAwRequire(request, parentDirectory = "") { | ||
| if (!request.startsWith(".")) { | ||
| return __ghAwNativeRequire(request); | ||
| } | ||
| let moduleName = __ghAwPath.normalize(__ghAwPath.join(parentDirectory, request)); | ||
| if (!moduleName.endsWith(".cjs") && !moduleName.endsWith(".js")) { | ||
| moduleName += ".cjs"; | ||
| } | ||
| if (!Object.hasOwn(__ghAwModules, moduleName)) { | ||
| throw new Error(` + "`Agentic commands module not found: ${moduleName}`" + `); | ||
| } | ||
| if (Object.hasOwn(__ghAwModuleCache, moduleName)) { | ||
| return __ghAwModuleCache[moduleName].exports; | ||
| } | ||
| const module = { exports: {} }; | ||
| __ghAwModuleCache[moduleName] = module; | ||
| const localRequire = dependency => __ghAwRequire(dependency, __ghAwPath.dirname(moduleName)); | ||
| __ghAwModules[moduleName](module, module.exports, localRequire); | ||
| return module.exports; | ||
| } | ||
| `) | ||
| fmt.Fprintf( | ||
| &script, | ||
| "const { main: __ghAwMain } = __ghAwRequire(%s);\n", | ||
| strconv.Quote("./"+path.Clean(strings.TrimPrefix(entrypoint, "./"))), | ||
| ) | ||
| script.WriteString(` | ||
| await __ghAwMain(); | ||
| `) | ||
| return trimAgenticCommandsScript(removeJavaScriptComments(script.String())), nil | ||
|
Contributor
There was a problem hiding this comment. L89: delete: bundleAgenticCommandsScript strips comments via removeJavaScriptComments, then WriteJavaScriptToYAML (central_slash_command_workflow.go) strips comments again on the same string. Drop the inner call, comment removal happens once at the YAML-write step. |
||
| } | ||
|
|
||
| func trimAgenticCommandsScript(script string) string { | ||
|
Contributor
There was a problem hiding this comment. L92-99: yagni: trimAgenticCommandsScript re-walks every line to trim trailing whitespace, but WriteJavaScriptToYAML already skips/rewrites each line when inlining to YAML. Drop this pass. |
||
| var trimmed strings.Builder | ||
| for line := range strings.SplitSeq(script, "\n") { | ||
| trimmed.WriteString(strings.TrimRight(line, " \t")) | ||
| trimmed.WriteByte('\n') | ||
| } | ||
| return trimmed.String() | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. [/codebase-design] 💡 Suggested fixAdd an in-progress set alongside the func collectAgenticCommandsModules(scripts fs.FS, moduleName string, modules map[string]string) error {
moduleName = path.Clean(strings.TrimPrefix(moduleName, "./"))
// ... path validation ...
if _, exists := modules[moduleName]; exists {
return nil // already collected
}
// Mark as in-progress *before* recursing to break cycles
modules[moduleName] = "" // placeholder
content, err := fs.ReadFile(scripts, moduleName)
if err != nil { ... }
modules[moduleName] = string(content)
// ... recurse into dependencies ...
}The placeholder @copilot please address this. |
||
| func collectAgenticCommandsModules(scripts fs.FS, moduleName string, modules map[string]string) error { | ||
| moduleName = path.Clean(strings.TrimPrefix(moduleName, "./")) | ||
| if moduleName == "." || moduleName == ".." || strings.HasPrefix(moduleName, "../") { | ||
| return fmt.Errorf("invalid agentic commands module path: %q", moduleName) | ||
| } | ||
| if _, exists := modules[moduleName]; exists { | ||
| return nil | ||
| } | ||
|
|
||
| content, err := fs.ReadFile(scripts, moduleName) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read agentic commands module %q: %w", moduleName, err) | ||
| } | ||
| modules[moduleName] = string(content) | ||
|
|
||
| for _, match := range agenticCommandsLocalRequirePattern.FindAllStringSubmatch(string(content), -1) { | ||
| dependency := path.Clean(path.Join(path.Dir(moduleName), match[1])) | ||
| if path.Ext(dependency) == "" { | ||
| dependency += ".cjs" | ||
| } | ||
| if err := collectAgenticCommandsModules(scripts, dependency, modules); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| //go:build !integration | ||
|
|
||
| package workflow | ||
|
|
||
| import ( | ||
| "testing" | ||
| "testing/fstest" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestBundleAgenticCommandsScript(t *testing.T) { | ||
| scripts := fstest.MapFS{ | ||
| "main.cjs": {Data: []byte(`const { value } = require("./dependency.cjs"); module.exports = { main: async () => value };`)}, | ||
| "dependency.cjs": {Data: []byte(`module.exports = { value: "ok" };`)}, | ||
| } | ||
|
|
||
| script, err := bundleAgenticCommandsScript(scripts, "main.cjs") | ||
| require.NoError(t, err) | ||
| require.Contains(t, script, `"dependency.cjs": (module, exports, require) => {`) | ||
| require.Contains(t, script, `"main.cjs": (module, exports, require) => {`) | ||
| require.Contains(t, script, `const __ghAwModuleCache = Object.create(null);`) | ||
| require.Contains(t, script, `const { main: __ghAwMain } = __ghAwRequire("./main.cjs");`) | ||
| require.Contains(t, script, `await __ghAwMain();`) | ||
| } | ||
|
|
||
| func TestBundleAgenticCommandsScriptFailsForMissingDependency(t *testing.T) { | ||
| scripts := fstest.MapFS{ | ||
| "main.cjs": {Data: []byte(`require("./missing.cjs");`)}, | ||
| } | ||
|
|
||
| _, err := bundleAgenticCommandsScript(scripts, "main.cjs") | ||
| require.ErrorContains(t, err, `failed to read agentic commands module "missing.cjs"`) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. [/tdd] No test covers circular dependency detection. The current implementation will infinitely recurse if two bundled modules require each other — a test should document this expectation before the guard is (or isn't) added. 💡 Suggested testfunc TestBundleAgenticCommandsScriptFailsForCircularDependency(t *testing.T) {
scripts := fstest.MapFS{
"a.cjs": {Data: []byte(`require("./b.cjs");`)},
"b.cjs": {Data: []byte(`require("./a.cjs");`)},
}
_, err := bundleAgenticCommandsScript(scripts, "a.cjs")
// Either succeeds (if cycles are harmless by design) or returns a
// descriptive error — either way the behaviour is explicit.
_ = err
}Even if circular deps cannot appear in practice with the current JS modules, the test pins the contract and prevents future regressions. @copilot please address this. |
||
|
|
||
| func TestGetAgenticCommandsScript(t *testing.T) { | ||
| script, err := getAgenticCommandsScript() | ||
| require.NoError(t, err) | ||
|
Contributor
There was a problem hiding this comment. [/tdd] @copilot please address this. |
||
| require.Contains(t, script, `"route_slash_command.cjs": (module, exports, require) => {`) | ||
| require.Contains(t, script, `"add_workflow_run_comment.cjs": (module, exports, require) => {`) | ||
| require.Contains(t, script, `async function main()`) | ||
| require.Contains(t, script, `await __ghAwMain();`) | ||
| } | ||
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/codebase-design] The
(go/redacted):embeddirectives inagentic_commands_embed.golist every.cjsfile explicitly. If a new module is added toactions/setup/js/but forgotten here, the compiler silently produces a broken bundle at runtime (missing module error). An explicit(go/redacted):embed *.cjswildcard would auto-include all current and future modules.💡 Suggested fix
This is safe because the bundler's
collectAgenticCommandsModulesonly loads files reachable from the entrypoint — unused.cjsfiles in the FS are simply ignored. The wildcard makes the embed self-maintaining.The explicit listing does have one advantage: it documents intent. If the explicit list is intentional (to restrict which files are embedded), add a comment explaining the policy.
@copilot please address this.