Skip to content

Commit ff734b3

Browse files
authored
Fix dispatch_workflow tool registration in Safe Outputs MCP HTTP server (#13179)
1 parent a2bc819 commit ff734b3

File tree

2 files changed

+215
-4
lines changed

2 files changed

+215
-4
lines changed

actions/setup/js/safe_outputs_mcp_server_http.cjs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,25 @@ function createMCPServer(options = {}) {
104104

105105
// Register predefined tools
106106
for (const tool of toolsWithHandlers) {
107-
// Check if tool is enabled in configuration
108-
if (!enabledTools.has(tool.name)) {
109-
logger.debug(`Skipping tool ${tool.name} - not enabled in config`);
110-
continue;
107+
// Check if this is a dispatch_workflow tool (has _workflow_name metadata)
108+
// These tools are dynamically generated with workflow-specific names
109+
const isDispatchWorkflowTool = !!tool._workflow_name;
110+
111+
if (isDispatchWorkflowTool) {
112+
logger.debug(`Found dispatch_workflow tool: ${tool.name} (_workflow_name: ${tool._workflow_name})`);
113+
if (!safeOutputsConfig.dispatch_workflow) {
114+
logger.debug(` WARNING: dispatch_workflow config is missing or falsy - tool will NOT be registered`);
115+
logger.debug(` Config keys: ${Object.keys(safeOutputsConfig).join(", ")}`);
116+
logger.debug(` config.dispatch_workflow value: ${JSON.stringify(safeOutputsConfig.dispatch_workflow)}`);
117+
continue;
118+
}
119+
logger.debug(` dispatch_workflow config exists, registering tool`);
120+
} else {
121+
// Check if regular tool is enabled in configuration
122+
if (!enabledTools.has(tool.name)) {
123+
logger.debug(`Skipping tool ${tool.name} - not enabled in config`);
124+
continue;
125+
}
111126
}
112127

113128
logger.debug(`Registering tool: ${tool.name}`);
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
// @ts-check
2+
import { describe, it, expect, beforeEach, vi } from "vitest";
3+
4+
/**
5+
* Test for dispatch_workflow tool registration in HTTP server
6+
*
7+
* This test validates that the HTTP server correctly registers dispatch_workflow tools
8+
* that have _workflow_name metadata. These tools use workflow-specific names
9+
* (e.g., "test_workflow") while the config key is "dispatch_workflow".
10+
*
11+
* Reference: Issue where dispatch_workflow tools were not being registered because
12+
* the HTTP server's registration loop only checked if the tool name matched a config key,
13+
* but dispatch_workflow tools don't match - they have workflow-specific names with metadata.
14+
*/
15+
describe("safe_outputs_mcp_server_http dispatch_workflow registration", () => {
16+
it("should register dispatch_workflow tools when config.dispatch_workflow exists", () => {
17+
// Simulate the tool with _workflow_name metadata
18+
const tool = {
19+
name: "test_workflow",
20+
_workflow_name: "test-workflow",
21+
description: "Dispatch the 'test-workflow' workflow",
22+
inputSchema: {
23+
type: "object",
24+
properties: {
25+
test_param: {
26+
type: "string",
27+
description: "Test parameter",
28+
},
29+
},
30+
additionalProperties: false,
31+
},
32+
};
33+
34+
// Simulate the config with dispatch_workflow key
35+
const config = {
36+
dispatch_workflow: {
37+
max: 1,
38+
workflows: ["test-workflow"],
39+
workflow_files: {
40+
"test-workflow": ".yml",
41+
},
42+
},
43+
missing_tool: {},
44+
missing_data: {},
45+
noop: { max: 1 },
46+
};
47+
48+
// Simulate the enabledTools set (based on config keys)
49+
const enabledTools = new Set();
50+
for (const [toolName, enabled] of Object.entries(config)) {
51+
if (enabled) {
52+
enabledTools.add(toolName);
53+
}
54+
}
55+
56+
// Test the registration logic (extracted from safe_outputs_mcp_server_http.cjs)
57+
const isDispatchWorkflowTool = !!tool._workflow_name;
58+
let shouldRegister = false;
59+
60+
if (isDispatchWorkflowTool) {
61+
// Dispatch workflow tools should be registered if config.dispatch_workflow exists
62+
if (config.dispatch_workflow) {
63+
shouldRegister = true;
64+
}
65+
} else {
66+
// Regular tools should be registered if their name is in enabledTools
67+
if (enabledTools.has(tool.name)) {
68+
shouldRegister = true;
69+
}
70+
}
71+
72+
// Verify that the dispatch_workflow tool should be registered
73+
expect(shouldRegister).toBe(true);
74+
expect(isDispatchWorkflowTool).toBe(true);
75+
expect(config.dispatch_workflow).toBeTruthy();
76+
});
77+
78+
it("should NOT register dispatch_workflow tools when config.dispatch_workflow is missing", () => {
79+
// Simulate the tool with _workflow_name metadata
80+
const tool = {
81+
name: "test_workflow",
82+
_workflow_name: "test-workflow",
83+
description: "Dispatch the 'test-workflow' workflow",
84+
inputSchema: { type: "object", properties: {} },
85+
};
86+
87+
// Config WITHOUT dispatch_workflow
88+
const config = {
89+
missing_tool: {},
90+
missing_data: {},
91+
noop: { max: 1 },
92+
};
93+
94+
const enabledTools = new Set();
95+
for (const [toolName, enabled] of Object.entries(config)) {
96+
if (enabled) {
97+
enabledTools.add(toolName);
98+
}
99+
}
100+
101+
const isDispatchWorkflowTool = !!tool._workflow_name;
102+
let shouldRegister = false;
103+
104+
if (isDispatchWorkflowTool) {
105+
if (config.dispatch_workflow) {
106+
shouldRegister = true;
107+
}
108+
} else {
109+
if (enabledTools.has(tool.name)) {
110+
shouldRegister = true;
111+
}
112+
}
113+
114+
// Verify that the tool should NOT be registered
115+
expect(shouldRegister).toBe(false);
116+
expect(isDispatchWorkflowTool).toBe(true);
117+
expect(config.dispatch_workflow).toBeFalsy();
118+
});
119+
120+
it("should register regular tools when their name is in config", () => {
121+
// Regular tool (no _workflow_name)
122+
const tool = {
123+
name: "missing_tool",
124+
description: "Report missing tool",
125+
inputSchema: { type: "object", properties: {} },
126+
};
127+
128+
const config = {
129+
missing_tool: {},
130+
missing_data: {},
131+
};
132+
133+
const enabledTools = new Set();
134+
for (const [toolName, enabled] of Object.entries(config)) {
135+
if (enabled) {
136+
enabledTools.add(toolName);
137+
}
138+
}
139+
140+
const isDispatchWorkflowTool = !!tool._workflow_name;
141+
let shouldRegister = false;
142+
143+
if (isDispatchWorkflowTool) {
144+
if (config.dispatch_workflow) {
145+
shouldRegister = true;
146+
}
147+
} else {
148+
if (enabledTools.has(tool.name)) {
149+
shouldRegister = true;
150+
}
151+
}
152+
153+
// Verify that regular tool should be registered
154+
expect(shouldRegister).toBe(true);
155+
expect(isDispatchWorkflowTool).toBe(false);
156+
expect(enabledTools.has(tool.name)).toBe(true);
157+
});
158+
159+
it("should NOT register regular tools when their name is not in config", () => {
160+
const tool = {
161+
name: "some_other_tool",
162+
description: "Some other tool",
163+
inputSchema: { type: "object", properties: {} },
164+
};
165+
166+
const config = {
167+
missing_tool: {},
168+
missing_data: {},
169+
};
170+
171+
const enabledTools = new Set();
172+
for (const [toolName, enabled] of Object.entries(config)) {
173+
if (enabled) {
174+
enabledTools.add(toolName);
175+
}
176+
}
177+
178+
const isDispatchWorkflowTool = !!tool._workflow_name;
179+
let shouldRegister = false;
180+
181+
if (isDispatchWorkflowTool) {
182+
if (config.dispatch_workflow) {
183+
shouldRegister = true;
184+
}
185+
} else {
186+
if (enabledTools.has(tool.name)) {
187+
shouldRegister = true;
188+
}
189+
}
190+
191+
// Verify that the tool should NOT be registered
192+
expect(shouldRegister).toBe(false);
193+
expect(isDispatchWorkflowTool).toBe(false);
194+
expect(enabledTools.has(tool.name)).toBe(false);
195+
});
196+
});

0 commit comments

Comments
 (0)