Skip to content

Commit 90ebf80

Browse files
committed
Merge branch 'main' of https://github.com/github/gh-aw
2 parents 07a9bbd + 0230474 commit 90ebf80

19 files changed

Lines changed: 1428 additions & 74 deletions

.changeset/patch-max-continuations.md

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.devcontainer/devcontainer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@
3333
"ghcr.io/anthropics/devcontainer-features/claude-code:1.0": {},
3434
"ghcr.io/devcontainers/features/copilot-cli:latest": {},
3535
"ghcr.io/devcontainers/features/docker-in-docker:2": {},
36-
"ghcr.io/devcontainers/features/github-cli:1": {},
3736
"ghcr.io/devcontainers/features/git-lfs:1": {},
37+
"ghcr.io/devcontainers/features/github-cli:1": {},
3838
"ghcr.io/devcontainers/features/node:1": {
3939
"version": "24"
4040
}

.github/workflows/smoke-copilot.lock.yml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/smoke-copilot.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ permissions:
1515
discussions: read
1616
actions: read
1717
name: Smoke Copilot
18-
engine: copilot
18+
engine:
19+
id: copilot
20+
max-continuations: 2
1921
imports:
2022
- shared/gh.md
2123
- shared/reporting.md
4.5 MB
Binary file not shown.
11.8 MB
Binary file not shown.

pkg/parser/import_cache_test.go

Lines changed: 120 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,38 +13,43 @@ import (
1313

1414
func TestImportCache(t *testing.T) {
1515
tempDir := t.TempDir()
16-
17-
// Create a new cache
1816
cache := NewImportCache(tempDir)
19-
20-
// Test Set and Get
17+
const (
18+
owner = "testowner"
19+
repo = "testrepo"
20+
path = "workflows/test.md"
21+
sha = "abc123"
22+
)
2123
testContent := []byte("# Test Workflow\n\nTest content")
22-
owner := "testowner"
23-
repo := "testrepo"
24-
path := "workflows/test.md"
25-
sha := "abc123"
26-
27-
cachedPath, err := cache.Set(owner, repo, path, sha, testContent)
28-
require.NoError(t, err, "Set should succeed for valid inputs")
29-
30-
// Verify file was created
31-
require.FileExists(t, cachedPath, "cache file should be created at expected path")
32-
33-
// Verify content
34-
content, err := os.ReadFile(cachedPath)
35-
require.NoError(t, err, "reading cached file should succeed")
36-
assert.Equal(t, string(testContent), string(content), "cached content should match original")
37-
38-
// Test Get
39-
retrievedPath, found := cache.Get(owner, repo, path, sha)
40-
assert.True(t, found, "cache entry should be found after Set")
41-
assert.Equal(t, cachedPath, retrievedPath, "retrieved path from Get should match path returned by Set")
4224

43-
// Test that a new cache instance can find the file
44-
cache2 := NewImportCache(tempDir)
45-
retrievedPath2, found := cache2.Get(owner, repo, path, sha)
46-
assert.True(t, found, "cache entry should be found from new cache instance")
47-
assert.Equal(t, cachedPath, retrievedPath2, "path from new cache instance should match original cached path")
25+
t.Run("Set creates file and returns path", func(t *testing.T) {
26+
cachedPath, err := cache.Set(owner, repo, path, sha, testContent)
27+
require.NoError(t, err, "Set should succeed for valid inputs")
28+
require.FileExists(t, cachedPath, "cache file should be created at expected path")
29+
})
30+
31+
t.Run("Get returns cached path after Set", func(t *testing.T) {
32+
cachedPath, _ := cache.Set(owner, repo, path, sha, testContent)
33+
retrievedPath, found := cache.Get(owner, repo, path, sha)
34+
assert.True(t, found, "cache entry should be found after Set")
35+
assert.Equal(t, cachedPath, retrievedPath, "retrieved path should match path returned by Set")
36+
})
37+
38+
t.Run("Cached file content matches original", func(t *testing.T) {
39+
cachedPath, err := cache.Set(owner, repo, path, sha, testContent)
40+
require.NoError(t, err, "Set should succeed")
41+
content, err := os.ReadFile(cachedPath)
42+
require.NoError(t, err, "reading cached file should succeed")
43+
assert.Equal(t, string(testContent), string(content), "cached content should match original")
44+
})
45+
46+
t.Run("New cache instance finds existing entry", func(t *testing.T) {
47+
cachedPath, _ := cache.Set(owner, repo, path, sha, testContent)
48+
cache2 := NewImportCache(tempDir)
49+
retrievedPath2, found := cache2.Get(owner, repo, path, sha)
50+
assert.True(t, found, "cache entry should be found from new cache instance")
51+
assert.Equal(t, cachedPath, retrievedPath2, "path from new instance should match original")
52+
})
4853
}
4954

5055
func TestImportCacheDirectory(t *testing.T) {
@@ -131,6 +136,26 @@ func TestSanitizePath(t *testing.T) {
131136
input: "a/./b/file.md",
132137
expected: "a_b_file.md",
133138
},
139+
{
140+
name: "empty string",
141+
input: "",
142+
expected: ".",
143+
},
144+
{
145+
name: "trailing slash",
146+
input: "a/b/",
147+
expected: "a_b",
148+
},
149+
{
150+
name: "single dot component",
151+
input: "a/./b",
152+
expected: "a_b",
153+
},
154+
{
155+
name: "leading slash becomes root-like",
156+
input: "/absolute/path",
157+
expected: "_absolute_path",
158+
},
134159
}
135160

136161
for _, tt := range tests {
@@ -204,6 +229,33 @@ func TestValidatePathComponents(t *testing.T) {
204229
shouldErr: true,
205230
errMsg: "absolute path",
206231
},
232+
{
233+
name: "empty repo",
234+
owner: "testowner",
235+
repo: "",
236+
path: "test.md",
237+
sha: "abc123",
238+
shouldErr: true,
239+
errMsg: "empty component",
240+
},
241+
{
242+
name: "empty path",
243+
owner: "testowner",
244+
repo: "testrepo",
245+
path: "",
246+
sha: "abc123",
247+
shouldErr: true,
248+
errMsg: "empty component",
249+
},
250+
{
251+
name: "path traversal embedded in sha",
252+
owner: "testowner",
253+
repo: "testrepo",
254+
path: "test.md",
255+
sha: "abc..def",
256+
shouldErr: true,
257+
errMsg: "..",
258+
},
207259
}
208260

209261
for _, tt := range tests {
@@ -272,6 +324,15 @@ func TestImportCacheSet_Validation(t *testing.T) {
272324
shouldErr: true,
273325
errMsg: "invalid path components",
274326
},
327+
{
328+
name: "valid inputs succeed",
329+
owner: "owner",
330+
repo: "repo",
331+
path: "workflows/test.md",
332+
sha: "abc123def456",
333+
content: []byte("# valid content"),
334+
shouldErr: false,
335+
},
275336
}
276337

277338
cache := NewImportCache(tempDir)
@@ -287,3 +348,33 @@ func TestImportCacheSet_Validation(t *testing.T) {
287348
})
288349
}
289350
}
351+
352+
func TestImportCacheSetIdempotent(t *testing.T) {
353+
tempDir := t.TempDir()
354+
cache := NewImportCache(tempDir)
355+
356+
firstContent := []byte("first content")
357+
secondContent := []byte("second content")
358+
359+
path1, err := cache.Set("owner", "repo", "test.md", "sha1", firstContent)
360+
require.NoError(t, err, "first Set should succeed")
361+
362+
path2, err := cache.Set("owner", "repo", "test.md", "sha1", secondContent)
363+
require.NoError(t, err, "second Set with same key should succeed (overwrite)")
364+
assert.Equal(t, path1, path2, "both Set calls should return the same cache path")
365+
366+
content, err := os.ReadFile(path2)
367+
require.NoError(t, err, "reading overwritten file should succeed")
368+
assert.Equal(t, string(secondContent), string(content), "file should contain second (latest) content")
369+
}
370+
371+
func TestImportCacheGetDoesNotValidateComponents(t *testing.T) {
372+
// Document that Get does not validate components (unlike Set).
373+
// If validation is added in the future, this test should be updated.
374+
tempDir := t.TempDir()
375+
cache := NewImportCache(tempDir)
376+
377+
// Should return not-found (not panic or error), even with suspicious inputs.
378+
_, found := cache.Get("../etc", "repo", "test.md", "sha")
379+
assert.False(t, found, "Get with path traversal input should return not-found, not panic")
380+
}

pkg/parser/schemas/main_workflow_schema.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7413,6 +7413,10 @@
74137413
"id": "copilot",
74147414
"version": "beta"
74157415
},
7416+
{
7417+
"id": "copilot",
7418+
"max-continuations": 5
7419+
},
74167420
{
74177421
"id": "claude",
74187422
"concurrency": {
@@ -7458,6 +7462,11 @@
74587462
],
74597463
"description": "Maximum number of chat iterations per run. Helps prevent runaway loops and control costs. Has sensible defaults and can typically be omitted. Note: Only supported by the claude engine."
74607464
},
7465+
"max-continuations": {
7466+
"type": "integer",
7467+
"minimum": 1,
7468+
"description": "Maximum number of continuations for multi-run autopilot mode. Default is 1 (single run, no autopilot). Values greater than 1 enable --autopilot mode for the copilot engine with --max-autopilot-continues set to this value. Note: Only supported by the copilot engine."
7469+
},
74617470
"concurrency": {
74627471
"oneOf": [
74637472
{

pkg/workflow/agent_validation.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//
1313
// - validateAgentFile() - Validates custom agent file exists
1414
// - validateMaxTurnsSupport() - Validates max-turns feature support
15+
// - validateMaxContinuationsSupport() - Validates max-continuations feature support
1516
// - validateWebSearchSupport() - Validates web-search feature support (warning)
1617
// - validateWorkflowRunBranches() - Validates workflow_run has branch restrictions
1718
//
@@ -122,6 +123,24 @@ func (c *Compiler) validateMaxTurnsSupport(frontmatter map[string]any, engine Co
122123
return nil
123124
}
124125

126+
// validateMaxContinuationsSupport validates that max-continuations is only used with engines that support this feature
127+
func (c *Compiler) validateMaxContinuationsSupport(frontmatter map[string]any, engine CodingAgentEngine) error {
128+
// Check if max-continuations is specified in the engine config
129+
_, engineConfig := c.ExtractEngineConfig(frontmatter)
130+
131+
if engineConfig == nil || engineConfig.MaxContinuations == 0 {
132+
// No max-continuations specified, no validation needed
133+
return nil
134+
}
135+
136+
// max-continuations is specified, check if the engine supports it
137+
if !engine.SupportsMaxContinuations() {
138+
return fmt.Errorf("max-continuations not supported: engine '%s' does not support the max-continuations feature", engine.GetID())
139+
}
140+
141+
return nil
142+
}
143+
125144
// validateWebSearchSupport validates that web-search tool is only used with engines that support this feature
126145
func (c *Compiler) validateWebSearchSupport(tools map[string]any, engine CodingAgentEngine) {
127146
// Check if web-search tool is requested

pkg/workflow/agentic_engine.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ type CapabilityProvider interface {
126126
// When true, plugins can be installed using the engine's plugin install command
127127
SupportsPlugins() bool
128128

129+
// SupportsMaxContinuations returns true if this engine supports the max-continuations feature
130+
// When true, max-continuations > 1 enables autopilot/multi-run mode for the engine
131+
SupportsMaxContinuations() bool
132+
129133
// SupportsLLMGateway returns the LLM gateway port number for this engine
130134
// Returns the port number (e.g., 10000) if the engine supports an LLM gateway
131135
// Returns -1 if the engine does not support an LLM gateway
@@ -207,17 +211,18 @@ type CodingAgentEngine interface {
207211

208212
// BaseEngine provides common functionality for agentic engines
209213
type BaseEngine struct {
210-
id string
211-
displayName string
212-
description string
213-
experimental bool
214-
supportsToolsAllowlist bool
215-
supportsMaxTurns bool
216-
supportsWebFetch bool
217-
supportsWebSearch bool
218-
supportsFirewall bool
219-
supportsPlugins bool
220-
supportsLLMGateway bool
214+
id string
215+
displayName string
216+
description string
217+
experimental bool
218+
supportsToolsAllowlist bool
219+
supportsMaxTurns bool
220+
supportsMaxContinuations bool
221+
supportsWebFetch bool
222+
supportsWebSearch bool
223+
supportsFirewall bool
224+
supportsPlugins bool
225+
supportsLLMGateway bool
221226
}
222227

223228
func (e *BaseEngine) GetID() string {
@@ -260,6 +265,10 @@ func (e *BaseEngine) SupportsPlugins() bool {
260265
return e.supportsPlugins
261266
}
262267

268+
func (e *BaseEngine) SupportsMaxContinuations() bool {
269+
return e.supportsMaxContinuations
270+
}
271+
263272
func (e *BaseEngine) SupportsLLMGateway() int {
264273
// Engines that support LLM gateway must override this method
265274
// to return their specific port number (e.g., 10000, 10001, 10002)

0 commit comments

Comments
 (0)