Skip to content

Commit f760044

Browse files
Complete feature flag testing with successful validation
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parent 6c1bde3 commit f760044

File tree

1 file changed

+68
-0
lines changed

1 file changed

+68
-0
lines changed

pkg/github/actions_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/github/github-mcp-server/internal/profiler"
1616
"github.com/github/github-mcp-server/internal/toolsnaps"
1717
buffer "github.com/github/github-mcp-server/pkg/buffer"
18+
"github.com/github/github-mcp-server/pkg/inventory"
1819
"github.com/github/github-mcp-server/pkg/translations"
1920
"github.com/google/go-github/v79/github"
2021
"github.com/google/jsonschema-go/jsonschema"
@@ -2300,6 +2301,73 @@ func Test_ActionsRunTrigger_CancelWorkflowRun(t *testing.T) {
23002301
})
23012302
}
23022303

2304+
// TestGetJobLogsFeatureFlagScenario tests the specific scenario from GitHub issue #878
2305+
// where get_job_logs tool calls were failing with "unknown tool" errors.
2306+
// This validates that both GetJobLogs and ActionsGetJobLogs tools are properly
2307+
// handled with feature flags and that only one is active at a time.
2308+
func TestGetJobLogsFeatureFlagScenario(t *testing.T) {
2309+
tools := AllTools(stubTranslation)
2310+
2311+
// Find all get_job_logs tools
2312+
var getJobLogsTools []inventory.ServerTool
2313+
for _, tool := range tools {
2314+
if tool.Tool.Name == "get_job_logs" {
2315+
getJobLogsTools = append(getJobLogsTools, tool)
2316+
}
2317+
}
2318+
2319+
// Should have exactly 2 variants
2320+
require.Len(t, getJobLogsTools, 2, "Should have exactly 2 get_job_logs tool variants")
2321+
2322+
// One should have FeatureFlagDisable, one should have FeatureFlagEnable
2323+
var hasEnable, hasDisable bool
2324+
var enableTool, disableTool inventory.ServerTool
2325+
2326+
for _, tool := range getJobLogsTools {
2327+
if tool.FeatureFlagEnable != "" {
2328+
hasEnable = true
2329+
enableTool = tool
2330+
assert.Equal(t, FeatureFlagConsolidatedActions, tool.FeatureFlagEnable,
2331+
"FeatureFlagEnable should be set to FeatureFlagConsolidatedActions")
2332+
}
2333+
if tool.FeatureFlagDisable != "" {
2334+
hasDisable = true
2335+
disableTool = tool
2336+
assert.Equal(t, FeatureFlagConsolidatedActions, tool.FeatureFlagDisable,
2337+
"FeatureFlagDisable should be set to FeatureFlagConsolidatedActions")
2338+
}
2339+
}
2340+
2341+
require.True(t, hasEnable, "One get_job_logs variant should have FeatureFlagEnable")
2342+
require.True(t, hasDisable, "One get_job_logs variant should have FeatureFlagDisable")
2343+
2344+
// Test that feature flag filtering works correctly
2345+
// Build inventory without feature checker (flag OFF by default)
2346+
regFlagOff := NewInventory(stubTranslation).WithToolsets([]string{"all"}).Build()
2347+
filteredOff := regFlagOff.ForMCPRequest(inventory.MCPMethodToolsCall, "get_job_logs")
2348+
availableOff := filteredOff.AvailableTools(context.Background())
2349+
2350+
require.Len(t, availableOff, 1, "With flag OFF, should have exactly 1 get_job_logs tool")
2351+
assert.Equal(t, FeatureFlagConsolidatedActions, availableOff[0].FeatureFlagDisable,
2352+
"With flag OFF, should get the tool with FeatureFlagDisable")
2353+
2354+
// Build inventory with feature checker (flag ON)
2355+
checkerOn := func(_ context.Context, flag string) (bool, error) {
2356+
return flag == FeatureFlagConsolidatedActions, nil
2357+
}
2358+
regFlagOn := NewInventory(stubTranslation).WithToolsets([]string{"all"}).WithFeatureChecker(checkerOn).Build()
2359+
filteredOn := regFlagOn.ForMCPRequest(inventory.MCPMethodToolsCall, "get_job_logs")
2360+
availableOn := filteredOn.AvailableTools(context.Background())
2361+
2362+
require.Len(t, availableOn, 1, "With flag ON, should have exactly 1 get_job_logs tool")
2363+
assert.Equal(t, FeatureFlagConsolidatedActions, availableOn[0].FeatureFlagEnable,
2364+
"With flag ON, should get the tool with FeatureFlagEnable")
2365+
2366+
// Verify both tools have handlers (this was the original issue - tool found but couldn't be called)
2367+
assert.True(t, enableTool.HasHandler(), "Consolidated tool should have handler")
2368+
assert.True(t, disableTool.HasHandler(), "Original tool should have handler")
2369+
}
2370+
23032371
func Test_ActionsGetJobLogs(t *testing.T) {
23042372
// Verify tool definition once
23052373
toolDef := ActionsGetJobLogs(translations.NullTranslationHelper)

0 commit comments

Comments
 (0)