Conversation
|
@copilot use TemplatableInt32 |
…ressions
Allow tools.timeout, tools.startup-timeout, and tools.playwright.version
to accept GitHub Actions expressions (e.g. ${{ inputs.tool-timeout }})
alongside their existing literal integer/string values, enabling
reusable workflow_call workflows to customize tool behavior at call time.
Changes:
- tools_types.go: Change Timeout/StartupTimeout from *int to *string
- tools_parser.go: parseTimeoutTool/parseStartupTimeoutTool accept expressions
- frontmatter_extraction_metadata.go: extractToolsTimeout/extractToolsStartupTimeout
return string and accept expression strings
- compiler_orchestrator_tools.go: toolsTimeout/toolsStartupTimeout fields are string
- compiler_types.go: WorkflowData.ToolsTimeout/ToolsStartupTimeout are string
- claude/codex/copilot engines: use templatableIntValue for ms calculations;
emit expressions directly as env var values
- mcp_renderer_github.go: use templatableIntValue for TOML config (falls back
to default for expressions since TOML doesn't support expressions)
- main_workflow_schema.json: allow integer or string for timeout/startup-timeout
- Tests updated to use string types and added expression test cases
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bf733584-88a5-48c0-bb70-2bf490a2ec0d
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Add isExpressionString() helper to templatables.go and use it in tools_parser.go and frontmatter_extraction_metadata.go, eliminating repeated inline expression validation checks. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bf733584-88a5-48c0-bb70-2bf490a2ec0d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…meout Replace *string with *TemplatableInt32 for the Timeout and StartupTimeout fields in ToolsConfig, using the established codebase pattern. Update parseTimeoutTool/parseStartupTimeoutTool to return *TemplatableInt32, and update ToMap() to emit the correct type via ToValue(). Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e2f00122-6390-4882-990e-ec1c3399bc83 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 0a50063. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Enables tools.timeout and tools.startup-timeout to be specified as either literal integers or GitHub Actions expressions (e.g. ${{ inputs.tool-timeout }}), allowing reusable workflow_call workflows to parameterize tool timeouts at call time.
Changes:
- Switched workflow timeout plumbing (
WorkflowData, tools processing results, engines) frominttostringto preserve expressions. - Updated parsing/extraction to accept
${{ ... }}strings while validating numeric literals (min 1) and rejecting non-expression strings. - Relaxed JSON schema for
tools.timeout/tools.startup-timeoutfrom integer-only tooneOf: [integer, string], and added helperisExpressionString().
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/tools_types.go | Uses TemplatableInt32 for tools timeout fields and round-trips via ToValue() |
| pkg/workflow/tools_timeout_test.go | Updates tests for string timeouts and adds expression coverage |
| pkg/workflow/tools_parser.go | Parses timeout fields into TemplatableInt32, accepting numeric types + expression strings |
| pkg/workflow/templatables.go | Adds isExpressionString() helper for strict ${{...}} detection |
| pkg/workflow/mcp_renderer_github.go | Falls back to defaults for TOML-rendered timeouts when values are expressions |
| pkg/workflow/frontmatter_extraction_metadata.go | Extracts tool timeouts as strings; validates expressions vs numeric literals |
| pkg/workflow/frontmatter_extraction_metadata_test.go | Updates startup-timeout extraction tests for string return + expressions |
| pkg/workflow/copilot_engine_execution.go | Emits timeout env vars as raw strings (literal or expression) |
| pkg/workflow/compiler_types.go | Changes timeout fields in WorkflowData from int to string |
| pkg/workflow/compiler_orchestrator_workflow_test.go | Updates orchestrator workflow tests for new timeout types |
| pkg/workflow/compiler_orchestrator_tools.go | Changes tools processing result timeout fields to string |
| pkg/workflow/compiler_orchestrator_tools_test.go | Updates assertions for string timeouts |
| pkg/workflow/codex_engine.go | Emits timeout env vars as raw strings (literal or expression) |
| pkg/workflow/claude_engine.go | Uses templatableIntValue() for ms conversion; emits timeout env vars as raw strings |
| pkg/parser/schemas/main_workflow_schema.json | Relaxes schema for timeout fields to accept expressions via oneOf |
Comments suppressed due to low confidence (1)
pkg/workflow/frontmatter_extraction_metadata.go:231
- This error message says tools.startup-timeout "must be an integer" even though expression strings are now accepted. Consider updating it to "must be an integer or a GitHub Actions expression" to match the updated behavior and improve clarity.
case float64:
timeout = int(v)
default:
return "", fmt.Errorf("tools.startup-timeout must be an integer, got %T", timeoutValue)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| "type": "string", | ||
| "description": "GitHub Actions expression (e.g. '${{ inputs.tool-timeout }}')" | ||
| } |
There was a problem hiding this comment.
The schema allows a string for tools.timeout but doesn’t constrain it to a GitHub Actions expression. Other templatable int fields (e.g. top-level timeout-minutes) add a pattern like ^\$\{\{.*\}\}$ to prevent accepting arbitrary strings that will later fail compilation. Consider adding the same pattern to the string branch here.
| }, | ||
| { | ||
| "type": "string", | ||
| "description": "GitHub Actions expression (e.g. '${{ inputs.startup-timeout }}')" |
There was a problem hiding this comment.
The schema allows a string for tools.startup-timeout but doesn’t constrain it to a GitHub Actions expression. For consistency with other templatable integer fields (e.g. timeout-minutes uses a ^\$\{\{.*\}\}$ pattern), add a pattern here so non-expression strings are rejected by schema validation rather than at compile time.
| "description": "GitHub Actions expression (e.g. '${{ inputs.startup-timeout }}')" | |
| "description": "GitHub Actions expression (e.g. '${{ inputs.startup-timeout }}')", | |
| "pattern": "^\\$\\{\\{.*\\}\\}$" |
| "minimum": 1, | ||
| "description": "Timeout in seconds for tool/MCP server operations. Applies to all tools and MCP servers if supported by the engine. Default varies by engine (Claude: 60s, Codex: 120s).", | ||
| "examples": [60, 120, 300] | ||
| "description": "Timeout in seconds for tool/MCP server operations. Applies to all tools and MCP servers if supported by the engine. Default varies by engine (Claude: 60s, Codex: 120s). Supports GitHub Actions expressions for reusable workflow_call workflows.", |
There was a problem hiding this comment.
The tools.timeout description says the Codex default is 120s, but the code uses constants.DefaultToolTimeout (60s) for tool timeouts (e.g. pkg/constants/constants.go defines DefaultToolTimeout = 60s, and Codex TOML rendering outputs tool_timeout_sec based on that). Please update the description to match actual behavior to avoid misleading users.
| "description": "Timeout in seconds for tool/MCP server operations. Applies to all tools and MCP servers if supported by the engine. Default varies by engine (Claude: 60s, Codex: 120s). Supports GitHub Actions expressions for reusable workflow_call workflows.", | |
| "description": "Timeout in seconds for tool/MCP server operations. Applies to all tools and MCP servers if supported by the engine. Default: 60 seconds (for both Claude and Codex). Supports GitHub Actions expressions for reusable workflow_call workflows.", |
| default: | ||
| frontmatterMetadataLog.Printf("Invalid tools.timeout type: %T", timeoutValue) | ||
| return 0, fmt.Errorf("tools.timeout must be an integer, got %T", timeoutValue) | ||
| return "", fmt.Errorf("tools.timeout must be an integer, got %T", timeoutValue) |
There was a problem hiding this comment.
This error message says tools.timeout "must be an integer" even though expression strings are now accepted. Consider updating it to "must be an integer or a GitHub Actions expression" for consistency with the string-type error path and the updated schema.
This issue also appears on line 227 of the same file.
| return "", fmt.Errorf("tools.timeout must be an integer, got %T", timeoutValue) | |
| return "", fmt.Errorf("tools.timeout must be an integer or a GitHub Actions expression, got %T", timeoutValue) |
|
@copilot review comments |
- Add pattern constraint to schema string branches for tools.timeout and tools.startup-timeout so non-expression strings are rejected at schema validation time - Fix tools.timeout description: default is 60s for both engines (not 120s for Codex) - Update error messages in frontmatter_extraction_metadata.go to say "must be an integer or a GitHub Actions expression" consistently Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4d9980f1-e3e9-4ffd-842d-bbde52fa5558 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all 4 review comments in 40787c1:
|
…engine.version, and tools.timeout
- frontmatter.md: note that timeout-minutes accepts expressions for workflow_call parameterization
- engines.md: add example showing engine.version with ${{ inputs.engine-version }}, noting injection-safe env-var treatment
- tools.md: add Tool Timeout Configuration section documenting tools.timeout and tools.startup-timeout fields, including expression support and Codex TOML limitation
Covers: #23863, #23870, #23888
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tools.timeoutandtools.startup-timeoutonly accepted literal integers, blocking reusableworkflow_callworkflows from customizing tool behavior at call time via inputs.Changes
Type changes
ToolsConfig.Timeout/StartupTimeout:*int→*stringWorkflowData.ToolsTimeout/ToolsStartupTimeout:int→stringtoolsProcessingResultfields likewise changed tostringParsing
parseTimeoutTool/parseStartupTimeoutToolhandle expression strings alongside all numeric typesextractToolsTimeout/extractToolsStartupTimeoutreturnstring; accept expressions, reject non-expression strings with actionable error messagesisExpressionString()helper intemplatables.goconsolidates the${{…}}checkEngine behavior
timeout: 60): unchanged — emitted as numeric string env vars; used for arithmetic (ms conversion in Claude)timeout: ${{ inputs.tool-timeout }}): emitted directly into env vars in compiled YAML; TOML-based configs (Codex MCP gateway) fall back to defaults since TOML has no expression syntaxSchema
tools.timeoutandtools.startup-timeoutrelaxed tooneOf: [integer (min:1), string]Example
Compiles to:
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/graphql/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GO111MODULE 64/pkg/tool/linu--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linu2 /usr/bin/git -json GO111MODULE ache/go/1.25.0/x--show-toplevel git(http block)/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw 993529/b430/imporev-parse ache/go/1.25.0/x--show-toplevel git rev-�� --show-toplevel git /usr/bin/git --show-toplevel nly /usr/bin/git git(http block)/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw git /usr/bin/git git rev-�� --show-toplevel git /usr/bin/git --show-toplevel git /usr/bin/git git(http block)https://api.github.com/orgs/test-owner/actions/secrets/usr/bin/gh gh api /orgs/test-owner/actions/secrets --jq .secrets[].name -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go k/gh�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD erignore go(http block)https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha user.name Test User /usr/bin/git ub/workflows **/*.cjs 64/bin/go git conf�� --get remote.origin.url /usr/bin/infocmp -json GO111MODULE x_amd64/compile infocmp(http block)/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha --show-toplevel git /usr/bin/git --show-toplevel go /usr/bin/git git rev-�� --show-toplevel git /usr/bin/git --show-toplevel go 64/bin/node git(http block)Arev-parse ache/node/24.14.--show-toplevel git` (http block)
Allow tools.timeout, tools.startu/tmp/fuzz-expr-2673999044.js bin/git --show-toplevel git /usr/lib/git-cor--show-toplevel git` (http block)
A-U0 flow.test name git /usr/bin/git flow.test` (http block)