fix(workflow): make core-consistency drift check trustworthy for rollout (#23807)
## Context Part of the workflow → core migration. Before enabling `IS_WORKFLOW_DISPATCH_FROM_CORE_ENABLED` per workspace, the drift signal that gates the rollout must be trustworthy. Two bugs in the (already merged) consistency cron made it lie in both directions. This PR fixes only those two; no new machinery. The actual pre-flight gate is a read-only SQL query run per batch of workspaces, so the heavier repair-command idea was dropped. ## What this does **1. Exclude soft-deleted trigger rows from the automated-trigger drift check.** `checkAutomatedTriggerSync` read the workspace `workflowAutomatedTrigger` table without a `deletedAt` filter (the sibling workflow/version checks have one). Workflow soft-delete soft-deletes the trigger row but removes the core-map entry, so every soft-deleted automated workflow emitted a permanent false `inTableNotCache` drift — inflating the exact metric meant to gate the flag. **2. Enumerate active workspaces in the consistency cron.** The scan was `SELECT DISTINCT "workspaceId" FROM core."workflow"`: a workspace whose mirror never succeeded has zero core rows and was therefore never checked — the worst-drifted tenants were invisible. It now enumerates ACTIVE workspaces and skips those with no (non-deleted) workflow rows, so cost stays close to actual workflow usage. No behavior change beyond the drift metrics themselves.
This commit is contained in:
+48
-6
@@ -19,6 +19,7 @@ describe('WorkflowCoreConsistencyService', () => {
|
||||
let metricsService: { incrementCounterBy: jest.Mock };
|
||||
let exceptionHandlerService: { captureExceptions: jest.Mock };
|
||||
|
||||
let shouldCheck: boolean;
|
||||
let workflowCounts: Counts;
|
||||
let workflowOrphan: number;
|
||||
let versionCounts: Counts;
|
||||
@@ -37,6 +38,7 @@ describe('WorkflowCoreConsistencyService', () => {
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
shouldCheck = true;
|
||||
workflowCounts = zero();
|
||||
workflowOrphan = 0;
|
||||
versionCounts = zero();
|
||||
@@ -46,11 +48,11 @@ describe('WorkflowCoreConsistencyService', () => {
|
||||
|
||||
coreDataSource = {
|
||||
query: jest.fn().mockImplementation((sql: string) => {
|
||||
if (sql.includes('DISTINCT "workspaceId"')) {
|
||||
return Promise.resolve([{ workspaceId }]);
|
||||
if (sql.includes('FROM core."workspace"')) {
|
||||
return Promise.resolve([{ workspaceId, databaseSchema: schema }]);
|
||||
}
|
||||
if (sql.includes('"databaseSchema"')) {
|
||||
return Promise.resolve([{ databaseSchema: schema }]);
|
||||
if (sql.includes('AS "shouldCheck"')) {
|
||||
return Promise.resolve([{ shouldCheck }]);
|
||||
}
|
||||
if (sql.includes('AS "orphanCore"')) {
|
||||
return Promise.resolve([
|
||||
@@ -204,10 +206,50 @@ describe('WorkflowCoreConsistencyService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('enumerates active and suspended workspaces instead of workspaces having core rows', async () => {
|
||||
await service.runConsistencyCheck();
|
||||
|
||||
expect(coreDataSource.query).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`"activationStatus" IN ('ACTIVE', 'SUSPENDED')`),
|
||||
);
|
||||
expect(coreDataSource.query).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`"databaseSchema" IS NOT NULL`),
|
||||
);
|
||||
});
|
||||
|
||||
it('gates on source workflows OR core rows so orphan core rows are still checked', async () => {
|
||||
await service.runConsistencyCheck();
|
||||
|
||||
expect(coreDataSource.query).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`OR EXISTS (SELECT 1 FROM core."workflow"`),
|
||||
[workspaceId],
|
||||
);
|
||||
});
|
||||
|
||||
it('skips workspaces that have no workflow rows', async () => {
|
||||
shouldCheck = false;
|
||||
workflowCounts = { unlinked: 5, missingCore: 0, fieldMismatch: 0 };
|
||||
|
||||
await service.runConsistencyCheck();
|
||||
|
||||
expect(metricsService.incrementCounterBy).not.toHaveBeenCalled();
|
||||
expect(workspaceCacheService.getOrRecompute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('excludes soft-deleted rows from the automated trigger comparison', async () => {
|
||||
await service.runConsistencyCheck();
|
||||
|
||||
expect(coreDataSource.query).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
`"workflowAutomatedTrigger" WHERE "deletedAt" IS NULL`,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('isolates a per-workspace failure and reports it to Sentry', async () => {
|
||||
coreDataSource.query.mockImplementation((sql: string) => {
|
||||
if (sql.includes('DISTINCT "workspaceId"')) {
|
||||
return Promise.resolve([{ workspaceId }]);
|
||||
if (sql.includes('FROM core."workspace"')) {
|
||||
return Promise.resolve([{ workspaceId, databaseSchema: schema }]);
|
||||
}
|
||||
|
||||
return Promise.reject(new Error('boom'));
|
||||
|
||||
+17
-13
@@ -32,16 +32,16 @@ export class WorkflowCoreConsistencyService {
|
||||
) {}
|
||||
|
||||
async runConsistencyCheck(): Promise<void> {
|
||||
// Only workspaces that actually use workflows; the soft-ref makes this a
|
||||
// cheap central lookup that skips the vast majority of workspaces.
|
||||
const workspaces: Array<{ workspaceId: string }> =
|
||||
const workspaces: Array<{ workspaceId: string; databaseSchema: string }> =
|
||||
await this.coreDataSource.query(
|
||||
`SELECT DISTINCT "workspaceId" FROM core."workflow"`,
|
||||
`SELECT id AS "workspaceId", "databaseSchema"
|
||||
FROM core."workspace"
|
||||
WHERE "activationStatus" IN ('ACTIVE', 'SUSPENDED') AND "databaseSchema" IS NOT NULL`,
|
||||
);
|
||||
|
||||
for (const { workspaceId } of workspaces) {
|
||||
for (const { workspaceId, databaseSchema } of workspaces) {
|
||||
try {
|
||||
await this.checkWorkspace(workspaceId);
|
||||
await this.checkWorkspace(workspaceId, databaseSchema);
|
||||
} catch (error) {
|
||||
this.exceptionHandlerService.captureExceptions([error], {
|
||||
workspace: { id: workspaceId },
|
||||
@@ -50,18 +50,22 @@ export class WorkflowCoreConsistencyService {
|
||||
}
|
||||
}
|
||||
|
||||
private async checkWorkspace(workspaceId: string): Promise<void> {
|
||||
const [workspace] = await this.coreDataSource.query(
|
||||
`SELECT "databaseSchema" FROM core."workspace" WHERE id = $1`,
|
||||
private async checkWorkspace(
|
||||
workspaceId: string,
|
||||
schema: string,
|
||||
): Promise<void> {
|
||||
const [{ shouldCheck }] = await this.coreDataSource.query(
|
||||
`SELECT (
|
||||
EXISTS (SELECT 1 FROM "${schema}"."workflow" WHERE "deletedAt" IS NULL)
|
||||
OR EXISTS (SELECT 1 FROM core."workflow" WHERE "workspaceId" = $1)
|
||||
) AS "shouldCheck"`,
|
||||
[workspaceId],
|
||||
);
|
||||
|
||||
if (!isDefined(workspace?.databaseSchema)) {
|
||||
if (!shouldCheck) {
|
||||
return;
|
||||
}
|
||||
|
||||
const schema: string = workspace.databaseSchema;
|
||||
|
||||
await this.checkWorkflowSync(workspaceId, schema);
|
||||
await this.checkWorkflowVersionSync(workspaceId, schema);
|
||||
await this.checkAutomatedTriggerSync(workspaceId, schema);
|
||||
@@ -170,7 +174,7 @@ export class WorkflowCoreConsistencyService {
|
||||
type: AutomatedTriggerType;
|
||||
settings: BaseDatabaseEventTriggerSettings | CronTriggerSettings | null;
|
||||
}> = await this.coreDataSource.query(
|
||||
`SELECT "workflowId", type, settings FROM "${schema}"."workflowAutomatedTrigger"`,
|
||||
`SELECT "workflowId", type, settings FROM "${schema}"."workflowAutomatedTrigger" WHERE "deletedAt" IS NULL`,
|
||||
);
|
||||
|
||||
const tableByWorkflowId = new Map(
|
||||
|
||||
Reference in New Issue
Block a user