From d01d468c7d0df8167d5ec6d8659ca9bdaa5090e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 00:45:11 +0000 Subject: [PATCH 1/4] Initial plan From b29538aa07f99146bd2126e79c2546760965c462 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 01:06:06 +0000 Subject: [PATCH 2/4] fix: handle float64 max-turns/max-continuations when engine config is loaded via shared import When engine config (including max-turns) is sourced from a shared import, it goes through a JSON roundtrip: YAML int -> json.Marshal -> json.Unmarshal -> float64. ExtractEngineConfig only handled int, uint64, and string types for max-turns and max-continuations, silently dropping the value when it arrived as float64 from the JSON deserialization. Add float64 type handling for max-turns and max-continuations in ExtractEngineConfig, with a unit test using float64(100) to simulate the JSON roundtrip scenario, and an integration test that verifies --max-turns appears in the compiled output when set via shared import. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a2e1bda8-a668-47d0-bf80-b5b2f373927d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/engine.go | 6 +++ pkg/workflow/engine_config_test.go | 13 ++++++ pkg/workflow/max_turns_test.go | 75 ++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) diff --git a/pkg/workflow/engine.go b/pkg/workflow/engine.go index e6a2db40dc9..9a3fa7ee8eb 100644 --- a/pkg/workflow/engine.go +++ b/pkg/workflow/engine.go @@ -190,6 +190,9 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng config.MaxTurns = strconv.Itoa(maxTurnsInt) } else if maxTurnsUint64, ok := maxTurns.(uint64); ok { config.MaxTurns = strconv.FormatUint(maxTurnsUint64, 10) + } else if maxTurnsFloat64, ok := maxTurns.(float64); ok { + // float64 occurs when engine config is loaded via JSON roundtrip (shared imports) + config.MaxTurns = strconv.FormatInt(int64(maxTurnsFloat64), 10) } else if maxTurnsStr, ok := maxTurns.(string); ok { config.MaxTurns = maxTurnsStr } @@ -201,6 +204,9 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng config.MaxContinuations = maxContInt } else if maxContUint64, ok := maxCont.(uint64); ok { config.MaxContinuations = int(maxContUint64) + } else if maxContFloat64, ok := maxCont.(float64); ok { + // float64 occurs when engine config is loaded via JSON roundtrip (shared imports) + config.MaxContinuations = int(maxContFloat64) } else if maxContStr, ok := maxCont.(string); ok { if parsed, err := strconv.Atoi(maxContStr); err == nil { config.MaxContinuations = parsed diff --git a/pkg/workflow/engine_config_test.go b/pkg/workflow/engine_config_test.go index a588fe9c6cc..df257648344 100644 --- a/pkg/workflow/engine_config_test.go +++ b/pkg/workflow/engine_config_test.go @@ -141,6 +141,19 @@ func TestExtractEngineConfig(t *testing.T) { expectedEngineSetting: "claude", expectedConfig: &EngineConfig{ID: "claude", Version: "beta", Model: "claude-3-5-sonnet-20241022", MaxTurns: "10"}, }, + { + // float64 is what json.Unmarshal produces for numbers when deserializing engine + // config JSON from shared imports (JSON roundtrip: YAML int -> JSON -> Go float64) + name: "object format - with max-turns as float64 (JSON roundtrip from shared import)", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "claude", + "max-turns": float64(100), + }, + }, + expectedEngineSetting: "claude", + expectedConfig: &EngineConfig{ID: "claude", MaxTurns: "100"}, + }, { name: "object format - with env vars", frontmatter: map[string]any{ diff --git a/pkg/workflow/max_turns_test.go b/pkg/workflow/max_turns_test.go index 981c2571ba9..3a6ebae5109 100644 --- a/pkg/workflow/max_turns_test.go +++ b/pkg/workflow/max_turns_test.go @@ -254,3 +254,78 @@ engine: }) } } + +func TestMaxTurnsFromSharedImport(t *testing.T) { + // This test verifies that engine.max-turns is correctly propagated when + // the engine config is sourced from a shared import rather than defined inline. + // The bug was that max-turns was silently dropped because it was serialized as + // JSON (int -> float64) but only int/uint64/string types were handled. + + // Create a temporary directory for the test + tmpDir := testutil.TempDir(t, "max-turns-import-test") + + // Create the shared import file with engine config including max-turns + sharedContent := `--- +engine: + id: claude + max-turns: 100 +permissions: + contents: read + issues: read + pull-requests: read +--- +` + sharedDir := filepath.Join(tmpDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatal(err) + } + sharedFile := filepath.Join(sharedDir, "common.md") + if err := os.WriteFile(sharedFile, []byte(sharedContent), 0644); err != nil { + t.Fatal(err) + } + + // Create the main workflow that imports the shared config + mainContent := `--- +on: + workflow_dispatch: +imports: + - shared/common.md +tools: + github: + allowed: [issue_read] +--- + +# Test Max Turns From Shared Import + +This workflow imports max-turns from a shared import. +` + mainFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(mainFile, []byte(mainContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler() + if err := compiler.CompileWorkflow(mainFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Read the generated lock file + lockFile := stringutil.MarkdownToLockFile(mainFile) + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + lockContentStr := string(lockContent) + + // Verify --max-turns 100 is present in the compiled output + if !strings.Contains(lockContentStr, "--max-turns 100") { + t.Errorf("Expected --max-turns 100 in compiled output when max-turns is set in shared import.\nLock file content:\n%s", lockContentStr) + } + + // Verify GH_AW_MAX_TURNS env var is set + if !strings.Contains(lockContentStr, "GH_AW_MAX_TURNS: 100") { + t.Errorf("Expected GH_AW_MAX_TURNS: 100 in compiled output.\nLock file content:\n%s", lockContentStr) + } +} From d36e3e08c7e34318c1a3d2f2c8d6235d021f1aec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 01:21:51 +0000 Subject: [PATCH 3/4] refactor: use typeutil.ParseIntValue for max-turns and max-continuations extraction Agent-Logs-Url: https://github.com/github/gh-aw/sessions/11f9df38-a6ab-4951-8aea-644807d527c5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/engine.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/pkg/workflow/engine.go b/pkg/workflow/engine.go index 9a3fa7ee8eb..5e067b5507f 100644 --- a/pkg/workflow/engine.go +++ b/pkg/workflow/engine.go @@ -8,6 +8,7 @@ import ( "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/stringutil" "github.com/github/gh-aw/pkg/types" + "github.com/github/gh-aw/pkg/typeutil" ) var engineLog = logger.New("workflow:engine") @@ -186,13 +187,8 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng // Extract optional 'max-turns' field if maxTurns, hasMaxTurns := engineObj["max-turns"]; hasMaxTurns { - if maxTurnsInt, ok := maxTurns.(int); ok { - config.MaxTurns = strconv.Itoa(maxTurnsInt) - } else if maxTurnsUint64, ok := maxTurns.(uint64); ok { - config.MaxTurns = strconv.FormatUint(maxTurnsUint64, 10) - } else if maxTurnsFloat64, ok := maxTurns.(float64); ok { - // float64 occurs when engine config is loaded via JSON roundtrip (shared imports) - config.MaxTurns = strconv.FormatInt(int64(maxTurnsFloat64), 10) + if val, ok := typeutil.ParseIntValue(maxTurns); ok { + config.MaxTurns = strconv.Itoa(val) } else if maxTurnsStr, ok := maxTurns.(string); ok { config.MaxTurns = maxTurnsStr } @@ -200,13 +196,8 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng // Extract optional 'max-continuations' field if maxCont, hasMaxCont := engineObj["max-continuations"]; hasMaxCont { - if maxContInt, ok := maxCont.(int); ok { - config.MaxContinuations = maxContInt - } else if maxContUint64, ok := maxCont.(uint64); ok { - config.MaxContinuations = int(maxContUint64) - } else if maxContFloat64, ok := maxCont.(float64); ok { - // float64 occurs when engine config is loaded via JSON roundtrip (shared imports) - config.MaxContinuations = int(maxContFloat64) + if val, ok := typeutil.ParseIntValue(maxCont); ok { + config.MaxContinuations = val } else if maxContStr, ok := maxCont.(string); ok { if parsed, err := strconv.Atoi(maxContStr); err == nil { config.MaxContinuations = parsed From c58f0784e8bf0e3dd7518a890b5c439934d710c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 01:39:51 +0000 Subject: [PATCH 4/4] fix: add permissions to main workflow in TestMaxTurnsFromSharedImport Agent-Logs-Url: https://github.com/github/gh-aw/sessions/53b73f01-653f-4d8c-a004-992916eb8bd3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/max_turns_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/workflow/max_turns_test.go b/pkg/workflow/max_turns_test.go index 3a6ebae5109..0139fdc75b9 100644 --- a/pkg/workflow/max_turns_test.go +++ b/pkg/workflow/max_turns_test.go @@ -288,6 +288,10 @@ permissions: mainContent := `--- on: workflow_dispatch: +permissions: + contents: read + issues: read + pull-requests: read imports: - shared/common.md tools: