Skip to content

Commit 6c41e78

Browse files
Copilotpelikhan
andauthored
[WIP] Fix failing GitHub Actions job Integration: CLI MCP Other (#35200)
* Initial plan * fix: fail fast on repo-only MCP add workflow specs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * test: cover repo-only MCP add spec validation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * test: align MCP tools management unit test conventions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * test: use testify assertions for MCP spec guard tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * docs: clarify repo-only MCP add spec detection Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent 10dbbd9 commit 6c41e78

2 files changed

Lines changed: 75 additions & 0 deletions

File tree

pkg/cli/mcp_tools_management.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli
33
import (
44
"context"
55
"strconv"
6+
"strings"
67

78
"github.com/github/gh-aw/pkg/logger"
89
"github.com/modelcontextprotocol/go-sdk/jsonrpc"
@@ -41,6 +42,14 @@ func registerAddTool(server *mcp.Server, execCmd execCmdFunc) {
4142
if len(args.Workflows) == 0 {
4243
return nil, nil, newMCPError(jsonrpc.CodeInvalidParams, "missing required parameter: at least one workflow specification is required", nil)
4344
}
45+
for _, workflowSpec := range args.Workflows {
46+
if isRepositoryOnlyWorkflowSpec(workflowSpec) {
47+
return nil, nil, newMCPError(jsonrpc.CodeInvalidParams, "failed to add workflows", map[string]any{
48+
"error": "workflow specification must be in format 'owner/repo/workflow-name[@version]'",
49+
"spec": workflowSpec,
50+
})
51+
}
52+
}
4453

4554
mcpToolsManagementLog.Printf("add tool invoked: workflows=%d, number=%d, name=%q", len(args.Workflows), args.Number, args.Name)
4655

@@ -74,6 +83,17 @@ func registerAddTool(server *mcp.Server, execCmd execCmdFunc) {
7483
})
7584
}
7685

86+
func isRepositoryOnlyWorkflowSpec(spec string) bool {
87+
if strings.HasPrefix(spec, "http://") || strings.HasPrefix(spec, "https://") || isLocalWorkflowPath(spec) {
88+
return false
89+
}
90+
91+
// Strip optional @version suffix so owner/repo@v1 is treated as repository-only.
92+
specWithoutVersion := strings.SplitN(spec, "@", 2)[0]
93+
slashParts := strings.Split(specWithoutVersion, "/")
94+
return len(slashParts) == 2 && slashParts[0] != "" && slashParts[1] != ""
95+
}
96+
7797
// updateArgs holds the input parameters for the update tool.
7898
type updateArgs struct {
7999
Workflows []string `json:"workflows,omitempty" jsonschema:"Workflow IDs to update (empty for all workflows)"`
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//go:build !integration
2+
3+
package cli
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestIsRepositoryOnlyWorkflowSpec(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
spec string
15+
want bool
16+
}{
17+
{
18+
name: "repo only",
19+
spec: "owner/repo",
20+
want: true,
21+
},
22+
{
23+
name: "repo only with version",
24+
spec: "owner/repo@v1.0.0",
25+
want: true,
26+
},
27+
{
28+
name: "repo with workflow path",
29+
spec: "owner/repo/workflow",
30+
want: false,
31+
},
32+
{
33+
name: "repo with workflow path and version",
34+
spec: "owner/repo/workflow@main",
35+
want: false,
36+
},
37+
{
38+
name: "github url",
39+
spec: "https://github.com/owner/repo/blob/main/.github/workflows/test.md",
40+
want: false,
41+
},
42+
{
43+
name: "local path",
44+
spec: "./.github/workflows/test.md",
45+
want: false,
46+
},
47+
}
48+
49+
for _, tt := range tests {
50+
t.Run(tt.name, func(t *testing.T) {
51+
got := isRepositoryOnlyWorkflowSpec(tt.spec)
52+
assert.Equal(t, tt.want, got, "isRepositoryOnlyWorkflowSpec(%q) should return %v", tt.spec, tt.want)
53+
})
54+
}
55+
}

0 commit comments

Comments
 (0)