diff --git a/.changeset/fix-model-lane-drift-visibility.md b/.changeset/fix-model-lane-drift-visibility.md new file mode 100644 index 0000000000..21b3870e87 --- /dev/null +++ b/.changeset/fix-model-lane-drift-visibility.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Warn when changing a workflow's model surfaces tasks still pinned to the old model, including default-workflow tasks. +category: fix +dev: PATCH /workflows/:id/setting-values now returns `modelDrift` for execution/planning/validator lanes. Drift baseline is captured inside the settings write transaction (no stale-read race), and default-workflow patches pass `includeNullSelection` so no-workflow-selection tasks are counted. New `TaskStore.updateWorkflowSettingValuesWithPrevious` and `getModelLaneDrift(..., { includeNullSelection })`. diff --git a/packages/core/src/__tests__/workflow-settings.test.ts b/packages/core/src/__tests__/workflow-settings.test.ts index 894eeb4872..75f2a161b0 100644 --- a/packages/core/src/__tests__/workflow-settings.test.ts +++ b/packages/core/src/__tests__/workflow-settings.test.ts @@ -308,3 +308,153 @@ 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([]); + }); + + // FN-5893: the invariant holds across ALL model lanes, not only `execution`. + it("flags the planning lane's pinned tasks when the planning model changes", async () => { + const store = harness.store(); + await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { + planningProvider: "anthropic", + planningModelId: "claude-opus-4-6", + }); + const before = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT); + const pinned = await store.createTask({ + description: "pinned to old planning model", + workflowId: BUILTIN_CODING, + planningModelProvider: "anthropic", + planningModelId: "claude-opus-4-6", + }); + await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { + planningModelId: "claude-opus-4-8", + }); + const after = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT); + + const drift = store.getModelLaneDrift(BUILTIN_CODING, before, after); + expect(drift).toHaveLength(1); + expect(drift[0].lane).toBe("planning"); + expect(drift[0].taskIds).toEqual([pinned.id]); + }); + + it("flags the validator lane's pinned tasks when the validator model changes", async () => { + const store = harness.store(); + await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { + validatorProvider: "anthropic", + validatorModelId: "claude-haiku-4-5", + }); + const before = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT); + const pinned = await store.createTask({ + description: "pinned to old validator model", + workflowId: BUILTIN_CODING, + validatorModelProvider: "anthropic", + validatorModelId: "claude-haiku-4-5", + }); + await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { + validatorModelId: "claude-haiku-5", + }); + const after = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT); + + const drift = store.getModelLaneDrift(BUILTIN_CODING, before, after); + expect(drift).toHaveLength(1); + expect(drift[0].lane).toBe("validator"); + expect(drift[0].taskIds).toEqual([pinned.id]); + }); + + // Greptile P1: when the default workflow is diffed, no-selection tasks resolve + // through it and are pinned to its lane values, so they must be counted — but + // only when the caller opts in via `includeNullSelection`. + it("includes no-workflow-selection tasks only when includeNullSelection is set", 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); + // No workflowId → no task_workflow_selection row → resolves to the default. + const nullSelected = await store.createTask({ + description: "no workflow selection, pinned to old model", + modelProvider: "anthropic", + modelId: "claude-sonnet-4-6", + }); + await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { + executionModelId: "claude-sonnet-5", + }); + const after = store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT); + + // Default excludes null-selection tasks: the route passes a concrete id. + const withoutNull = store.getModelLaneDrift(BUILTIN_CODING, before, after); + expect(withoutNull).toHaveLength(1); + expect(withoutNull[0].taskIds).not.toContain(nullSelected.id); + + // Opt in (the route does this when patching the default workflow). + const withNull = store.getModelLaneDrift(BUILTIN_CODING, before, after, { + includeNullSelection: true, + }); + expect(withNull).toHaveLength(1); + expect(withNull[0].taskIds).toContain(nullSelected.id); + }); +}); diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index efb933fc46..9dca25656b 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -8847,6 +8847,22 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} projectId: string, patch: Record, ): Promise> { + return (await this.updateWorkflowSettingValuesWithPrevious(workflowId, projectId, patch)).stored; + } + + /** + * Like {@link updateWorkflowSettingValues} but also returns the row's value + * snapshot as it was read INSIDE the same serialized transaction, immediately + * before the merge. Callers that diff before→after (e.g. model-lane drift on + * `PATCH /workflows/:id/setting-values`) must use this — reading `before` + * outside the write transaction races a concurrent patch of the same row and + * can pair a stale `previous` with another writer's `stored` (Greptile P2). + */ + async updateWorkflowSettingValuesWithPrevious( + workflowId: string, + projectId: string, + patch: Record, + ): Promise<{ previous: Record; stored: Record }> { const declarations = await this.resolveWorkflowSettingDeclarations(workflowId); const result = validateSettingValuePatch(declarations, patch); if (result.rejections.length > 0) { @@ -8861,8 +8877,8 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} // write transaction. Validation/declaration resolution above stays outside // since it's async and doesn't read the row being mutated. return this.db.transactionImmediate(() => { - const current = this.getWorkflowSettingValues(workflowId, projectId); - const next: Record = { ...current }; + const previous = this.getWorkflowSettingValues(workflowId, projectId); + const next: Record = { ...previous }; for (const [key, value] of Object.entries(result.accepted)) { if (value === null) { delete next[key]; @@ -8881,7 +8897,9 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} ) .run(workflowId, projectId, JSON.stringify(next), now); this.db.bumpLastModified(); - return next; + // `previous` is captured before the merge; it and `next` are a consistent + // before/after pair from the same transaction snapshot. + return { previous, stored: next }; }); } @@ -15855,6 +15873,87 @@ ${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. + * + * When the diffed workflow is the project default, no-selection tasks resolve + * THROUGH it and are pinned to its lane values, so they must be counted as + * occupants. Callers pass `includeNullSelection: true` in that case (the + * route never hands us the internal `DEFAULT_WORKFLOW_POOL_ID` sentinel — it + * has the concrete default id — so we cannot infer this from `workflowId` + * alone; Greptile P1). When omitted, it falls back to the sentinel check. + */ + getModelLaneDrift( + workflowId: string, + before: Record, + after: Record, + options?: { includeNullSelection?: boolean }, + ): 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 = + options?.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( diff --git a/packages/dashboard/src/routes/register-workflow-routes.ts b/packages/dashboard/src/routes/register-workflow-routes.ts index f0d50129fc..43b936203f 100644 --- a/packages/dashboard/src/routes/register-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-workflow-routes.ts @@ -481,16 +481,35 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void { await assertWorkflowExists(store, workflowId); const projectId = store.getWorkflowSettingsProjectId(); try { - const stored = await store.updateWorkflowSettingValues( + // FNXC:ModelLaneDrift 2026-07-08-07:24: + // Capture `before` INSIDE the write transaction (paired with `stored`) + // so a concurrent patch of the same row cannot pair a stale baseline + // with another writer's result (Greptile P2). + const { previous: before, stored } = await store.updateWorkflowSettingValuesWithPrevious( workflowId, projectId, values as Record, ); 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. + // FNXC:ModelLaneDrift 2026-07-08-07:24: + // When the patched workflow is the project default, no-selection tasks + // resolve through it and must be counted, so flag includeNullSelection + // then (Greptile P1) — the route holds the concrete default id, not the + // internal DEFAULT_WORKFLOW_POOL_ID sentinel getModelLaneDrift checks. + const defaultWorkflowId = (await store.getDefaultWorkflowId()) ?? "builtin:coding"; + const modelDrift = store + .getModelLaneDrift(workflowId, before, stored, { + includeNullSelection: workflowId === defaultWorkflowId, + }) + .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