feat(FN-834): add per-workflow-step model overrides for execution

- Extend WorkflowStep type with optional modelProvider, modelId, validatorModelProvider, validatorModelId fields
- Add API endpoints (PATCH /api/workflow-steps/:id) to update model overrides on workflow steps
- Update executor to use per-step model overrides when running workflow step agents
- Add AgentDetailView support for displaying workflow step model configuration
- Add store and executor tests for new model override behavior
This commit is contained in:
gsxdsm
2026-04-04 04:08:16 -07:00
parent 757bca3f3c
commit 4e0077f48a
8 changed files with 406 additions and 9 deletions

View File

@@ -125,7 +125,9 @@ What the task should accomplish.
"id": "WS-001", "id": "WS-001",
"name": "Documentation Review", "name": "Documentation Review",
"prompt": "Review the task changes...", "prompt": "Review the task changes...",
"enabled": true "enabled": true,
"modelProvider": "anthropic",
"modelId": "claude-sonnet-4-5"
} }
] ]
} }

View File

@@ -4450,6 +4450,81 @@ Task with acceptance criteria
expect(task.enabledWorkflowSteps).toBeUndefined(); expect(task.enabledWorkflowSteps).toBeUndefined();
}); });
it("should create a workflow step with model override", async () => {
const ws = await store.createWorkflowStep({
name: "Security Audit",
description: "Check for security issues",
prompt: "Scan for vulnerabilities.",
enabled: true,
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
});
expect(ws.modelProvider).toBe("anthropic");
expect(ws.modelId).toBe("claude-sonnet-4-5");
});
it("should create a workflow step without model override", async () => {
const ws = await store.createWorkflowStep({
name: "QA Check",
description: "Run tests",
});
expect(ws.modelProvider).toBeUndefined();
expect(ws.modelId).toBeUndefined();
});
it("should update a workflow step model override", async () => {
const ws = await store.createWorkflowStep({
name: "Docs",
description: "Check docs",
});
const updated = await store.updateWorkflowStep(ws.id, {
modelProvider: "openai",
modelId: "gpt-4o",
});
expect(updated.modelProvider).toBe("openai");
expect(updated.modelId).toBe("gpt-4o");
});
it("should clear a workflow step model override by setting to undefined", async () => {
const ws = await store.createWorkflowStep({
name: "Docs",
description: "Check docs",
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
});
expect(ws.modelProvider).toBe("anthropic");
const updated = await store.updateWorkflowStep(ws.id, {
modelProvider: undefined,
modelId: undefined,
});
expect(updated.modelProvider).toBeUndefined();
expect(updated.modelId).toBeUndefined();
});
it("should persist model override across list/get", async () => {
const ws = await store.createWorkflowStep({
name: "Perf Review",
description: "Check performance",
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
});
const listed = await store.listWorkflowSteps();
expect(listed[0].modelProvider).toBe("anthropic");
expect(listed[0].modelId).toBe("claude-sonnet-4-5");
const found = await store.getWorkflowStep(ws.id);
expect(found!.modelProvider).toBe("anthropic");
expect(found!.modelId).toBe("claude-sonnet-4-5");
});
}); });
// ── Title Summarization Tests ──────────────────────────────────────────── // ── Title Summarization Tests ────────────────────────────────────────────

View File

@@ -2529,6 +2529,8 @@ ${stepsSection}`;
description: input.description, description: input.description,
prompt: input.prompt || "", prompt: input.prompt || "",
enabled: input.enabled !== undefined ? input.enabled : true, enabled: input.enabled !== undefined ? input.enabled : true,
modelProvider: input.modelProvider,
modelId: input.modelId,
createdAt: now, createdAt: now,
updatedAt: now, updatedAt: now,
}; };
@@ -2579,6 +2581,8 @@ ${stepsSection}`;
if (updates.description !== undefined) step.description = updates.description; if (updates.description !== undefined) step.description = updates.description;
if (updates.prompt !== undefined) step.prompt = updates.prompt; if (updates.prompt !== undefined) step.prompt = updates.prompt;
if (updates.enabled !== undefined) step.enabled = updates.enabled; if (updates.enabled !== undefined) step.enabled = updates.enabled;
if ("modelProvider" in updates) step.modelProvider = updates.modelProvider;
if ("modelId" in updates) step.modelId = updates.modelId;
step.updatedAt = new Date().toISOString(); step.updatedAt = new Date().toISOString();
config.workflowSteps = steps; config.workflowSteps = steps;

View File

@@ -55,6 +55,14 @@ export interface WorkflowStep {
prompt: string; prompt: string;
/** Whether this step is available for selection on new tasks */ /** Whether this step is available for selection on new tasks */
enabled: boolean; enabled: boolean;
/** AI model provider override for the workflow step agent (e.g., "anthropic").
* Must be set together with `modelId`. When both model fields are undefined,
* the executor uses global settings defaults. */
modelProvider?: string;
/** AI model ID override for the workflow step agent (e.g., "claude-sonnet-4-5").
* Must be set together with `modelProvider`. When both model fields are undefined,
* the executor uses global settings defaults. */
modelId?: string;
/** ISO-8601 timestamp of creation */ /** ISO-8601 timestamp of creation */
createdAt: string; createdAt: string;
/** ISO-8601 timestamp of last update */ /** ISO-8601 timestamp of last update */
@@ -72,6 +80,10 @@ export interface WorkflowStepInput {
prompt?: string; prompt?: string;
/** Defaults to true if not specified */ /** Defaults to true if not specified */
enabled?: boolean; enabled?: boolean;
/** AI model provider override. Must be set together with modelId. */
modelProvider?: string;
/** AI model ID override. Must be set together with modelProvider. */
modelId?: string;
} }
/** Result of a workflow step execution on a task. */ /** Result of a workflow step execution on a task. */

View File

@@ -6295,6 +6295,72 @@ describe("POST /workflow-steps", () => {
expect(res.status).toBe(409); expect(res.status).toBe(409);
expect(res.body.error).toContain("already exists"); expect(res.body.error).toContain("already exists");
}); });
it("creates a workflow step with model override", async () => {
const created = { id: "WS-002", name: "Security", description: "Security audit", prompt: "", enabled: true, modelProvider: "anthropic", modelId: "claude-sonnet-4-5", createdAt: "2026-01-01", updatedAt: "2026-01-01" };
(store.createWorkflowStep as ReturnType<typeof vi.fn>).mockResolvedValueOnce(created);
const res = await REQUEST(buildApp(), "POST", "/api/workflow-steps", JSON.stringify({
name: "Security",
description: "Security audit",
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
}), { "Content-Type": "application/json" });
expect(res.status).toBe(201);
expect(store.createWorkflowStep).toHaveBeenCalledWith({
name: "Security",
description: "Security audit",
prompt: undefined,
enabled: undefined,
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
});
});
it("returns 400 when model provider is set without modelId", async () => {
const res = await REQUEST(buildApp(), "POST", "/api/workflow-steps", JSON.stringify({
name: "Security",
description: "Security audit",
modelProvider: "anthropic",
}), { "Content-Type": "application/json" });
expect(res.status).toBe(400);
expect(res.body.error).toContain("must include both provider and modelId");
});
it("returns 400 when modelId is set without model provider", async () => {
const res = await REQUEST(buildApp(), "POST", "/api/workflow-steps", JSON.stringify({
name: "Security",
description: "Security audit",
modelId: "claude-sonnet-4-5",
}), { "Content-Type": "application/json" });
expect(res.status).toBe(400);
expect(res.body.error).toContain("must include both provider and modelId");
});
it("creates a workflow step without model fields when both empty strings", async () => {
const created = { id: "WS-001", name: "Docs", description: "Check docs", prompt: "", enabled: true, createdAt: "2026-01-01", updatedAt: "2026-01-01" };
(store.createWorkflowStep as ReturnType<typeof vi.fn>).mockResolvedValueOnce(created);
const res = await REQUEST(buildApp(), "POST", "/api/workflow-steps", JSON.stringify({
name: "Docs",
description: "Check docs",
modelProvider: "",
modelId: "",
}), { "Content-Type": "application/json" });
expect(res.status).toBe(201);
expect(store.createWorkflowStep).toHaveBeenCalledWith({
name: "Docs",
description: "Check docs",
prompt: undefined,
enabled: undefined,
modelProvider: undefined,
modelId: undefined,
});
});
}); });
describe("PATCH /workflow-steps/:id", () => { describe("PATCH /workflow-steps/:id", () => {
@@ -6334,6 +6400,40 @@ describe("PATCH /workflow-steps/:id", () => {
expect(res.status).toBe(404); expect(res.status).toBe(404);
expect(res.body.error).toContain("not found"); expect(res.body.error).toContain("not found");
}); });
it("updates a workflow step with model override", async () => {
const updated = { id: "WS-001", name: "Security", description: "Audit", prompt: "", enabled: true, modelProvider: "anthropic", modelId: "claude-sonnet-4-5", createdAt: "2026-01-01", updatedAt: "2026-01-02" };
(store.updateWorkflowStep as ReturnType<typeof vi.fn>).mockResolvedValueOnce(updated);
const res = await REQUEST(buildApp(), "PATCH", "/api/workflow-steps/WS-001", JSON.stringify({
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
}), { "Content-Type": "application/json" });
expect(res.status).toBe(200);
expect(store.updateWorkflowStep).toHaveBeenCalledWith("WS-001", expect.objectContaining({
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
}));
});
it("returns 400 when updating with only modelProvider", async () => {
const res = await REQUEST(buildApp(), "PATCH", "/api/workflow-steps/WS-001", JSON.stringify({
modelProvider: "anthropic",
}), { "Content-Type": "application/json" });
expect(res.status).toBe(400);
expect(res.body.error).toContain("must include both provider and modelId");
});
it("returns 400 when updating with only modelId", async () => {
const res = await REQUEST(buildApp(), "PATCH", "/api/workflow-steps/WS-001", JSON.stringify({
modelId: "claude-sonnet-4-5",
}), { "Content-Type": "application/json" });
expect(res.status).toBe(400);
expect(res.body.error).toContain("must include both provider and modelId");
});
}); });
describe("DELETE /workflow-steps/:id", () => { describe("DELETE /workflow-steps/:id", () => {

View File

@@ -5810,7 +5810,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
router.post("/workflow-steps", async (req, res) => { router.post("/workflow-steps", async (req, res) => {
try { try {
const scopedStore = await getScopedStore(req); const scopedStore = await getScopedStore(req);
const { name, description, prompt, enabled } = req.body; const { name, description, prompt, enabled, modelProvider, modelId } = req.body;
if (!name || typeof name !== "string" || !name.trim()) { if (!name || typeof name !== "string" || !name.trim()) {
res.status(400).json({ error: "name is required" }); res.status(400).json({ error: "name is required" });
@@ -5829,6 +5829,9 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
return; return;
} }
// Validate model override pair
const modelPair = assertConsistentOptionalPair(modelProvider, modelId, "workflow step model");
// Check for name conflicts // Check for name conflicts
const existing = await scopedStore.listWorkflowSteps(); const existing = await scopedStore.listWorkflowSteps();
if (existing.some((ws) => ws.name.toLowerCase() === name.trim().toLowerCase())) { if (existing.some((ws) => ws.name.toLowerCase() === name.trim().toLowerCase())) {
@@ -5841,10 +5844,13 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
description: description.trim(), description: description.trim(),
prompt: prompt?.trim(), prompt: prompt?.trim(),
enabled, enabled,
modelProvider: modelPair.provider,
modelId: modelPair.modelId,
}); });
res.status(201).json(step); res.status(201).json(step);
} catch (err: any) { } catch (err: any) {
res.status(500).json({ error: err.message }); const status = typeof err?.message === "string" && err.message.includes("must include both provider and modelId") ? 400 : 500;
res.status(status).json({ error: err.message });
} }
}); });
@@ -5857,7 +5863,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
router.patch("/workflow-steps/:id", async (req, res) => { router.patch("/workflow-steps/:id", async (req, res) => {
try { try {
const scopedStore = await getScopedStore(req); const scopedStore = await getScopedStore(req);
const { name, description, prompt, enabled } = req.body; const { name, description, prompt, enabled, modelProvider, modelId } = req.body;
const updates: Record<string, unknown> = {}; const updates: Record<string, unknown> = {};
if (name !== undefined) { if (name !== undefined) {
@@ -5889,13 +5895,21 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
updates.enabled = enabled; updates.enabled = enabled;
} }
// Validate and apply model override pair
if (modelProvider !== undefined || modelId !== undefined) {
const modelPair = assertConsistentOptionalPair(modelProvider, modelId, "workflow step model");
updates.modelProvider = modelPair.provider;
updates.modelId = modelPair.modelId;
}
const step = await scopedStore.updateWorkflowStep(req.params.id, updates); const step = await scopedStore.updateWorkflowStep(req.params.id, updates);
res.json(step); res.json(step);
} catch (err: any) { } catch (err: any) {
if (err.message?.includes("not found")) { if (err.message?.includes("not found")) {
res.status(404).json({ error: err.message }); res.status(404).json({ error: err.message });
} else { } else {
res.status(500).json({ error: err.message }); const status = typeof err?.message === "string" && err.message.includes("must include both provider and modelId") ? 400 : 500;
res.status(status).json({ error: err.message });
} }
} }
}); });

View File

@@ -4920,6 +4920,191 @@ describe("Workflow Steps Execution", () => {
// Task should still move to in-review // Task should still move to in-review
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review"); expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review");
}); });
it("uses workflow step model override when both provider and modelId are set", async () => {
const store = createMockStore();
store.getTask.mockResolvedValue({
id: "FN-001",
title: "Test",
description: "Test task",
column: "in-progress",
dependencies: [],
steps: [{ name: "Preflight", status: "pending" }],
currentStep: 0,
log: [],
enabledWorkflowSteps: ["WS-001"],
prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check",
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
store.getWorkflowStep.mockResolvedValue({
id: "WS-001",
name: "Security Audit",
description: "Check security",
prompt: "Scan for vulnerabilities.",
enabled: true,
modelProvider: "anthropic",
modelId: "claude-sonnet-4-5",
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
let callIdx = 0;
mockedCreateHaiAgent.mockImplementation((async (opts: any) => {
callIdx++;
if (callIdx === 1) {
// Main execution agent
const customTools = opts.customTools || [];
const session = {
prompt: vi.fn().mockImplementation(async () => {
const taskDoneTool = customTools.find((t: any) => t.name === "task_done");
if (taskDoneTool) await taskDoneTool.execute("tool-1", {});
}),
dispose: vi.fn(),
subscribe: vi.fn(),
on: vi.fn(),
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
state: {},
};
return { session };
} else {
// Workflow step agent
return {
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
subscribe: vi.fn(),
on: vi.fn(),
state: {},
},
};
}
}) as any);
const onComplete = vi.fn();
const executor = new TaskExecutor(store, "/tmp/test", { onComplete });
await executor.execute({
id: "FN-001",
title: "Test",
description: "Test task",
column: "in-progress",
dependencies: [],
steps: [{ name: "Preflight", status: "pending" }],
currentStep: 0,
log: [],
enabledWorkflowSteps: ["WS-001"],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
// createKbAgent called twice: main agent + workflow step agent
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(2);
// Second call should use the workflow step's model override
const secondCall = mockedCreateHaiAgent.mock.calls[1];
expect(secondCall[0].defaultProvider).toBe("anthropic");
expect(secondCall[0].defaultModelId).toBe("claude-sonnet-4-5");
// Log should indicate the override
expect(store.logEntry).toHaveBeenCalledWith(
"FN-001",
expect.stringContaining("workflow step override"),
);
});
it("uses global defaults when workflow step has no model override", async () => {
const store = createMockStore();
store.getTask.mockResolvedValue({
id: "FN-001",
title: "Test",
description: "Test task",
column: "in-progress",
dependencies: [],
steps: [{ name: "Preflight", status: "pending" }],
currentStep: 0,
log: [],
enabledWorkflowSteps: ["WS-001"],
prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check",
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
// Workflow step without model override
store.getWorkflowStep.mockResolvedValue({
id: "WS-001",
name: "Docs Review",
description: "Check documentation",
prompt: "Review all docs.",
enabled: true,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
let callIdx = 0;
mockedCreateHaiAgent.mockImplementation((async (opts: any) => {
callIdx++;
if (callIdx === 1) {
const customTools = opts.customTools || [];
const session = {
prompt: vi.fn().mockImplementation(async () => {
const taskDoneTool = customTools.find((t: any) => t.name === "task_done");
if (taskDoneTool) await taskDoneTool.execute("tool-1", {});
}),
dispose: vi.fn(),
subscribe: vi.fn(),
on: vi.fn(),
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
state: {},
};
return { session };
} else {
return {
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
subscribe: vi.fn(),
on: vi.fn(),
state: {},
},
};
}
}) as any);
const onComplete = vi.fn();
const executor = new TaskExecutor(store, "/tmp/test", { onComplete });
await executor.execute({
id: "FN-001",
title: "Test",
description: "Test task",
column: "in-progress",
dependencies: [],
steps: [{ name: "Preflight", status: "pending" }],
currentStep: 0,
log: [],
enabledWorkflowSteps: ["WS-001"],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(2);
// Second call should use settings defaults (no override indicator)
const secondCall = mockedCreateHaiAgent.mock.calls[1];
// defaults come from the mock store's getSettings
expect(secondCall[0].defaultProvider).toBeUndefined();
expect(secondCall[0].defaultModelId).toBeUndefined();
// Log should NOT indicate override
expect(store.logEntry).not.toHaveBeenCalledWith(
"FN-001",
expect.stringContaining("workflow step override"),
);
});
}); });
describe("Real-time steering injection", () => { describe("Real-time steering injection", () => {

View File

@@ -1424,19 +1424,24 @@ If issues are found that need attention, describe them clearly.`;
}); });
try { try {
// Determine model: prefer workflow step override, fall back to global settings
const stepProvider = workflowStep.modelProvider || settings.defaultProvider;
const stepModelId = workflowStep.modelId || settings.defaultModelId;
const useOverride = !!(workflowStep.modelProvider && workflowStep.modelId);
const { session } = await createKbAgent({ const { session } = await createKbAgent({
cwd: worktreePath, cwd: worktreePath,
systemPrompt, systemPrompt,
tools: "readonly", tools: "readonly",
defaultProvider: settings.defaultProvider, defaultProvider: stepProvider,
defaultModelId: settings.defaultModelId, defaultModelId: stepModelId,
fallbackProvider: settings.fallbackProvider, fallbackProvider: settings.fallbackProvider,
fallbackModelId: settings.fallbackModelId, fallbackModelId: settings.fallbackModelId,
defaultThinkingLevel: settings.defaultThinkingLevel, defaultThinkingLevel: settings.defaultThinkingLevel,
}); });
executorLog.log(`${task.id}: workflow step '${workflowStep.name}' using model ${describeModel(session)}`); executorLog.log(`${task.id}: workflow step '${workflowStep.name}' using model ${describeModel(session)}${useOverride ? " (workflow step override)" : ""}`);
await this.store.logEntry(task.id, `Workflow step '${workflowStep.name}' using model: ${describeModel(session)}`); await this.store.logEntry(task.id, `Workflow step '${workflowStep.name}' using model: ${describeModel(session)}${useOverride ? " (workflow step override)" : ""}`);
let output = ""; let output = "";
session.subscribe((event) => { session.subscribe((event) => {