fix: cubic feedback for workflow files

Author: RomneyDa

extensions/cli/src/hooks/useService.ts

@@ -123,7 +123,7 @@ export function useServices<T extends Record<string, any>>(
       hasError,
       services,
     };
-  }, [serviceNames]);
+  }, [serviceNames, container]);
 
   const [loading, setLoading] = useState(true);
   const [error, setError] = useState<Error | null>(null);

extensions/cli/src/services/workflow-integration.test.ts

@@ -379,6 +379,7 @@ describe("Workflow Integration Tests", () => {
 
       const baseConfig = {
         rules: ["existing rule"],
+        prompts: [],
       };
 
       const enhancedConfig = await configEnhancer.enhanceConfig(
@@ -387,34 +388,42 @@ describe("Workflow Integration Tests", () => {
         workflowService.getState(),
       );
 
-      // Prompts should be processed at runtime, not injected into config
-      // The workflow prompt is added to options.prompt and will be processed
-      // by processAndCombinePrompts() in chat.ts
+      // Workflow prompt should be added to config.prompts
+      expect(enhancedConfig.prompts).toBeDefined();
+      expect(enhancedConfig.prompts?.length).toBeGreaterThan(0);
+      expect(enhancedConfig.prompts?.[0]).toMatchObject({
+        prompt: "You are a workflow assistant.",
+        name: expect.stringContaining("Test Workflow"),
+      });
       expect(enhancedConfig.rules).toHaveLength(2);
     });
 
-    it("should add workflow prompt as prefix to existing prompts", async () => {
+    it("should add workflow prompt to config alongside other prompts", async () => {
       // Setup workflow service with active workflow
       mockLoadPackageFromHub.mockResolvedValue(mockWorkflowFile);
       await workflowService.initialize("owner/workflow");
 
-      // Test that the workflow prompt gets added to options.prompt as prefix
-      const baseOptions = { prompt: ["existing-prompt"] };
-      const modifiedOptions = { ...baseOptions };
+      const baseConfig = {
+        prompts: [{ name: "Existing", prompt: "existing-prompt" }],
+      };
+      const baseOptions = {};
 
-      // Simulate what ConfigEnhancer does internally
-      const workflowState = workflowService.getState();
-      if (workflowState.workflowFile?.prompt) {
-        modifiedOptions.prompt = [
-          workflowState.workflowFile.prompt,
-          ...(modifiedOptions.prompt || []),
-        ];
-      }
-
-      expect(modifiedOptions.prompt).toEqual([
-        "You are a workflow assistant.",
-        "existing-prompt",
-      ]);
+      const enhancedConfig = await configEnhancer.enhanceConfig(
+        baseConfig as any,
+        baseOptions,
+        workflowService.getState(),
+      );
+
+      // Workflow prompt should be prepended to existing prompts
+      expect(enhancedConfig.prompts).toHaveLength(2);
+      expect(enhancedConfig.prompts?.[0]).toMatchObject({
+        name: expect.stringContaining("Test Workflow"),
+        prompt: "You are a workflow assistant.",
+      });
+      expect(enhancedConfig.prompts?.[1]).toMatchObject({
+        name: "Existing",
+        prompt: "existing-prompt",
+      });
     });
 
     it("should not add workflow prompt when workflow has no prompt", async () => {
@@ -426,43 +435,53 @@ describe("Workflow Integration Tests", () => {
       mockLoadPackageFromHub.mockResolvedValue(workflowWithoutPrompt);
       await workflowService.initialize("owner/workflow");
 
-      const baseOptions = { prompt: ["existing-prompt"] };
-      const modifiedOptions = { ...baseOptions };
+      const baseConfig = {
+        prompts: [{ name: "Existing", prompt: "existing-prompt" }],
+      };
+      const baseOptions = {};
+
+      const enhancedConfig = await configEnhancer.enhanceConfig(
+        baseConfig as any,
+        baseOptions,
+        workflowService.getState(),
+      );
 
-      // Simulate what ConfigEnhancer does internally
-      const workflowState = workflowService.getState();
-      if (workflowState.workflowFile?.prompt) {
-        modifiedOptions.prompt = [
-          workflowState.workflowFile.prompt,
-          ...(modifiedOptions.prompt || []),
-        ];
-      }
-
-      expect(modifiedOptions.prompt).toEqual(["existing-prompt"]);
+      // Should only have the existing prompt, no workflow prompt added
+      expect(enhancedConfig.prompts).toHaveLength(1);
+      expect(enhancedConfig.prompts?.[0]).toMatchObject({
+        name: "Existing",
+        prompt: "existing-prompt",
+      });
     });
   });
 
   describe("ConfigEnhancer prompt integration", () => {
-    it("should modify options to include workflow prompt as prefix", async () => {
+    it("should add workflow prompt to config.prompts when workflow is active", async () => {
       // Setup workflow service with active workflow
       mockLoadPackageFromHub.mockResolvedValue(mockWorkflowFile);
       await workflowService.initialize("owner/workflow");
 
       const baseOptions = { prompt: ["user-prompt"] };
+      const baseConfig = { prompts: [] };
 
-      // This should internally modify the options to include workflow prompt
-      await configEnhancer.enhanceConfig(
-        {} as any,
+      // Enhance config with workflow state
+      const enhancedConfig = await configEnhancer.enhanceConfig(
+        baseConfig as any,
         baseOptions,
         workflowService.getState(),
       );
 
-      // Verify that the workflow prompt was added to the options
-      // by checking that injectPrompts was called with workflow prompt prefixed
-      expect(baseOptions.prompt).toHaveLength(1);
+      // Verify that the workflow prompt was added to config.prompts
+      expect(enhancedConfig.prompts).toBeDefined();
+      expect(enhancedConfig.prompts).toHaveLength(1);
+      expect(enhancedConfig.prompts?.[0]).toMatchObject({
+        name: expect.stringContaining("Test Workflow"),
+        prompt: "You are a workflow assistant.",
+        description: "A test workflow for integration testing",
+      });
     });
 
-    it("should work end-to-end with workflow prompt processing", async () => {
+    it("should work end-to-end with workflow prompt in config", async () => {
       // Setup workflow service with active workflow
       mockLoadPackageFromHub.mockResolvedValue(mockWorkflowFile);
       await workflowService.initialize("owner/workflow");
@@ -472,24 +491,23 @@ describe("Workflow Integration Tests", () => {
         "You are a workflow assistant.",
       );
 
-      // Verify that if we manually apply the same logic as ConfigEnhancer,
-      // the workflow prompt gets added as prefix
-      const options = { prompt: ["Tell me about TypeScript"] };
+      const baseConfig = { prompts: [] };
+      const baseOptions = { prompt: ["Tell me about TypeScript"] };
 
-      if (workflowState.workflowFile?.prompt) {
-        options.prompt = [workflowState.workflowFile.prompt, ...options.prompt];
-      }
+      // Enhance config with workflow
+      const enhancedConfig = await configEnhancer.enhanceConfig(
+        baseConfig as any,
+        baseOptions,
+        workflowState,
+      );
 
-      expect(options.prompt).toEqual([
+      // Verify workflow prompt is added to config.prompts
+      expect(enhancedConfig.prompts).toBeDefined();
+      expect(enhancedConfig.prompts?.length).toBeGreaterThan(0);
+      expect(enhancedConfig.prompts?.[0]?.prompt).toBe(
         "You are a workflow assistant.",
-        "Tell me about TypeScript",
-      ]);
-
-      // This mimics what processAndCombinePrompts would do
-      const combinedPrompt = options.prompt.join("\n\n");
-      expect(combinedPrompt).toBe(
-        "You are a workflow assistant.\n\nTell me about TypeScript",
       );
+      expect(enhancedConfig.prompts?.[0]?.name).toContain("Test Workflow");
     });
   });
 

extensions/cli/src/stream/streamChatResponse.getAllTools.test.ts

@@ -13,16 +13,7 @@ import { getAllTools } from "./streamChatResponse.js";
 describe("getAllTools - Tool Filtering", () => {
   beforeEach(() => {
     // Clean up service container state before each test
-    const services = [
-      SERVICE_NAMES.TOOL_PERMISSIONS,
-      SERVICE_NAMES.AUTH,
-      SERVICE_NAMES.API_CLIENT,
-      SERVICE_NAMES.CONFIG,
-      SERVICE_NAMES.MODEL,
-      SERVICE_NAMES.MCP,
-    ];
-
-    services.forEach((service) => {
+    Object.values(SERVICE_NAMES).forEach((service) => {
       // Reset service state
       (serviceContainer as any).services.delete(service);
       (serviceContainer as any).factories.delete(service);

extensions/cli/src/stream/streamChatResponse.modeSwitch.test.ts

@@ -13,9 +13,7 @@ describe("streamChatResponse - Mode Switch During Streaming", () => {
   const toolPermissionService = services.toolPermissions;
   beforeEach(async () => {
     // Clean up service container state before each test
-    const services = Object.keys(SERVICE_NAMES);
-
-    services.forEach((service) => {
+    Object.values(SERVICE_NAMES).forEach((service) => {
       (serviceContainer as any).services.delete(service);
       (serviceContainer as any).factories.delete(service);
       (serviceContainer as any).dependencies.delete(service);

extensions/cli/src/stream/streamChatResponse.ts

@@ -391,6 +391,7 @@ export async function streamChatResponse(
     // Get response from LLM
     const { content, toolCalls, shouldContinue } =
       await processStreamingResponse({
+        isHeadless,
         chatHistory,
         model,
         llmApi,

packages/config-yaml/src/markdown/workflowFiles.test.ts

@@ -485,4 +485,120 @@ describe("parseWorkflowTools", () => {
       allBuiltIn: false,
     });
   });
+
+  describe("whitespace validation in colon-separated MCP tool references", () => {
+    it("should reject MCP tool reference with space after colon", () => {
+      expect(() => parseWorkflowTools("owner/slug: tool")).toThrow(
+        'Invalid MCP tool reference "owner/slug: tool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with space before colon", () => {
+      expect(() => parseWorkflowTools("owner/slug :tool")).toThrow(
+        'Invalid MCP tool reference "owner/slug :tool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with spaces around colon", () => {
+      expect(() => parseWorkflowTools("owner/slug : tool")).toThrow(
+        'Invalid MCP tool reference "owner/slug : tool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with space in tool name", () => {
+      expect(() => parseWorkflowTools("owner/slug:my tool")).toThrow(
+        'Invalid MCP tool reference "owner/slug:my tool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with space in server slug", () => {
+      expect(() => parseWorkflowTools("owner /slug:tool")).toThrow(
+        'Invalid MCP tool reference "owner /slug:tool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with tab character", () => {
+      expect(() => parseWorkflowTools("owner/slug:\ttool")).toThrow(
+        'Invalid MCP tool reference "owner/slug:\ttool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with newline character", () => {
+      expect(() => parseWorkflowTools("owner/slug:\ntool")).toThrow(
+        'Invalid MCP tool reference "owner/slug:\ntool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with multiple spaces", () => {
+      expect(() => parseWorkflowTools("owner/slug:  tool  ")).toThrow(
+        'Invalid MCP tool reference "owner/slug:  tool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should reject MCP tool reference with leading spaces in tool name", () => {
+      expect(() => parseWorkflowTools("owner/slug:  tool")).toThrow(
+        'Invalid MCP tool reference "owner/slug:  tool": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should accept valid colon-separated MCP tool reference without whitespace", () => {
+      const result = parseWorkflowTools("owner/package:tool_name");
+      expect(result).toEqual({
+        tools: [{ mcpServer: "owner/package", toolName: "tool_name" }],
+        mcpServers: ["owner/package"],
+        allBuiltIn: false,
+      });
+    });
+
+    it("should accept valid MCP tool references with hyphens and underscores", () => {
+      const result = parseWorkflowTools(
+        "owner-name/package_name:tool-name_123",
+      );
+      expect(result).toEqual({
+        tools: [
+          {
+            mcpServer: "owner-name/package_name",
+            toolName: "tool-name_123",
+          },
+        ],
+        mcpServers: ["owner-name/package_name"],
+        allBuiltIn: false,
+      });
+    });
+
+    it("should allow whitespace between comma-separated items", () => {
+      const result = parseWorkflowTools(
+        "owner/package:tool1,   owner/package:tool2  ,  bash",
+      );
+      expect(result).toEqual({
+        tools: [
+          { mcpServer: "owner/package", toolName: "tool1" },
+          { mcpServer: "owner/package", toolName: "tool2" },
+          { toolName: "bash" },
+        ],
+        mcpServers: ["owner/package"],
+        allBuiltIn: false,
+      });
+    });
+
+    it("should reject whitespace in one reference but accept valid references in same string", () => {
+      expect(() =>
+        parseWorkflowTools("owner/package:tool1, owner/package: tool2, bash"),
+      ).toThrow(
+        'Invalid MCP tool reference "owner/package: tool2": colon-separated tool references cannot contain whitespace',
+      );
+    });
+
+    it("should not reject whitespace in MCP server references without colon", () => {
+      // Note: This behavior may or may not be desired, but documents current behavior
+      // Server-only references (no colon) don't trigger the whitespace check
+      expect(() => parseWorkflowTools("owner /package")).not.toThrow();
+    });
+
+    it("should not reject whitespace in built-in tool names", () => {
+      // Note: This behavior may or may not be desired, but documents current behavior
+      // Built-in tools (no slash) don't trigger the whitespace check
+      expect(() => parseWorkflowTools("my tool")).not.toThrow();
+    });
+  });
 });

packages/config-yaml/src/markdown/workflowFiles.ts

@@ -123,6 +123,14 @@ export function parseWorkflowTools(toolsString?: string): ParsedWorkflowTools {
 
       if (colonIndex > 0) {
         // Specific tool: "owner/package:tool_name"
+        // Reject references with whitespace to prevent silent misconfigurations
+        if (/\s/.test(toolRef)) {
+          throw new Error(
+            `Invalid MCP tool reference "${toolRef}": colon-separated tool references cannot contain whitespace. ` +
+              `Use format "owner/slug:tool_name" without spaces.`,
+          );
+        }
+
         const mcpServer = toolRef.substring(0, colonIndex);
         const toolName = toolRef.substring(colonIndex + 1);