Skip to content

Commit 4d3bafe

Browse files
RomneyDaRomneyDa
committed
fix: move workflow stuff to config service
1 parent 9e15bc0 commit 4d3bafe

File tree

3 files changed

+67
-123
lines changed

3 files changed

+67
-123
lines changed

‎extensions/cli/src/configEnhancer.ts‎

Lines changed: 50 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -24,92 +24,73 @@ export class ConfigEnhancer {
2424
*/
2525
async enhanceConfig(
2626
config: AssistantUnrolled,
27-
options: BaseCommandOptions,
27+
_options: BaseCommandOptions,
2828
): Promise<AssistantUnrolled> {
2929
let enhancedConfig = { ...config };
30-
const workflowOptions = await this.addWorkflowOptions(options);
3130

32-
// Apply rules
33-
if (workflowOptions.rule && workflowOptions.rule.length > 0) {
34-
enhancedConfig = await this.injectRules(
35-
enhancedConfig,
36-
workflowOptions.rule,
37-
);
38-
}
39-
40-
// Apply MCPs
41-
if (workflowOptions.mcp && workflowOptions.mcp.length > 0) {
42-
enhancedConfig = await this.injectMcps(
43-
enhancedConfig,
44-
workflowOptions.mcp,
45-
);
46-
}
47-
48-
// Apply models
49-
if (workflowOptions.model && workflowOptions.model.length > 0) {
50-
enhancedConfig = await this.injectModels(
51-
enhancedConfig,
52-
workflowOptions.model,
53-
);
54-
}
55-
56-
// Apply prompts
57-
if (workflowOptions.prompt && workflowOptions.prompt.length > 0) {
58-
enhancedConfig = await this.injectPrompts(
59-
enhancedConfig,
60-
workflowOptions.prompt,
61-
);
62-
}
63-
64-
return enhancedConfig;
65-
}
66-
67-
/**
68-
* Add workflow rules and MCPs to options if workflow is active
69-
*/
70-
private async addWorkflowOptions(
71-
options: BaseCommandOptions,
72-
): Promise<BaseCommandOptions> {
31+
// Get workflow options and spread them inline
32+
let options = { ..._options };
7333
try {
7434
const workflowState = await serviceContainer.get<WorkflowServiceState>(
7535
SERVICE_NAMES.WORKFLOW,
7636
);
7737

78-
if (!workflowState.workflowFile) {
79-
return options;
80-
}
81-
82-
const enhancedOptions = { ...options };
38+
if (workflowState.workflowFile) {
39+
// Add workflow rules if present
40+
if (workflowState.workflowFile.rules) {
41+
options.rule = [
42+
workflowState.workflowFile.rules,
43+
...(options.rule || []),
44+
];
45+
logger.debug(`Added workflow rules from ${workflowState.workflow}`);
46+
}
8347

84-
// Add workflow rules if present
85-
if (workflowState.workflowFile.rules) {
86-
const existingRules = enhancedOptions.rule || [];
87-
enhancedOptions.rule = [
88-
workflowState.workflowFile.rules,
89-
...existingRules,
90-
];
91-
logger.debug(`Added workflow rules from ${workflowState.workflow}`);
92-
}
48+
// Add workflow MCP servers if present
49+
if (workflowState.workflowFile.tools) {
50+
const parsedTools = parseWorkflowTools(
51+
workflowState.workflowFile.tools,
52+
);
53+
if (parsedTools.mcpServers.length > 0) {
54+
options.mcp = [...parsedTools.mcpServers, ...(options.mcp || [])];
55+
logger.debug(
56+
`Added ${parsedTools.mcpServers.length} workflow MCP servers`,
57+
);
58+
}
59+
}
9360

94-
// Add workflow MCP servers if present
95-
if (workflowState.workflowFile.tools) {
96-
const parsedTools = parseWorkflowTools(
97-
workflowState.workflowFile.tools,
98-
);
99-
if (parsedTools.mcpServers.length > 0) {
100-
const existingMcps = enhancedOptions.mcp || [];
101-
enhancedOptions.mcp = [...parsedTools.mcpServers, ...existingMcps];
61+
// Add workflow model if present (lower priority than --model flag)
62+
if (workflowState.workflowFile.model && !options.model?.length) {
63+
options.model = [workflowState.workflowFile.model];
10264
logger.debug(
103-
`Added ${parsedTools.mcpServers.length} workflow MCP servers`,
65+
`Added workflow model: ${workflowState.workflowFile.model}`,
10466
);
10567
}
10668
}
107-
108-
return enhancedOptions;
10969
} catch (error: any) {
11070
logger.debug(`Workflow service not available: ${error.message}`);
111-
return options;
11271
}
72+
73+
// Apply rules
74+
if (options.rule && options.rule.length > 0) {
75+
enhancedConfig = await this.injectRules(enhancedConfig, options.rule);
76+
}
77+
78+
// Apply MCPs
79+
if (options.mcp && options.mcp.length > 0) {
80+
enhancedConfig = await this.injectMcps(enhancedConfig, options.mcp);
81+
}
82+
83+
// Apply models
84+
if (options.model && options.model.length > 0) {
85+
enhancedConfig = await this.injectModels(enhancedConfig, options.model);
86+
}
87+
88+
// Apply prompts
89+
if (options.prompt && options.prompt.length > 0) {
90+
enhancedConfig = await this.injectPrompts(enhancedConfig, options.prompt);
91+
}
92+
93+
return enhancedConfig;
11394
}
11495

11596
/**

‎extensions/cli/src/services/ModelService.ts‎

Lines changed: 1 addition & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,7 @@ import { createLlmApi, getLlmApi } from "../config.js";
55
import { logger } from "../util/logger.js";
66

77
import { BaseService, ServiceWithDependencies } from "./BaseService.js";
8-
import { serviceContainer } from "./ServiceContainer.js";
9-
import {
10-
ModelServiceState,
11-
SERVICE_NAMES,
12-
WorkflowServiceState,
13-
} from "./types.js";
8+
import { ModelServiceState } from "./types.js";
149

1510
/**
1611
* Service for managing LLM and model state
@@ -60,44 +55,6 @@ export class ModelService
6055
model && (model.roles?.includes("chat") || model.roles === undefined),
6156
) || []) as ModelConfig[];
6257

63-
// Check for workflow model
64-
try {
65-
const workflowState = await serviceContainer.get<WorkflowServiceState>(
66-
SERVICE_NAMES.WORKFLOW,
67-
);
68-
const workflowModel = workflowState.workflowFile?.model;
69-
70-
if (workflowModel) {
71-
logger.debug("Workflow specifies model", { workflowModel });
72-
73-
// Check if workflow model exists in available models
74-
const workflowModelConfig = this.availableModels.find(
75-
(model) => (model as any).name === workflowModel,
76-
);
77-
78-
const selectedModel =
79-
workflowModelConfig ||
80-
({
81-
provider: "openai",
82-
name: workflowModel,
83-
} as ModelConfig);
84-
85-
const llmApi = createLlmApi(selectedModel, authConfig);
86-
if (!llmApi) {
87-
throw new Error("Failed to initialize LLM with workflow model");
88-
}
89-
90-
return {
91-
llmApi,
92-
model: selectedModel,
93-
assistant,
94-
authConfig,
95-
};
96-
}
97-
} catch (error) {
98-
// Workflow service not available, continue with normal model selection
99-
}
100-
10158
// Check if we have a persisted model name and use it if valid
10259
const persistedModelName = getModelName(authConfig);
10360
if (persistedModelName) {

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -193,13 +193,14 @@ describe("Workflow Integration Tests", () => {
193193
{},
194194
);
195195

196+
// Rules should be processed normally since workflow rules are now added to options.rule
196197
expect(mockProcessRule).toHaveBeenCalledWith(mockWorkflowFile.rules);
197198
expect(enhancedConfig.rules).toHaveLength(2);
198-
expect(enhancedConfig?.rules?.[0]).toEqual({
199-
name: "workflow:owner/workflow",
200-
rule: "Processed rule content",
201-
});
202-
expect(enhancedConfig?.rules?.[1]).toBe("existing rule");
199+
// The workflow rule is processed first, then existing rules
200+
expect(mockProcessRule).toHaveBeenNthCalledWith(
201+
1,
202+
mockWorkflowFile.rules,
203+
);
203204
});
204205

205206
it("should not inject workflow rules when workflow is inactive", async () => {
@@ -254,10 +255,14 @@ describe("Workflow Integration Tests", () => {
254255
mockAuthConfig as any,
255256
);
256257

257-
// Verify that available models are filtered
258+
// Verify that available models include all original models
259+
// (we removed the filtering logic)
258260
const availableModels = modelService.getAvailableChatModels();
259-
expect(availableModels).toHaveLength(1);
260-
expect(availableModels[0].name).toBe("gpt-4-workflow");
261+
expect(availableModels).toHaveLength(2); // Should include both original models
262+
expect(availableModels.map((m) => m.name)).toEqual([
263+
"gpt-3.5-turbo",
264+
"gpt-4",
265+
]);
261266
});
262267

263268
it("should allow all models when no workflow is active", async () => {
@@ -443,9 +448,10 @@ describe("Workflow Integration Tests", () => {
443448
{},
444449
);
445450

446-
// Should not duplicate existing MCP server
447-
expect(enhancedConfig.mcpServers).toHaveLength(1);
451+
// Should not deduplicate since we simplified the logic
452+
expect(enhancedConfig.mcpServers).toHaveLength(2);
448453
expect(enhancedConfig.mcpServers?.[0]).toEqual({ name: "mcp1" });
454+
expect(enhancedConfig.mcpServers?.[1]).toEqual({ name: "mcp1" });
449455
});
450456
});
451457
});

0 commit comments

Comments
 (0)