Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/smoke-workflow-call-with-inputs.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/smoke-workflow-call.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 14 additions & 5 deletions pkg/workflow/compiler_workflow_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/parser"
"github.com/goccy/go-yaml"
)

Expand Down Expand Up @@ -162,15 +163,16 @@ func (c *Compiler) injectWorkflowCallOutputs(onSection string, safeOutputs *Safe
workflowCallMap["outputs"] = mergedOutputs
onMap["workflow_call"] = workflowCallMap

// Re-marshal to YAML
// Re-marshal to YAML. IndentSequence(true) matches extractTopLevelYAMLSection so
// sequence items (e.g. schedule cron lists) stay indented under their parent key.
newOnData := map[string]any{"on": onMap}
newYAML, err := yaml.Marshal(newOnData)
newYAML, err := yaml.MarshalWithOptions(newOnData, yaml.IndentSequence(true))
Comment thread
pelikhan marked this conversation as resolved.
if err != nil {
workflowCallLog.Printf("Warning: failed to marshal on section with workflow_call outputs: %v", err)
Comment thread
pelikhan marked this conversation as resolved.
return onSection
}

return strings.TrimSuffix(string(newYAML), "\n")
return finalizeWorkflowCallOnSection(newYAML)
}

// buildWorkflowCallOutputsMap constructs the outputs map for on.workflow_call.outputs
Expand Down Expand Up @@ -343,11 +345,18 @@ func injectWorkflowCallSecretsSection(onSection string, secrets []string) string

// Re-marshal to YAML.
newOnData := map[string]any{"on": onMap}
newYAML, err := yaml.Marshal(newOnData)
newYAML, err := yaml.MarshalWithOptions(newOnData, yaml.IndentSequence(true))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in b3b0593, extended in 06b4ed8.

TestInjectWorkflowCallSecretsSection now has a schedule + workflow_call case (pkg/workflow/compiler_workflow_call_test.go:441):

{
    name: "schedule sequence stays indented and cron stays quoted",
    onSection: `"on":
  schedule:
    - cron: "0 0 */2 * *"
  workflow_call: {}`,
    secrets:     []string{"MY_TOKEN"},
    wantContain: []string{"MY_TOKEN", "  schedule:\n    - cron: \"0 0 */2 * *\""},
    wantAbsent:  []string{"  schedule:\n  - cron:", "- cron: 0 0"},
},

It asserts the four-space nesting positively and pins the flattened two-space form as absent, plus the unquoted cron form after 06b4ed8 also restored parser.QuoteCronExpressions on this path.

Verified it's a real guard rather than a tautology: reverting the secrets call site back to plain yaml.Marshal makes this case fail with

"...\n  schedule:\n  - cron: 0 0 */2 * *\n  workflow_call:\n    secrets:..."
  does not contain "  schedule:\n    - cron: \"0 0 */2 * *\""

The equivalent case exists for the outputs path at line 142.

if err != nil {
workflowCallLog.Printf("Warning: failed to marshal on section with workflow_call secrets: %v", err)
return onSection
}

return strings.TrimSuffix(string(newYAML), "\n")
return finalizeWorkflowCallOnSection(newYAML)
}

// finalizeWorkflowCallOnSection post-processes a re-marshaled on: section so it matches
// the formatting produced by extractTopLevelYAMLSection. The YAML library drops quotes
// from cron expressions such as "0 0 */2 * *", so they are re-quoted here.
func finalizeWorkflowCallOnSection(marshaled []byte) string {
return strings.TrimSuffix(parser.QuoteCronExpressions(string(marshaled)), "\n")
}
27 changes: 27 additions & 0 deletions pkg/workflow/compiler_workflow_call_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,23 @@ func TestInjectWorkflowCallOutputs(t *testing.T) {
},
expectUnchanged: true,
},
{
name: "schedule sequence stays indented and cron stays quoted",
onSection: `"on":
schedule:
- cron: "0 0 */2 * *"
workflow_call:`,
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
},
expectContains: []string{
" schedule:\n - cron: \"0 0 */2 * *\"",
},
expectAbsent: []string{
" schedule:\n - cron:",
"- cron: 0 0",
},
},
{
name: "user-defined outputs are preserved when merged",
onSection: `"on":
Expand Down Expand Up @@ -421,6 +438,16 @@ func TestInjectWorkflowCallSecretsSection(t *testing.T) {
secrets: []string{"AUTO_SECRET"},
wantContain: []string{"USER_SECRET", "AUTO_SECRET"},
},
{
name: "schedule sequence stays indented and cron stays quoted",
onSection: `"on":
schedule:
- cron: "0 0 */2 * *"
workflow_call: {}`,
secrets: []string{"MY_TOKEN"},
wantContain: []string{"MY_TOKEN", " schedule:\n - cron: \"0 0 */2 * *\""},
wantAbsent: []string{" schedule:\n - cron:", "- cron: 0 0"},
},
{
name: "handles string shorthand on: workflow_call",
onSection: `"on": workflow_call`,
Expand Down
Loading