fix: surface model-lane drift when a workflow's default model changes
Task model fields (execution/planning/validator provider+modelId) are snapshotted once at task-creation time from the workflow's model-lane default and never re-synced. Changing a workflow's default (e.g. fixing a stale model id) silently leaves already-created tasks pinned to the old value with no visibility — this is exactly how 52 tasks stayed pinned to a stale claude-sonnet-4-6 default after it was corrected. Add TaskStore.getModelLaneDrift(workflowId, before, after), a read-only diff over the three model lanes that lists non-terminal tasks still pinned to a lane's old value. Wire it into PATCH /workflows/:id/setting-values so the response includes `modelDrift` whenever a lane change orphans existing tasks. Operators can then act via the existing POST /tasks/batch-update-models.
This commit is contained in:
@@ -308,3 +308,70 @@ describe("TaskStore.updateWorkflowSettingValues", () => {
|
||||
expect(effective.workflowStepTimeoutMs).toBe(900_000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskStore.getModelLaneDrift", () => {
|
||||
const harness = createSharedTaskStoreTestHarness();
|
||||
|
||||
beforeAll(harness.beforeAll);
|
||||
afterAll(harness.afterAll);
|
||||
beforeEach(harness.beforeEach);
|
||||
afterEach(harness.afterEach);
|
||||
|
||||
it("flags non-terminal tasks still pinned to a lane's old value, and excludes done/unrelated tasks", async () => {
|
||||
const store = harness.store();
|
||||
|
||||
await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, {
|
||||
executionProvider: "anthropic",
|
||||
executionModelId: "claude-sonnet-4-6",
|
||||
});
|
||||
const before = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT);
|
||||
|
||||
const pinned = await store.createTask({
|
||||
description: "pinned to old model",
|
||||
workflowId: BUILTIN_CODING,
|
||||
modelProvider: "anthropic",
|
||||
modelId: "claude-sonnet-4-6",
|
||||
});
|
||||
const alreadyCurrent = await store.createTask({
|
||||
description: "already on the new model",
|
||||
workflowId: BUILTIN_CODING,
|
||||
modelProvider: "anthropic",
|
||||
modelId: "claude-sonnet-5",
|
||||
});
|
||||
const doneTask = await store.createTask({
|
||||
description: "terminal task, excluded even though pinned to the old model",
|
||||
workflowId: BUILTIN_CODING,
|
||||
modelProvider: "anthropic",
|
||||
modelId: "claude-sonnet-4-6",
|
||||
column: "done",
|
||||
});
|
||||
|
||||
await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, {
|
||||
executionModelId: "claude-sonnet-5",
|
||||
});
|
||||
const after = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT);
|
||||
|
||||
const drift = store.getModelLaneDrift(BUILTIN_CODING, before, after);
|
||||
expect(drift).toHaveLength(1);
|
||||
const execution = drift[0];
|
||||
expect(execution.lane).toBe("execution");
|
||||
expect(execution.from).toEqual({ provider: "anthropic", modelId: "claude-sonnet-4-6" });
|
||||
expect(execution.to).toEqual({ provider: "anthropic", modelId: "claude-sonnet-5" });
|
||||
expect(execution.taskIds).toEqual([pinned.id]);
|
||||
expect(execution.taskIds).not.toContain(alreadyCurrent.id);
|
||||
expect(execution.taskIds).not.toContain(doneTask.id);
|
||||
});
|
||||
|
||||
it("reports no drift when the lane value is unchanged", async () => {
|
||||
const store = harness.store();
|
||||
await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, {
|
||||
executionProvider: "anthropic",
|
||||
executionModelId: "claude-sonnet-5",
|
||||
});
|
||||
const before = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT);
|
||||
await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { requirePrApproval: true });
|
||||
const after = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT);
|
||||
|
||||
expect(store.getModelLaneDrift(BUILTIN_CODING, before, after)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15806,6 +15806,78 @@ ${stepsSection}`;
|
||||
return ids;
|
||||
}
|
||||
|
||||
/** Model-lane setting keys (workflow_settings) paired with the task columns
|
||||
* they seed at task-creation time (a permanent point-in-time snapshot,
|
||||
* never re-synced when the workflow default later changes). */
|
||||
private static readonly MODEL_LANES = [
|
||||
{ lane: "execution", providerKey: "executionProvider", modelIdKey: "executionModelId", providerCol: "modelProvider", modelIdCol: "modelId" },
|
||||
{ lane: "planning", providerKey: "planningProvider", modelIdKey: "planningModelId", providerCol: "planningModelProvider", modelIdCol: "planningModelId" },
|
||||
{ lane: "validator", providerKey: "validatorProvider", modelIdKey: "validatorModelId", providerCol: "validatorModelProvider", modelIdCol: "validatorModelId" },
|
||||
] as const;
|
||||
|
||||
/**
|
||||
* Diff `before`/`after` workflow setting values for the three model lanes
|
||||
* and, for each lane whose provider+modelId pair changed, list the
|
||||
* non-terminal tasks on this workflow still pinned to the OLD pair. A
|
||||
* task's model columns are captured once at creation and never re-synced,
|
||||
* so changing a workflow's default silently orphans already-created tasks
|
||||
* unless this drift is surfaced. Read-only — never mutates `tasks`; pair
|
||||
* with `POST /tasks/batch-update-models` to actually re-pin flagged ids.
|
||||
*/
|
||||
getModelLaneDrift(
|
||||
workflowId: string,
|
||||
before: Record<string, unknown>,
|
||||
after: Record<string, unknown>,
|
||||
): Array<{
|
||||
lane: string;
|
||||
from: { provider: string | null; modelId: string | null };
|
||||
to: { provider: string | null; modelId: string | null };
|
||||
taskIds: string[];
|
||||
}> {
|
||||
const asStringOrNull = (v: unknown): string | null => (typeof v === "string" ? v : null);
|
||||
const includeNullSelection = workflowId === TaskStore.DEFAULT_WORKFLOW_POOL_ID;
|
||||
const drift: Array<{
|
||||
lane: string;
|
||||
from: { provider: string | null; modelId: string | null };
|
||||
to: { provider: string | null; modelId: string | null };
|
||||
taskIds: string[];
|
||||
}> = [];
|
||||
|
||||
for (const l of TaskStore.MODEL_LANES) {
|
||||
const fromProvider = asStringOrNull(before[l.providerKey]);
|
||||
const fromModelId = asStringOrNull(before[l.modelIdKey]);
|
||||
const toProvider = asStringOrNull(after[l.providerKey]);
|
||||
const toModelId = asStringOrNull(after[l.modelIdKey]);
|
||||
if (fromProvider === toProvider && fromModelId === toModelId) continue;
|
||||
if (fromProvider === null && fromModelId === null) continue;
|
||||
|
||||
const occupants = this.listWorkflowOccupantTaskIds(workflowId, includeNullSelection);
|
||||
let taskIds: string[] = [];
|
||||
if (occupants.length > 0) {
|
||||
const placeholders = occupants.map(() => "?").join(",");
|
||||
const rows = this.db
|
||||
.prepare(
|
||||
`SELECT id FROM tasks
|
||||
WHERE id IN (${placeholders})
|
||||
AND "deletedAt" IS NULL
|
||||
AND "column" != 'archived'
|
||||
AND "column" != 'done'
|
||||
AND "${l.providerCol}" IS ?
|
||||
AND "${l.modelIdCol}" IS ?`,
|
||||
)
|
||||
.all(...occupants, fromProvider, fromModelId) as Array<{ id: string }>;
|
||||
taskIds = rows.map((r) => r.id);
|
||||
}
|
||||
drift.push({
|
||||
lane: l.lane,
|
||||
from: { provider: fromProvider, modelId: fromModelId },
|
||||
to: { provider: toProvider, modelId: toModelId },
|
||||
taskIds,
|
||||
});
|
||||
}
|
||||
return drift;
|
||||
}
|
||||
|
||||
/** Map column id → occupant count for the tasks selecting `workflowId`
|
||||
* (plus null-selection tasks when `includeNullSelection`). */
|
||||
private occupantsByColumnForWorkflow(
|
||||
|
||||
@@ -481,16 +481,22 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void {
|
||||
await assertWorkflowExists(store, workflowId);
|
||||
const projectId = store.getWorkflowSettingsProjectId();
|
||||
try {
|
||||
const before = store.getWorkflowSettingValues(workflowId, projectId);
|
||||
const stored = await store.updateWorkflowSettingValues(
|
||||
workflowId,
|
||||
projectId,
|
||||
values as Record<string, unknown>,
|
||||
);
|
||||
const declarations = await resolveSettingDeclarations(store, workflowId);
|
||||
// Model-lane drift: tasks already pinned to the value being replaced
|
||||
// are never auto-resynced (see getModelLaneDrift), so surface them
|
||||
// here rather than let the change silently orphan those tasks.
|
||||
const modelDrift = store.getModelLaneDrift(workflowId, before, stored).filter((d) => d.taskIds.length > 0);
|
||||
res.json({
|
||||
stored,
|
||||
effective: resolveEffectiveSettingValues(declarations, stored),
|
||||
orphaned: findOrphanedSettingValues(declarations, stored),
|
||||
...(modelDrift.length > 0 ? { modelDrift } : {}),
|
||||
});
|
||||
} catch (writeErr: unknown) {
|
||||
// Typed rejection → 400 with the structured rejections so the client can
|
||||
|
||||
Reference in New Issue
Block a user