0baa333809
## Why Fast instance commands run in the ArgoCD **PreSync** hook, before the new pods roll. A bulk `UPDATE`/`INSERT`/`DELETE` held in the **same transaction** as an `ADD COLUMN`/`ALTER` keeps an `ACCESS EXCLUSIVE` lock on the table for the whole write, blocking every read of it. That is what froze prod during the 2.13 `isUIReadOnly → isUIEditable` rename — a bulk `UPDATE "fieldMetadata"` inside the same `up()` transaction as the `ADD COLUMN`s → read timeouts → failed PreSync → aborted sync. @charlesBochet already caught this exact pattern by hand on #21527 ("data migration => make a slow instance command :)"). This turns that manual review into something CI enforces. ## What New oxlint rule **`twenty/no-data-mutation-in-fast-instance-command`**: - Flags statement-leading `UPDATE`/`INSERT`/`DELETE`/`MERGE` passed to `.query(...)` **inside `up()`** of a `*-instance-command-fast-*` file. - Allows: schema DDL (`ALTER`/`CREATE`/`DROP`); `ON DELETE CASCADE` / a column named `updatedAt` (not statement-leading, so never matched); rollback DML in `down()`; and data migrations in **slow** commands' `runDataMigration()`. - The error message points the author straight at the slow-command pattern. Enabled as `error` in `twenty-server`. ## Grandfathering Scoping to `up()` means **only one** existing file violates the rule: the already-shipped 2.13 rename command. It's recorded complete in cloud and must not be rewritten, so it's grandfathered with a documented file-level `oxlint-disable` (the comment makes clear it's an exception, not a precedent). The four other fast commands that contain DML keep theirs in `down()` and are correctly unaffected. ## Tests - 9 RuleTester cases — valid: DDL, FK cascade, `updatedAt`, `down()` DML, slow-command DML, non-upgrade files; invalid: `UPDATE`/`INSERT`/`DELETE` in `up()`. - Verified end-to-end with oxlint: a throwaway violating file → 1 error; all 141 upgrade-command files → 0 errors; full oxlint-rules suite 225/225; typecheck clean. Part of the v2.13 deploy post-mortem follow-ups. https://claude.ai/code/session_013Az1etaGyxWRRVhgjhPWeB --- _Generated by [Claude Code](https://claude.ai/code/session_013Az1etaGyxWRRVhgjhPWeB)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21547?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Claude <noreply@anthropic.com>
144 lines
3.6 KiB
TypeScript
144 lines
3.6 KiB
TypeScript
import { defineRule } from '@oxlint/plugins';
|
|
|
|
export const RULE_NAME = 'no-data-mutation-in-fast-instance-command';
|
|
|
|
const UPGRADE_COMMAND_MARKER = 'upgrade-version-command/';
|
|
const FAST_INSTANCE_COMMAND_SEGMENT = 'instance-command-fast-';
|
|
const SKIPPED_FILE_REGEX = /\.(spec|test)\.ts$/;
|
|
|
|
const DATA_MUTATION_STATEMENT_REGEX = /^(UPDATE|INSERT|DELETE|MERGE)\b/i;
|
|
const CTE_DATA_MUTATION_REGEX =
|
|
/^WITH\b[\s\S]*[()]\s*(UPDATE|INSERT|DELETE|MERGE)\b/i;
|
|
|
|
const isFastInstanceCommandFile = (filename: string): boolean => {
|
|
const markerIndex = filename.indexOf(UPGRADE_COMMAND_MARKER);
|
|
|
|
if (markerIndex === -1) {
|
|
return false;
|
|
}
|
|
|
|
const basename =
|
|
filename
|
|
.slice(markerIndex + UPGRADE_COMMAND_MARKER.length)
|
|
.split('/')
|
|
.pop() ?? '';
|
|
|
|
if (SKIPPED_FILE_REGEX.test(basename)) {
|
|
return false;
|
|
}
|
|
|
|
return basename.includes(FAST_INSTANCE_COMMAND_SEGMENT);
|
|
};
|
|
|
|
const isInsideDownMethod = (node: any): boolean => {
|
|
let current = node.parent;
|
|
|
|
while (current) {
|
|
if (
|
|
(current.type === 'MethodDefinition' ||
|
|
current.type === 'PropertyDefinition') &&
|
|
current.key?.type === 'Identifier' &&
|
|
current.key.name === 'down'
|
|
) {
|
|
return true;
|
|
}
|
|
|
|
current = current.parent;
|
|
}
|
|
|
|
return false;
|
|
};
|
|
|
|
const getSqlText = (argument: any): string | null => {
|
|
if (argument.type === 'Literal' && typeof argument.value === 'string') {
|
|
return argument.value;
|
|
}
|
|
|
|
if (argument.type === 'TemplateLiteral') {
|
|
return argument.quasis.map((quasi: any) => quasi.value.raw).join(' ');
|
|
}
|
|
|
|
return null;
|
|
};
|
|
|
|
const findDataMutationKeyword = (sql: string): string | null => {
|
|
const normalized = sql
|
|
.replace(/--[^\n]*/g, '')
|
|
.replace(/\/\*[\s\S]*?\*\//g, '')
|
|
.replace(/'(?:[^']|'')*'/g, "''")
|
|
.replace(/"(?:[^"]|"")*"/g, '""');
|
|
|
|
for (const statement of normalized.split(';')) {
|
|
const trimmed = statement.trim();
|
|
const match =
|
|
trimmed.match(DATA_MUTATION_STATEMENT_REGEX) ??
|
|
trimmed.match(CTE_DATA_MUTATION_REGEX);
|
|
|
|
if (match) {
|
|
return match[1].toUpperCase();
|
|
}
|
|
}
|
|
|
|
return null;
|
|
};
|
|
|
|
export const rule = defineRule({
|
|
meta: {
|
|
type: 'problem',
|
|
docs: {
|
|
description:
|
|
'Disallow data mutations (UPDATE/INSERT/DELETE) in a fast instance command outside down(); backfills belong in a slow instance command',
|
|
},
|
|
schema: [],
|
|
messages: {
|
|
dataMutationInFastInstanceCommand:
|
|
"Fast instance commands must not run data mutations outside down() (found '{{ keyword }}'). A bulk write held in the same transaction as ADD/ALTER COLUMN keeps an ACCESS EXCLUSIVE lock and can stall reads during the deploy. Put backfills in a slow instance command's runDataMigration(); rollback writes belong in down().",
|
|
},
|
|
},
|
|
create: (context) => {
|
|
if (!isFastInstanceCommandFile(context.filename)) {
|
|
return {};
|
|
}
|
|
|
|
return {
|
|
CallExpression: (node: any) => {
|
|
const callee = node.callee;
|
|
|
|
if (
|
|
callee?.type !== 'MemberExpression' ||
|
|
callee.property?.type !== 'Identifier' ||
|
|
callee.property.name !== 'query'
|
|
) {
|
|
return;
|
|
}
|
|
|
|
if (isInsideDownMethod(node)) {
|
|
return;
|
|
}
|
|
|
|
const sqlArgument = node.arguments?.[0];
|
|
|
|
if (!sqlArgument) {
|
|
return;
|
|
}
|
|
|
|
const sql = getSqlText(sqlArgument);
|
|
|
|
if (sql === null) {
|
|
return;
|
|
}
|
|
|
|
const keyword = findDataMutationKeyword(sql);
|
|
|
|
if (keyword) {
|
|
context.report({
|
|
node: sqlArgument,
|
|
messageId: 'dataMutationInFastInstanceCommand',
|
|
data: { keyword },
|
|
});
|
|
}
|
|
},
|
|
};
|
|
},
|
|
});
|