From 17d09fe920027df5f0f5c7a66488196ffe918cea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 29 Aug 2026 21:45:48 +0000 Subject: [PATCH 1/5] Initial plan From 1912044a669949331a49e9da109488a6ded68ac9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:03:40 +0000 Subject: [PATCH 2/5] Add explicit job and step timeouts to Visual Regression Checker workflow Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../visual-regression-checker.lock.yml | 11 ++- .../workflows/visual-regression-checker.md | 7 ++ pkg/workflow/step_types.go | 73 ++++++++++++------- pkg/workflow/step_types_test.go | 27 +++++++ 4 files changed, 89 insertions(+), 29 deletions(-) diff --git a/.github/workflows/visual-regression-checker.lock.yml b/.github/workflows/visual-regression-checker.lock.yml index 7b9d06ba6fd..5ab5246e11f 100644 --- a/.github/workflows/visual-regression-checker.lock.yml +++ b/.github/workflows/visual-regression-checker.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"f13a0a8d581f4502ec81e7b02a06459ea1b7dac13e89a56d9806ef0dc10e03a9","body_hash":"c06ffaa0dd77f6147f6a4c27aacf37d8b48a63face007301645c1bfde07efc33","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.80"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"1a865cc2ea6cef19739fd073c2ab1c42fd2c3ebe4399cf7e71f387fc3aa2a3da","body_hash":"c06ffaa0dd77f6147f6a4c27aacf37d8b48a63face007301645c1bfde07efc33","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.80"}} # gh-aw-manifest: {"version":1,"secrets":["COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"3d3c42e5aac5ba805825da76410c181273ba90b1","version":"v7.0.1"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.28.10","digest":"sha256:c01e6d16d11ea4f2a46cc023a9f402224a3b3861b026818eec0dc586d7e6918e","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.28.10@sha256:c01e6d16d11ea4f2a46cc023a9f402224a3b3861b026818eec0dc586d7e6918e"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.10","digest":"sha256:c3a18aebb8251339117ea998296315de17bada366f8d03919b3348ea71112e64","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.10@sha256:c3a18aebb8251339117ea998296315de17bada366f8d03919b3348ea71112e64"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.28.10","digest":"sha256:c06076f7aca95df713e0748c44d80c0a3c2538fad67bfdd04296d45158e083e6","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.28.10@sha256:c06076f7aca95df713e0748c44d80c0a3c2538fad67bfdd04296d45158e083e6"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.13","digest":"sha256:ec4008521c610e1113ed557ecec0ff64a2c2111e4cfa817bab54d9b7da24c7cc","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.13@sha256:ec4008521c610e1113ed557ecec0ff64a2c2111e4cfa817bab54d9b7da24c7cc"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:bac2192f6374d6262116399b34fc5e143d576f82719e90a18261cae7480f4d4e","pinned_image":"ghcr.io/github/gh-aw-node@sha256:bac2192f6374d6262116399b34fc5e143d576f82719e90a18261cae7480f4d4e"},{"image":"ghcr.io/github/github-mcp-server:v1.11.0","digest":"sha256:fbec75de11c255213fa08d80fb166abe73d851fff631c51c0079872967720699","pinned_image":"ghcr.io/github/github-mcp-server:v1.11.0@sha256:fbec75de11c255213fa08d80fb166abe73d851fff631c51c0079872967720699"}],"has_pull_request":true,"mcp_servers":[{"name":"github","tools":["get_commit","get_file_contents","get_latest_release","get_me","get_pull_request","get_pull_request_comments","get_pull_request_diff","get_pull_request_files","get_pull_request_review_comments","get_pull_request_reviews","get_pull_request_status","get_release_by_tag","get_tag","issue_read","list_branches","list_commits","list_issue_types","list_issues","list_pull_requests","list_releases","list_starred_repositories","list_tags","pull_request_read","search_code","search_issues","search_pull_requests","search_repositories"]},{"name":"safeoutputs","tools":["add_comment","missing_data","missing_tool","noop"]}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # @@ -407,7 +407,7 @@ jobs: permissions: contents: read pull-requests: read - timeout-minutes: 60 + timeout-minutes: 15 env: DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} GH_AW_ASSETS_ALLOWED_EXTS: "" @@ -508,15 +508,18 @@ jobs: package-manager-cache: false - name: Install dependencies run: npm ci + timeout-minutes: 5 working-directory: ./docs - name: Build documentation run: npm run build + timeout-minutes: 5 working-directory: ./docs - name: Start docs server run: "nohup npm run dev -- --host 0.0.0.0 --port 4321 > /tmp/gh-aw/agent/preview.log 2>&1 &\nPID=$!\necho \"$PID\" > /tmp/gh-aw/agent/server.pid\necho \"Server PID: $PID\"\n" working-directory: ./docs - name: Wait for server readiness - run: "MAX_WAIT=90\nWAITED=0\n# runner-guard:ignore RGS-012 -- loopback-only port probe for the docs server started in this job; no external network or secret data is involved.\nuntil (echo > /dev/tcp/127.0.0.1/4321) > /dev/null 2>&1; do\n if [ -f /tmp/gh-aw/agent/server.pid ] && ! kill -0 \"$(cat /tmp/gh-aw/agent/server.pid)\" 2>/dev/null; then\n echo \"Docs server process exited before opening port 4321\" >&2\n cat /tmp/gh-aw/agent/preview.log >&2\n exit 1\n fi\n WAITED=$((WAITED + 3))\n if [ $WAITED -ge $MAX_WAIT ]; then\n echo \"Docs server port 4321 did not open in ${MAX_WAIT}s\" >&2\n cat /tmp/gh-aw/agent/preview.log >&2\n exit 1\n fi\n echo \"Waiting for docs port... ($WAITED/${MAX_WAIT}s)\"\n sleep 3\ndone\nWAITED=0\n# runner-guard:ignore RGS-012 -- localhost readiness request to the docs server started above; response is discarded and no secrets are sent.\nuntil curl -sf http://localhost:4321/gh-aw/ > /dev/null 2>&1; do\n WAITED=$((WAITED + 3))\n if [ $WAITED -ge $MAX_WAIT ]; then\n echo \"Dev server did not become ready in ${MAX_WAIT}s\" >&2\n cat /tmp/gh-aw/agent/preview.log >&2\n exit 1\n fi\n echo \"Waiting for dev server response... ($WAITED/${MAX_WAIT}s)\"\n sleep 3\ndone\necho \"Dev server is ready\"" + run: "MAX_WAIT=90\nWAITED=0\n# runner-guard:ignore RGS-012 -- loopback-only port probe for the docs server started in this job; no external network or secret data is involved.\nuntil (echo > /dev/tcp/127.0.0.1/4321) > /dev/null 2>&1; do\n if [ -f /tmp/gh-aw/agent/server.pid ] && ! kill -0 \"$(cat /tmp/gh-aw/agent/server.pid)\" 2>/dev/null; then\n echo \"Docs server process exited before opening port 4321\" >&2\n cat /tmp/gh-aw/agent/preview.log >&2\n exit 1\n fi\n WAITED=$((WAITED + 3))\n if [ $WAITED -ge $MAX_WAIT ]; then\n echo \"Docs server port 4321 did not open in ${MAX_WAIT}s\" >&2\n cat /tmp/gh-aw/agent/preview.log >&2\n exit 1\n fi\n echo \"Waiting for docs port... ($WAITED/${MAX_WAIT}s)\"\n sleep 3\ndone\nWAITED=0\n# runner-guard:ignore RGS-012 -- localhost readiness request to the docs server started above; response is discarded and no secrets are sent.\nuntil curl -sf http://localhost:4321/gh-aw/ > /dev/null 2>&1; do\n WAITED=$((WAITED + 3))\n if [ $WAITED -ge $MAX_WAIT ]; then\n echo \"Dev server did not become ready in ${MAX_WAIT}s\" >&2\n cat /tmp/gh-aw/agent/preview.log >&2\n exit 1\n fi\n echo \"Waiting for dev server response... ($WAITED/${MAX_WAIT}s)\"\n sleep 3\ndone\necho \"Dev server is ready\"\n" + timeout-minutes: 2 - name: Configure Git credentials env: @@ -1813,7 +1816,7 @@ jobs: permissions: issues: write pull-requests: write - timeout-minutes: 45 + timeout-minutes: 10 env: GH_AW_AGENT_AIC: ${{ needs.agent.outputs.aic }} GH_AW_AIC: ${{ needs.agent.outputs.aic }} diff --git a/.github/workflows/visual-regression-checker.md b/.github/workflows/visual-regression-checker.md index dc2d42fa77e..92b4d30426c 100644 --- a/.github/workflows/visual-regression-checker.md +++ b/.github/workflows/visual-regression-checker.md @@ -35,7 +35,11 @@ network: - playwright - local - node +jobs: + agent: + timeout-minutes: 15 safe-outputs: + timeout-minutes: 10 add-comment: max: 1 timeout-minutes: 15 @@ -54,10 +58,12 @@ steps: - name: Install dependencies working-directory: ./docs + timeout-minutes: 5 run: npm ci - name: Build documentation working-directory: ./docs + timeout-minutes: 5 run: npm run build - name: Start docs server @@ -70,6 +76,7 @@ steps: - name: Wait for server readiness # runner-guard:ignore RGS-012 -- loopback-only port/readiness checks for the docs server started in this job; no external network or secrets are involved. + timeout-minutes: 2 run: | MAX_WAIT=90 WAITED=0 diff --git a/pkg/workflow/step_types.go b/pkg/workflow/step_types.go index 3223fed6c6b..3fe61956dcb 100644 --- a/pkg/workflow/step_types.go +++ b/pkg/workflow/step_types.go @@ -117,34 +117,13 @@ func MapToStep(stepMap map[string]any) (*WorkflowStep, error) { step.With = with } if env, ok := stepMap["env"].(map[string]any); ok { - // Convert map[string]any to map[string]string - step.Env = make(map[string]string) - for k, v := range env { - if strVal, ok := v.(string); ok { - step.Env[k] = strVal - } else if v != nil { - // Arrays and maps are serialized as JSON so that shell consumers - // (e.g. jq --argjson) receive valid JSON. This handles both the - // []any / map[string]any case returned by encoding/json and the - // typed-slice case (e.g. []string) returned by goccy/go-yaml. - step.Env[k] = marshalEnvValue(v) - } - } + step.Env = parseStepEnv(env) } if continueOnError, ok := stepMap["continue-on-error"]; ok { - switch value := continueOnError.(type) { - case bool: - templatableValue := TemplatableBool(strconv.FormatBool(value)) - step.ContinueOnError = &templatableValue - case string: - if value == "true" || value == "false" || isExpression(value) { - templatableValue := TemplatableBool(value) - step.ContinueOnError = &templatableValue - } - } + step.ContinueOnError = parseStepContinueOnError(continueOnError) } - if timeoutMinutes, ok := stepMap["timeout-minutes"].(int); ok { - step.TimeoutMinutes = timeoutMinutes + if timeoutMinutesVal, ok := stepMap["timeout-minutes"]; ok { + step.TimeoutMinutes = parseStepTimeoutMinutes(timeoutMinutesVal) } stepType := "unknown" @@ -157,6 +136,50 @@ func MapToStep(stepMap map[string]any) (*WorkflowStep, error) { return step, nil } +func parseStepEnv(env map[string]any) map[string]string { + result := make(map[string]string) + for k, v := range env { + if strVal, ok := v.(string); ok { + result[k] = strVal + } else if v != nil { + result[k] = marshalEnvValue(v) + } + } + return result +} + +func parseStepContinueOnError(val any) *TemplatableBool { + switch value := val.(type) { + case bool: + templatableValue := TemplatableBool(strconv.FormatBool(value)) + return &templatableValue + case string: + if value == "true" || value == "false" || isExpression(value) { + templatableValue := TemplatableBool(value) + return &templatableValue + } + } + return nil +} + +func parseStepTimeoutMinutes(val any) int { + switch v := val.(type) { + case int: + return v + case int64: + return int(v) + case uint64: + return int(v) + case float64: + return int(v) + case string: + if n, err := strconv.Atoi(v); err == nil { + return n + } + } + return 0 +} + // Clone creates a deep copy of the WorkflowStep func (s *WorkflowStep) Clone() *WorkflowStep { clone := &WorkflowStep{ diff --git a/pkg/workflow/step_types_test.go b/pkg/workflow/step_types_test.go index 53b71130f6e..cf2b6e61844 100644 --- a/pkg/workflow/step_types_test.go +++ b/pkg/workflow/step_types_test.go @@ -663,6 +663,33 @@ func TestSliceToSteps_RoundTrip(t *testing.T) { } } +func TestMapToStep_TimeoutMinutesNumericTypes(t *testing.T) { + tests := []struct { + name string + val any + want int + }{ + {"int", 5, 5}, + {"int64", int64(10), 10}, + {"uint64", uint64(15), 15}, + {"float64", float64(20), 20}, + {"string", "25", 25}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stepMap := map[string]any{ + "name": "Test", + "run": "echo test", + "timeout-minutes": tt.val, + } + step, err := MapToStep(stepMap) + require.NoError(t, err) + assert.Equal(t, tt.want, step.TimeoutMinutes) + }) + } +} + func TestMapToStep_InvalidTypes(t *testing.T) { tests := []struct { name string From 97752088a7e6d256b2dd5f8e2674ddd3e7114215 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:12:14 +0000 Subject: [PATCH 3/5] Add explicit job and step timeouts to Visual Regression Checker workflow and add bounds check for step timeout-minutes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/step_types.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/step_types.go b/pkg/workflow/step_types.go index 3fe61956dcb..42199b4e237 100644 --- a/pkg/workflow/step_types.go +++ b/pkg/workflow/step_types.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "maps" + "math" "strconv" "github.com/github/gh-aw/pkg/importinpututil" @@ -167,11 +168,17 @@ func parseStepTimeoutMinutes(val any) int { case int: return v case int64: - return int(v) + if v > 0 && v <= int64(math.MaxInt) { + return int(v) + } case uint64: - return int(v) + if v <= uint64(math.MaxInt) { + return int(v) + } case float64: - return int(v) + if v > 0 && v <= float64(math.MaxInt) { + return int(v) + } case string: if n, err := strconv.Atoi(v); err == nil { return n From 289029ef8b9b160118802fe29809c91bed3d9781 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:58:41 +0000 Subject: [PATCH 4/5] Reject non-positive and fractional step timeout-minutes values Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/step_types.go | 19 +++++++++++++++---- pkg/workflow/step_types_test.go | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/step_types.go b/pkg/workflow/step_types.go index 42199b4e237..2704d0934e4 100644 --- a/pkg/workflow/step_types.go +++ b/pkg/workflow/step_types.go @@ -163,24 +163,35 @@ func parseStepContinueOnError(val any) *TemplatableBool { return nil } +// parseStepTimeoutMinutes converts a YAML `timeout-minutes` value into a positive +// number of minutes. Values that are not positive integers within the platform int +// range are ignored (returning 0, which omits the field from the rendered step). func parseStepTimeoutMinutes(val any) int { switch v := val.(type) { case int: - return v + if v > 0 { + return v + } case int64: if v > 0 && v <= int64(math.MaxInt) { return int(v) } case uint64: - if v <= uint64(math.MaxInt) { + if v > 0 && v <= uint64(math.MaxInt) { return int(v) } case float64: - if v > 0 && v <= float64(math.MaxInt) { + // float64 loses integer precision near MaxInt on 64-bit platforms, so treat + // values at or above the rounded float boundary as out of range. Only + // integral values are accepted so fractional timeouts are not truncated. + if math.IsNaN(v) || math.IsInf(v, 0) || v != math.Trunc(v) { + return 0 + } + if v >= 1 && v < float64(math.MaxInt) { return int(v) } case string: - if n, err := strconv.Atoi(v); err == nil { + if n, err := strconv.Atoi(v); err == nil && n > 0 { return n } } diff --git a/pkg/workflow/step_types_test.go b/pkg/workflow/step_types_test.go index cf2b6e61844..ca500613c24 100644 --- a/pkg/workflow/step_types_test.go +++ b/pkg/workflow/step_types_test.go @@ -3,6 +3,7 @@ package workflow import ( + "math" "testing" "github.com/stretchr/testify/assert" @@ -674,6 +675,19 @@ func TestMapToStep_TimeoutMinutesNumericTypes(t *testing.T) { {"uint64", uint64(15), 15}, {"float64", float64(20), 20}, {"string", "25", 25}, + {"negative int", -5, 0}, + {"zero int", 0, 0}, + {"negative int64", int64(-10), 0}, + {"zero uint64", uint64(0), 0}, + {"negative float64", float64(-20), 0}, + {"fractional float64", 1.9, 0}, + {"out of range float64", math.MaxFloat64, 0}, + {"NaN float64", math.NaN(), 0}, + {"negative string", "-25", 0}, + {"zero string", "0", 0}, + {"non-numeric string", "abc", 0}, + {"bool", true, 0}, + {"nil", nil, 0}, } for _, tt := range tests { From d7c32e9205a4a784a53b2301847a305d548b1c89 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 29 Aug 2026 23:24:23 +0000 Subject: [PATCH 5/5] Assert provider-agnostic explicit model in PR code quality reviewer contract test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/pr_code_quality_reviewer_workflow_contract_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/cli/pr_code_quality_reviewer_workflow_contract_test.go b/pkg/cli/pr_code_quality_reviewer_workflow_contract_test.go index 0eb2445304e..b9cc1922a69 100644 --- a/pkg/cli/pr_code_quality_reviewer_workflow_contract_test.go +++ b/pkg/cli/pr_code_quality_reviewer_workflow_contract_test.go @@ -5,6 +5,7 @@ package cli import ( "os" "path/filepath" + "regexp" "testing" "github.com/github/gh-aw/pkg/gitutil" @@ -12,6 +13,10 @@ import ( "github.com/stretchr/testify/require" ) +// explicitMainAgentModelPattern matches a top-level, provider-qualified model +// declaration (for example "model: openai/gpt-5.4") in the workflow frontmatter. +var explicitMainAgentModelPattern = regexp.MustCompile(`(?m)^model: \S+/\S+$`) + func TestPRCodeQualityReviewerWorkflowSubAgentModelContract(t *testing.T) { t.Parallel() repoRoot, err := gitutil.FindGitRoot() @@ -24,7 +29,7 @@ func TestPRCodeQualityReviewerWorkflowSubAgentModelContract(t *testing.T) { require.NoError(t, err, "Should read pr-code-quality-reviewer workflow") text := string(content) - assert.Contains(t, text, "model: copilot/gpt-5.4", "Main agent should use an explicit Copilot model") + assert.Regexp(t, explicitMainAgentModelPattern, text, "Main agent should use an explicit provider-qualified model") assert.Contains(t, text, "## agent: `grumpy-coder`", "Workflow should define the grumpy-coder sub-agent") assert.Contains(t, text, "model: small", "Sub-agent should use the portable small alias") assert.NotContains(t, text, "model: inherited", "Sub-agent should not inherit an unsupported tier-specific model")