Standardize heredoc delimiters with GH_AW_ prefix#14942
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧪 Smoke Project is now testing project operations... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Changeset Generator completed successfully! |
Agent Container Tool Check ✅
Result: 12/12 tools available ✅ All required development tools are present and functional in the agent container environment.
|
|
✅ Smoke Project completed successfully. All project operations validated. |
There was a problem hiding this comment.
Pull request overview
This PR standardizes heredoc delimiters emitted by the workflow compiler so compiled *.lock.yml files consistently use a GH_AW_<NAME>_EOF-style delimiter, reducing inconsistency across generated shell snippets.
Changes:
- Added
GenerateHeredocDelimiter()helper and tests to centralize delimiter naming. - Updated multiple workflow compilation/generation paths (prompt writing, MCP setup/config rendering, bundled file writing, SRT wrapper generation) to use the helper.
- Recompiled a large set of
*.lock.ymlworkflow outputs to reflect the standardized delimiters (and incidentally updated at least one pinned action SHA).
Reviewed changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/strings.go | Introduces GenerateHeredocDelimiter() helper for consistent heredoc delimiter generation. |
| pkg/workflow/strings_test.go | Adds unit tests covering delimiter generation and basic format assertions. |
| pkg/workflow/sh.go | Uses generated delimiter for prompt heredocs written into YAML. |
| pkg/workflow/prompt_step.go | Uses generated delimiter in prompt step heredoc emission. |
| pkg/workflow/prompt_step_test.go | Updates expected compiled output strings for new prompt delimiter. |
| pkg/workflow/prompt_step_helper_test.go | Updates expected compiled output strings for new prompt delimiter. |
| pkg/workflow/mcp_setup_generator.go | Standardizes delimiters for safe-outputs/safe-inputs file generation and tool scripts. |
| pkg/workflow/mcp_renderer.go | Standardizes the MCP config heredoc delimiter used for gateway startup piping. |
| pkg/workflow/heredoc_interpolation_test.go | Updates assertions to the new prompt delimiter naming. |
| pkg/workflow/engine_helpers_shared_test.go | Updates MCP config rendering expectations for the new delimiter. |
| pkg/workflow/custom_engine_test.go | Updates MCP config rendering expectations for the new delimiter. |
| pkg/workflow/copilot_srt.go | Uses generated delimiters for SRT wrapper/config heredocs. |
| pkg/workflow/codex_mcp.go | Uses generated delimiter for Codex TOML config heredoc end marker. |
| pkg/workflow/codex_engine_test.go | Updates expectations for Codex MCP config heredoc delimiters. |
| pkg/workflow/bundler_file_mode.go | Uses generated delimiter for file hash based heredocs when writing bundled scripts. |
| pkg/workflow/data/action_pins.json | Adds an action pin entry for docker/build-push-action@v6. |
| .github/aw/actions-lock.json | Mirrors the docker/build-push-action@v6 pin in the GitHub config lock file. |
| .github/workflows/*.lock.yml (147 files) | Recompiled workflow lockfiles to reflect standardized heredoc delimiters (and updated some pinned SHAs). |
Comments suppressed due to low confidence (1)
.github/workflows/release.lock.yml:1299
- This workflow lock file changes the pinned commit for docker/build-push-action@v6. Because action pin changes are security- and behavior-sensitive, it would be good to confirm this update is expected as part of the recompile (and ideally mention it in the PR description alongside the delimiter changes).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // GenerateHeredocDelimiter creates a standardized heredoc delimiter with the GH_AW prefix. | ||
| // All heredoc delimiters in compiled lock.yml files should use this format for consistency. | ||
| // | ||
| // The function generates delimiters in the format: GH_AW_<NAME>_EOF | ||
| // | ||
| // Parameters: | ||
| // - name: A descriptive identifier for the heredoc content (e.g., "PROMPT", "MCP_CONFIG", "TOOLS_JSON") | ||
| // The name should use SCREAMING_SNAKE_CASE without the _EOF suffix. | ||
| // | ||
| // Returns a delimiter string in the format "GH_AW_<NAME>_EOF" | ||
| // | ||
| // Example: | ||
| // | ||
| // GenerateHeredocDelimiter("PROMPT") // returns "GH_AW_PROMPT_EOF" | ||
| // GenerateHeredocDelimiter("MCP_CONFIG") // returns "GH_AW_MCP_CONFIG_EOF" | ||
| // GenerateHeredocDelimiter("TOOLS_JSON") // returns "GH_AW_TOOLS_JSON_EOF" | ||
| // GenerateHeredocDelimiter("SRT_CONFIG") // returns "GH_AW_SRT_CONFIG_EOF" | ||
| // GenerateHeredocDelimiter("FILE_123ABC") // returns "GH_AW_FILE_123ABC_EOF" | ||
| // | ||
| // Usage in heredoc generation: | ||
| // | ||
| // delimiter := GenerateHeredocDelimiter("PROMPT") | ||
| // yaml.WriteString(fmt.Sprintf("cat << '%s' >> \"$GH_AW_PROMPT\"\n", delimiter)) | ||
| // yaml.WriteString("content here\n") | ||
| // yaml.WriteString(delimiter + "\n") | ||
| func GenerateHeredocDelimiter(name string) string { | ||
| if name == "" { | ||
| return "GH_AW_EOF" | ||
| } | ||
| return "GH_AW_" + strings.ToUpper(name) + "_EOF" | ||
| } |
There was a problem hiding this comment.
GenerateHeredocDelimiter()’s docstring says the input should be SCREAMING_SNAKE_CASE and the examples imply delimiters only contain A–Z/0–9/underscores, but the implementation only uppercases and will preserve dashes or other characters. This already shows up in recompiled lock files (e.g. SAFE_INPUTS_* tool names include '-'). Consider normalizing the name (e.g., replace '-' with '_' and strip any trailing '_EOF') so the function enforces the documented format and avoids surprising delimiters.
| for _, toolName := range safeInputToolNames { | ||
| toolConfig := workflowData.SafeInputs.Tools[toolName] | ||
| if toolConfig.Script != "" { | ||
| // JavaScript tool | ||
| toolScript := generateSafeInputJavaScriptToolScript(toolConfig) | ||
| fmt.Fprintf(yaml, " cat > /opt/gh-aw/safe-inputs/%s.cjs << 'EOFJS_%s'\n", toolName, toolName) | ||
| jsDelimiter := GenerateHeredocDelimiter(fmt.Sprintf("SAFE_INPUTS_JS_%s", strings.ToUpper(toolName))) | ||
| fmt.Fprintf(yaml, " cat > /opt/gh-aw/safe-inputs/%s.cjs << '%s'\n", toolName, jsDelimiter) | ||
| for _, line := range FormatJavaScriptForYAML(toolScript) { | ||
| yaml.WriteString(line) | ||
| } | ||
| fmt.Fprintf(yaml, " EOFJS_%s\n", toolName) | ||
| fmt.Fprintf(yaml, " %s\n", jsDelimiter) | ||
| } else if toolConfig.Run != "" { | ||
| // Shell script tool | ||
| toolScript := generateSafeInputShellToolScript(toolConfig) | ||
| fmt.Fprintf(yaml, " cat > /opt/gh-aw/safe-inputs/%s.sh << 'EOFSH_%s'\n", toolName, toolName) | ||
| shDelimiter := GenerateHeredocDelimiter(fmt.Sprintf("SAFE_INPUTS_SH_%s", strings.ToUpper(toolName))) | ||
| fmt.Fprintf(yaml, " cat > /opt/gh-aw/safe-inputs/%s.sh << '%s'\n", toolName, shDelimiter) |
There was a problem hiding this comment.
These heredoc delimiters are derived from safe-inputs toolName, and tool names are allowed to contain '-' per schema; as written this produces delimiters with '-' which conflicts with the stated GH_AW_EOF convention and makes delimiter format less predictable. Either normalize toolName before passing it (e.g., convert '-'→'' using existing normalization helpers) or rely on GenerateHeredocDelimiter() to do that normalization centrally.
| func TestGenerateHeredocDelimiter(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| input string | ||
| expected string | ||
| }{ | ||
| { | ||
| name: "simple name", | ||
| input: "PROMPT", | ||
| expected: "GH_AW_PROMPT_EOF", | ||
| }, | ||
| { | ||
| name: "multi-word name with underscores", | ||
| input: "MCP_CONFIG", | ||
| expected: "GH_AW_MCP_CONFIG_EOF", | ||
| }, | ||
| { | ||
| name: "tools json", | ||
| input: "TOOLS_JSON", | ||
| expected: "GH_AW_TOOLS_JSON_EOF", | ||
| }, | ||
| { | ||
| name: "SRT config", | ||
| input: "SRT_CONFIG", | ||
| expected: "GH_AW_SRT_CONFIG_EOF", | ||
| }, | ||
| { | ||
| name: "SRT wrapper", | ||
| input: "SRT_WRAPPER", | ||
| expected: "GH_AW_SRT_WRAPPER_EOF", | ||
| }, | ||
| { | ||
| name: "file with hash", | ||
| input: "FILE_123ABC", | ||
| expected: "GH_AW_FILE_123ABC_EOF", | ||
| }, | ||
| { | ||
| name: "safe inputs", | ||
| input: "SAFE_INPUTS", | ||
| expected: "GH_AW_SAFE_INPUTS_EOF", | ||
| }, | ||
| { | ||
| name: "JS file suffix", | ||
| input: "EOFJS_TOOL_NAME", | ||
| expected: "GH_AW_EOFJS_TOOL_NAME_EOF", | ||
| }, | ||
| { | ||
| name: "shell file suffix", | ||
| input: "EOFSH_TOOL_NAME", | ||
| expected: "GH_AW_EOFSH_TOOL_NAME_EOF", | ||
| }, | ||
| { | ||
| name: "python file suffix", | ||
| input: "EOFPY_TOOL_NAME", | ||
| expected: "GH_AW_EOFPY_TOOL_NAME_EOF", | ||
| }, | ||
| { | ||
| name: "go file suffix", | ||
| input: "EOFGO_TOOL_NAME", | ||
| expected: "GH_AW_EOFGO_TOOL_NAME_EOF", | ||
| }, | ||
| { | ||
| name: "lowercase input gets uppercased", | ||
| input: "prompt", | ||
| expected: "GH_AW_PROMPT_EOF", | ||
| }, | ||
| { | ||
| name: "mixed case input", | ||
| input: "Mcp_Config", | ||
| expected: "GH_AW_MCP_CONFIG_EOF", | ||
| }, | ||
| { | ||
| name: "empty string returns default", | ||
| input: "", | ||
| expected: "GH_AW_EOF", | ||
| }, | ||
| { | ||
| name: "single character", | ||
| input: "A", | ||
| expected: "GH_AW_A_EOF", | ||
| }, | ||
| { | ||
| name: "numbers only", | ||
| input: "123", | ||
| expected: "GH_AW_123_EOF", | ||
| }, | ||
| { | ||
| name: "alphanumeric with underscores", | ||
| input: "CONFIG_V2_TEST", | ||
| expected: "GH_AW_CONFIG_V2_TEST_EOF", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| result := GenerateHeredocDelimiter(tt.input) | ||
| assert.Equal(t, tt.expected, result, "GenerateHeredocDelimiter failed for test case: %s", tt.name) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestGenerateHeredocDelimiter_Usage(t *testing.T) { | ||
| // Test that the delimiter can be used in actual heredoc patterns | ||
| delimiter := GenerateHeredocDelimiter("TEST") | ||
| assert.Equal(t, "GH_AW_TEST_EOF", delimiter) | ||
|
|
||
| // Verify format is correct for heredoc usage | ||
| assert.True(t, strings.HasPrefix(delimiter, "GH_AW_"), "Delimiter should start with GH_AW_") | ||
| assert.True(t, strings.HasSuffix(delimiter, "_EOF"), "Delimiter should end with _EOF") | ||
|
|
||
| // Test that it contains only uppercase alphanumeric and underscores (valid for heredoc) | ||
| validPattern := regexp.MustCompile(`^[A-Z0-9_]+$`) | ||
| assert.True(t, validPattern.MatchString(delimiter), "Delimiter should contain only uppercase alphanumeric and underscores") | ||
| } |
There was a problem hiding this comment.
TestGenerateHeredocDelimiter currently doesn’t cover inputs containing dashes (which occur in safe-inputs tool names) even though other tests assert delimiters should match ^[A-Z0-9_]+$. Add a test case with a dashed name (e.g. "github-discussion-query") to lock in the intended normalization behavior once GenerateHeredocDelimiter is updated.
| "docker/build-push-action@v6": { | ||
| "repo": "docker/build-push-action", | ||
| "version": "v6", | ||
| "sha": "ee4ca427a2f43b6a16632044ca514c076267da23" | ||
| }, |
There was a problem hiding this comment.
This PR’s stated scope is heredoc delimiter standardization, but it also adds a new action pin for docker/build-push-action@v6. If this is intentional (e.g., needed because source workflows reference @v6), please call it out explicitly in the PR description/changelog since changing action pins affects supply-chain posture and may alter which exact version is used.
|
Smoke test (Codex)
|
Smoke Test ResultsStatus: PRs:
Tests: ✅ GitHub MCP | ✅ Safe Inputs | ✅ Serena | ❌ Playwright | ✅ File Ops | ✅ Bash | ✅ Discussion | ✅ Build | ✅ Dispatch cc @pelikhan
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Heredoc delimiters in compiled lock.yml files were inconsistent (
EOF,PROMPT_EOF,MCPCONFIG_EOF,EOFJS_<name>, etc.). CreatedGenerateHeredocDelimiter()helper to enforceGH_AW_<NAME>_EOFformat across all 100+ occurrences.Implementation
Changes
pkg/workflow/strings.go-GenerateHeredocDelimiter()with comprehensive test coverageprompt_step.go,sh.go,mcp_renderer.go,mcp_setup_generator.go,bundler_file_mode.go,copilot_srt.go,codex_mcp.goDelimiter Mapping
PROMPT_EOFGH_AW_PROMPT_EOFMCPCONFIG_EOFGH_AW_MCP_CONFIG_EOFSRT_CONFIG_EOFGH_AW_SRT_CONFIG_EOFEOF_TOOLS_JSONGH_AW_SAFE_INPUTS_TOOLS_EOFEOFJS_<name>GH_AW_SAFE_INPUTS_JS_<NAME>_EOFEOF_<hash>GH_AW_FILE_<HASH>_EOFOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Changeset