Skip to content

Commit dcddb54

Browse files
RomneyDaRomneyDa
committed
fix: fix workflow tests
1 parent 3e80f3d commit dcddb54

File tree

6 files changed

+58
-53
lines changed

6 files changed

+58
-53
lines changed

‎extensions/cli/src/configEnhancer.test.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -237,25 +237,24 @@ describe("ConfigEnhancer", () => {
237237

238238
it("should handle workflow integration when workflow is active", async () => {
239239
// Mock service container to return active workflow
240-
const { serviceContainer } = await import("./services/ServiceContainer.js");
241-
(serviceContainer.get as any).mockResolvedValueOnce({
240+
const options: BaseCommandOptions = {
241+
rule: ["user-rule"],
242+
prompt: ["user-prompt"],
243+
};
244+
245+
const config = await enhancer.enhanceConfig(mockConfig, options, {
242246
workflowFile: {
243247
name: "Test Workflow",
244248
prompt: "You are a test assistant",
245249
rules: "Always be helpful",
246250
model: "gpt-4",
247251
tools: "bash,read",
248252
},
249-
workflow: "owner/test-workflow",
253+
slug: "owner/test-workflow",
254+
workflowModelName: null,
255+
workflowService: null,
250256
});
251257

252-
const options: BaseCommandOptions = {
253-
rule: ["user-rule"],
254-
prompt: ["user-prompt"],
255-
};
256-
257-
const config = await enhancer.enhanceConfig(mockConfig, options);
258-
259258
// Should have both workflow and user rules
260259
expect(config.rules).toHaveLength(2);
261260
expect(config.rules?.[0]).toBe("Always be helpful"); // Workflow rule first

‎extensions/cli/src/configEnhancer.ts‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ import {
1212
modelProcessor,
1313
processRule,
1414
} from "./hubLoader.js";
15-
import { serviceContainer } from "./services/ServiceContainer.js";
16-
import { SERVICE_NAMES, WorkflowServiceState } from "./services/types.js";
15+
import { WorkflowServiceState } from "./services/types.js";
1716
import { logger } from "./util/logger.js";
1817

1918
/**
@@ -26,18 +25,13 @@ export class ConfigEnhancer {
2625
async enhanceConfig(
2726
config: AssistantUnrolled,
2827
_options: BaseCommandOptions,
28+
workflowState?: WorkflowServiceState,
2929
): Promise<AssistantUnrolled> {
3030
let enhancedConfig = { ...config };
3131
const options = { ..._options };
3232

33-
// Add workflow rules/mcp servers if present
34-
const workflowState = await serviceContainer.get<WorkflowServiceState>(
35-
SERVICE_NAMES.WORKFLOW,
36-
);
37-
const { workflowFile, workflowService } = workflowState;
38-
39-
if (workflowFile) {
40-
const { rules, model, tools, prompt } = workflowFile;
33+
if (workflowState?.workflowFile) {
34+
const { rules, model, tools, prompt } = workflowState?.workflowFile;
4135
if (rules) {
4236
options.rule = [
4337
...rules.split(",").filter(Boolean),
@@ -59,7 +53,9 @@ export class ConfigEnhancer {
5953
workflowModel,
6054
...(enhancedConfig.models ?? []),
6155
];
62-
workflowService?.setWorkflowModelName(workflowModel.name);
56+
workflowState?.workflowService?.setWorkflowModelName(
57+
workflowModel.name,
58+
);
6359
}
6460

6561
// Add workflow prompt as prefix (see processAndCombinePrompts)

‎extensions/cli/src/services/ConfigService.test.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import * as configLoader from "../configLoader.js";
1212

1313
import { ConfigService } from "./ConfigService.js";
1414
import { serviceContainer } from "./ServiceContainer.js";
15-
import { SERVICE_NAMES } from "./types.js";
15+
import { SERVICE_NAMES, WorkflowServiceState } from "./types.js";
1616

1717
describe("ConfigService", () => {
1818
let service: ConfigService;
@@ -23,7 +23,12 @@ describe("ConfigService", () => {
2323
systemMessage: "Test system message",
2424
} as any;
2525
const mockApiClient = { get: vi.fn(), post: vi.fn() };
26-
26+
const mockWorkflowState: WorkflowServiceState = {
27+
slug: null,
28+
workflowFile: null,
29+
workflowModelName: null,
30+
workflowService: null,
31+
};
2732
beforeEach(() => {
2833
vi.clearAllMocks();
2934
service = new ConfigService();
@@ -41,6 +46,7 @@ describe("ConfigService", () => {
4146
"/path/to/config.yaml",
4247
"org-123",
4348
mockApiClient as any,
49+
mockWorkflowState,
4450
);
4551

4652
expect(state).toEqual({
@@ -60,6 +66,7 @@ describe("ConfigService", () => {
6066
undefined,
6167
"org-123",
6268
mockApiClient as any,
69+
mockWorkflowState,
6370
);
6471

6572
expect(state).toEqual({
@@ -87,13 +94,15 @@ describe("ConfigService", () => {
8794
"/config.yaml",
8895
"org-123",
8996
mockApiClient as any,
97+
mockWorkflowState,
9098
{ rule: ["rule1", "rule2"] },
9199
);
92100

93101
// Verify configEnhancer was called with the right parameters
94102
expect(vi.mocked(configEnhancer.enhanceConfig)).toHaveBeenCalledWith(
95103
mockConfig,
96104
{ rule: ["rule1", "rule2"] },
105+
mockWorkflowState,
97106
);
98107

99108
expect(state.config).toEqual(expectedConfig);
@@ -111,6 +120,7 @@ describe("ConfigService", () => {
111120
{ accessToken: "token" } as any,
112121
"/old.yaml",
113122
"org-123",
123+
mockWorkflowState,
114124
mockApiClient as any,
115125
);
116126

@@ -140,6 +150,7 @@ describe("ConfigService", () => {
140150
{ accessToken: "token" } as any,
141151
"/old.yaml",
142152
"org-123",
153+
mockWorkflowState,
143154
mockApiClient as any,
144155
);
145156

@@ -169,6 +180,7 @@ describe("ConfigService", () => {
169180
{ accessToken: "token" } as any,
170181
"/config.yaml",
171182
"org-123",
183+
mockWorkflowState,
172184
mockApiClient as any,
173185
);
174186

@@ -201,6 +213,7 @@ describe("ConfigService", () => {
201213
{ accessToken: "token" } as any,
202214
undefined,
203215
"org-123",
216+
mockWorkflowState,
204217
mockApiClient as any,
205218
);
206219

@@ -225,6 +238,7 @@ describe("ConfigService", () => {
225238
{ accessToken: "token" } as any,
226239
"/old.yaml",
227240
"org-123",
241+
mockWorkflowState,
228242
mockApiClient as any,
229243
);
230244

@@ -267,6 +281,7 @@ describe("ConfigService", () => {
267281
{ accessToken: "token" } as any,
268282
"/old.yaml",
269283
"org-123",
284+
mockWorkflowState,
270285
mockApiClient as any,
271286
);
272287

@@ -304,6 +319,7 @@ describe("ConfigService", () => {
304319
{ accessToken: "token" } as any,
305320
"/old.yaml",
306321
"org-123",
322+
mockWorkflowState,
307323
mockApiClient as any,
308324
);
309325

@@ -337,6 +353,7 @@ describe("ConfigService", () => {
337353
{ accessToken: "token" } as any,
338354
"/old.yaml",
339355
"org-123",
356+
mockWorkflowState,
340357
mockApiClient as any,
341358
);
342359

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
ApiClientServiceState,
1212
ConfigServiceState,
1313
SERVICE_NAMES,
14+
WorkflowServiceState,
1415
} from "./types.js";
1516

1617
/**
@@ -47,6 +48,7 @@ export class ConfigService
4748
configPath: string | undefined,
4849
_organizationId: string | null,
4950
apiClient: DefaultApiInterface,
51+
workflowState: WorkflowServiceState,
5052
injectedConfigOptions?: BaseCommandOptions,
5153
): Promise<ConfigServiceState> {
5254
// Use the new streamlined config loader
@@ -63,6 +65,7 @@ export class ConfigService
6365
config = await configEnhancer.enhanceConfig(
6466
config,
6567
injectedConfigOptions,
68+
workflowState,
6669
);
6770

6871
logger.debug("Applied injected configuration");

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,8 @@ export async function initializeServices(
214214
configPath,
215215
finalAuthState.organizationId || null,
216216
apiClientState.apiClient,
217-
commandOptions,
218217
workflowState,
218+
commandOptions,
219219
);
220220
},
221221
[SERVICE_NAMES.AUTH, SERVICE_NAMES.API_CLIENT, SERVICE_NAMES.WORKFLOW], // Dependencies

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

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import { vi } from "vitest";
33
import { ConfigEnhancer } from "../configEnhancer.js";
44

55
import { ModelService } from "./ModelService.js";
6-
import { serviceContainer } from "./ServiceContainer.js";
7-
import { SERVICE_NAMES } from "./types.js";
86
import { WorkflowService } from "./WorkflowService.js";
97

108
// Mock the hubLoader module
@@ -100,16 +98,6 @@ describe("Workflow Integration Tests", () => {
10098
modelService = new ModelService();
10199
configEnhancer = new ConfigEnhancer();
102100

103-
// Mock service container
104-
vi.spyOn(serviceContainer, "get").mockImplementation(
105-
async (serviceName: string) => {
106-
if (serviceName === SERVICE_NAMES.WORKFLOW) {
107-
return workflowService.getState();
108-
}
109-
throw new Error(`Service ${serviceName} not mocked`);
110-
},
111-
);
112-
113101
// Setup default mocks
114102
mockProcessRule.mockResolvedValue("Processed rule content");
115103
mockLoadPackagesFromHub.mockResolvedValue([{ name: "test-mcp" }]);
@@ -146,6 +134,7 @@ describe("Workflow Integration Tests", () => {
146134
const enhancedConfig = await configEnhancer.enhanceConfig(
147135
baseConfig as any,
148136
baseOptions,
137+
workflowState,
149138
);
150139

151140
// Should have loaded the workflow model directly via loadPackageFromHub
@@ -176,6 +165,7 @@ describe("Workflow Integration Tests", () => {
176165
const enhancedConfig = await configEnhancer.enhanceConfig(
177166
baseConfig as any,
178167
baseOptions,
168+
workflowState,
179169
);
180170

181171
// Should not have enhanced with any models
@@ -206,6 +196,7 @@ describe("Workflow Integration Tests", () => {
206196
const enhancedConfig = await configEnhancer.enhanceConfig(
207197
baseConfig as any,
208198
baseOptions,
199+
workflowService.getState(),
209200
);
210201

211202
// Should process the user model via loadPackagesFromHub
@@ -246,6 +237,7 @@ describe("Workflow Integration Tests", () => {
246237
const enhancedConfig = await configEnhancer.enhanceConfig(
247238
baseConfig as any,
248239
{},
240+
workflowService.getState(),
249241
);
250242

251243
// Rules should be processed normally since workflow rules are now added to options.rule
@@ -269,6 +261,7 @@ describe("Workflow Integration Tests", () => {
269261
const enhancedConfig = await configEnhancer.enhanceConfig(
270262
baseConfig as any,
271263
{},
264+
workflowService.getState(),
272265
);
273266

274267
expect(mockProcessRule).not.toHaveBeenCalled();
@@ -292,6 +285,7 @@ describe("Workflow Integration Tests", () => {
292285
const enhancedConfig = await configEnhancer.enhanceConfig(
293286
baseConfig as any,
294287
{},
288+
workflowService.getState(),
295289
);
296290

297291
expect(mockProcessRule).not.toHaveBeenCalled();
@@ -367,6 +361,7 @@ describe("Workflow Integration Tests", () => {
367361
const enhancedConfig = await configEnhancer.enhanceConfig(
368362
baseConfig as any,
369363
{},
364+
workflowService.getState(),
370365
);
371366

372367
// Should not inject workflow rule but should preserve existing rules
@@ -389,6 +384,7 @@ describe("Workflow Integration Tests", () => {
389384
const enhancedConfig = await configEnhancer.enhanceConfig(
390385
baseConfig as any,
391386
{},
387+
workflowService.getState(),
392388
);
393389

394390
// Prompts should be processed at runtime, not injected into config
@@ -452,27 +448,18 @@ describe("Workflow Integration Tests", () => {
452448
mockLoadPackageFromHub.mockResolvedValue(mockWorkflowFile);
453449
await workflowService.initialize("owner/workflow");
454450

455-
// Create a spy to verify the internal behavior of ConfigEnhancer
456-
const originalGet = serviceContainer.get;
457-
const getSpy = vi
458-
.spyOn(serviceContainer, "get")
459-
.mockImplementation(async (serviceName: string) => {
460-
if (serviceName === SERVICE_NAMES.WORKFLOW) {
461-
return workflowService.getState();
462-
}
463-
return originalGet.call(serviceContainer, serviceName);
464-
});
465-
466451
const baseOptions = { prompt: ["user-prompt"] };
467452

468453
// This should internally modify the options to include workflow prompt
469-
await configEnhancer.enhanceConfig({} as any, baseOptions);
470-
471-
// The ConfigEnhancer should have been called with workflow service
472-
expect(getSpy).toHaveBeenCalledWith(SERVICE_NAMES.WORKFLOW);
454+
await configEnhancer.enhanceConfig(
455+
{} as any,
456+
baseOptions,
457+
workflowService.getState(),
458+
);
473459

474-
// Clean up
475-
getSpy.mockRestore();
460+
// Verify that the workflow prompt was added to the options
461+
// by checking that injectPrompts was called with workflow prompt prefixed
462+
expect(baseOptions.prompt).toHaveLength(1);
476463
});
477464

478465
it("should work end-to-end with workflow prompt processing", async () => {
@@ -564,6 +551,7 @@ describe("Workflow Integration Tests", () => {
564551
const enhancedConfig = await configEnhancer.enhanceConfig(
565552
baseConfig as any,
566553
{},
554+
workflowService.getState(),
567555
);
568556

569557
expect(enhancedConfig.mcpServers).toHaveLength(3);
@@ -588,6 +576,7 @@ describe("Workflow Integration Tests", () => {
588576
const enhancedConfig = await configEnhancer.enhanceConfig(
589577
baseConfig as any,
590578
{},
579+
workflowService.getState(),
591580
);
592581

593582
expect(enhancedConfig.mcpServers).toHaveLength(1);
@@ -612,6 +601,7 @@ describe("Workflow Integration Tests", () => {
612601
const enhancedConfig = await configEnhancer.enhanceConfig(
613602
baseConfig as any,
614603
{},
604+
workflowService.getState(),
615605
);
616606

617607
// Should not deduplicate since we simplified the logic

0 commit comments

Comments
 (0)