diff --git a/packages/twenty-oxlint-rules/oxlint-plugin.ts b/packages/twenty-oxlint-rules/oxlint-plugin.ts index 058503f240..062c5d0477 100644 --- a/packages/twenty-oxlint-rules/oxlint-plugin.ts +++ b/packages/twenty-oxlint-rules/oxlint-plugin.ts @@ -32,6 +32,10 @@ import { rule as maxConstsPerFile, RULE_NAME as maxConstsPerFileName, } from './rules/max-consts-per-file'; +import { + rule as noDataMutationInFastInstanceCommand, + RULE_NAME as noDataMutationInFastInstanceCommandName, +} from './rules/no-data-mutation-in-fast-instance-command'; import { rule as noDirectAtomFamilyInSelector, RULE_NAME as noDirectAtomFamilyInSelectorName, @@ -84,6 +88,8 @@ export default definePlugin({ [injectWorkspaceRepositoryName]: injectWorkspaceRepository, [matchingStateVariableName]: matchingStateVariable, [maxConstsPerFileName]: maxConstsPerFile, + [noDataMutationInFastInstanceCommandName]: + noDataMutationInFastInstanceCommand, [noDirectAtomFamilyInSelectorName]: noDirectAtomFamilyInSelector, [noHardcodedColorsName]: noHardcodedColors, [noJotaiStoreInSelectorName]: noJotaiStoreInSelector, diff --git a/packages/twenty-oxlint-rules/rules/no-data-mutation-in-fast-instance-command.spec.ts b/packages/twenty-oxlint-rules/rules/no-data-mutation-in-fast-instance-command.spec.ts new file mode 100644 index 0000000000..62a8c566da --- /dev/null +++ b/packages/twenty-oxlint-rules/rules/no-data-mutation-in-fast-instance-command.spec.ts @@ -0,0 +1,102 @@ +import { RuleTester } from 'oxlint/plugins-dev'; + +import { rule, RULE_NAME } from './no-data-mutation-in-fast-instance-command'; + +const ruleTester = new RuleTester(); + +const BASE = + '/project/packages/twenty-server/src/database/commands/upgrade-version-command'; + +const FAST_FILE = `${BASE}/2-13/2-13-instance-command-fast-1781277453604-rename-flag.ts`; +const SLOW_FILE = `${BASE}/2-13/2-13-instance-command-slow-1781277480000-backfill-flag.ts`; + +ruleTester.run(RULE_NAME, rule, { + valid: [ + // Schema changes are exactly what a fast command is for. + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query('ALTER TABLE "core"."x" ADD COLUMN IF NOT EXISTS "y" boolean NOT NULL DEFAULT true'); } }`, + }, + // A foreign key clause contains DELETE/UPDATE but is not statement-leading DML. + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query('ALTER TABLE "core"."a" ADD CONSTRAINT "fk" FOREIGN KEY ("bId") REFERENCES "core"."b"("id") ON DELETE CASCADE ON UPDATE NO ACTION'); } }`, + }, + // A column named "updatedAt" must not trip the UPDATE keyword. + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query('ALTER TABLE "core"."x" ADD COLUMN "updatedAt" timestamptz'); } }`, + }, + // A read-only CTE has no data mutation. + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query(\`WITH recent AS (SELECT "id" FROM "core"."x") SELECT count(*) FROM recent\`); } }`, + }, + // A read-only CTE that only mentions a keyword in a string is not flagged. + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query(\`WITH note AS (SELECT 'needs update later' AS msg) SELECT msg FROM note\`); } }`, + }, + // A keyword after a parenthesis inside a string literal is not flagged. + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query(\`WITH note AS (SELECT 'reminder: (UPDATE later)' AS msg) SELECT msg FROM note\`); } }`, + }, + // Rollback DML lives in down() and is allowed (incl. via a CTE). + { + filename: FAST_FILE, + code: `class C { async down(q) { await q.query(\`UPDATE "core"."x" SET "y" = false WHERE "z" = true\`); } }`, + }, + { + filename: FAST_FILE, + code: `class C { async down(q) { await q.query(\`WITH ids AS (SELECT "id" FROM "core"."x") UPDATE "core"."x" SET "y" = true\`); } }`, + }, + // down() written as an arrow-function property is also recognized. + { + filename: FAST_FILE, + code: `class C { down = async (q) => { await q.query('DELETE FROM "core"."x" WHERE 1 = 1'); }; }`, + }, + // A slow command is the correct home for a data migration. + { + filename: SLOW_FILE, + code: `class C { async runDataMigration(ds) { await ds.query(\`UPDATE "core"."x" SET "y" = false\`); } }`, + }, + // Files outside the upgrade-command tree are ignored. + { + filename: '/project/packages/twenty-front/src/foo.ts', + code: `class C { async up(q) { await q.query(\`UPDATE "x" SET "y" = 1\`); } }`, + }, + ], + invalid: [ + // The incident pattern: bulk UPDATE in up(). + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query(\`UPDATE "core"."fieldMetadata" SET "isUIEditable" = false WHERE "isUIReadOnly" = true\`); } }`, + errors: [{ messageId: 'dataMutationInFastInstanceCommand' }], + }, + // INSERT in up(). + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query('INSERT INTO "core"."x" ("id") VALUES (1)'); } }`, + errors: [{ messageId: 'dataMutationInFastInstanceCommand' }], + }, + // DELETE in up(). + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query('DELETE FROM "core"."x" WHERE "t" = 1'); } }`, + errors: [{ messageId: 'dataMutationInFastInstanceCommand' }], + }, + // Data mutation moved into a helper called by up() is still forbidden. + { + filename: FAST_FILE, + code: `class C { async backfill(q) { await q.query('DELETE FROM "core"."x" WHERE 1 = 1'); } }`, + errors: [{ messageId: 'dataMutationInFastInstanceCommand' }], + }, + // CTE-wrapped data mutation in up(). + { + filename: FAST_FILE, + code: `class C { async up(q) { await q.query(\`WITH ids AS (SELECT "id" FROM "core"."x") UPDATE "core"."x" SET "y" = false\`); } }`, + errors: [{ messageId: 'dataMutationInFastInstanceCommand' }], + }, + ], +}); diff --git a/packages/twenty-oxlint-rules/rules/no-data-mutation-in-fast-instance-command.ts b/packages/twenty-oxlint-rules/rules/no-data-mutation-in-fast-instance-command.ts new file mode 100644 index 0000000000..f9cb040631 --- /dev/null +++ b/packages/twenty-oxlint-rules/rules/no-data-mutation-in-fast-instance-command.ts @@ -0,0 +1,143 @@ +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 }, + }); + } + }, + }; + }, +}); diff --git a/packages/twenty-server/.oxlintrc.json b/packages/twenty-server/.oxlintrc.json index bed13e34f6..1ec6c4c250 100644 --- a/packages/twenty-server/.oxlintrc.json +++ b/packages/twenty-server/.oxlintrc.json @@ -82,6 +82,7 @@ "twenty/rest-api-methods-should-be-guarded": "error", "twenty/graphql-resolvers-should-be-guarded": "error", "twenty/upgrade-command-filename": "error", + "twenty/no-data-mutation-in-fast-instance-command": "error", "twenty/enforce-module-boundaries": [ "error", { diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-13/2-13-instance-command-fast-1781277453604-rename-is-ui-read-only-to-is-ui-editable.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-13/2-13-instance-command-fast-1781277453604-rename-is-ui-read-only-to-is-ui-editable.ts index 98c99f563c..e1a6a2a92d 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/2-13/2-13-instance-command-fast-1781277453604-rename-is-ui-read-only-to-is-ui-editable.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-13/2-13-instance-command-fast-1781277453604-rename-is-ui-read-only-to-is-ui-editable.ts @@ -1,3 +1,10 @@ +// Grandfathered: this 2.13 command already shipped to cloud and is recorded +// complete there, so per "never rewrite a committed instance command" it is +// frozen as-is. Its up() does ADD COLUMN + bulk UPDATE in one transaction — the +// exact pattern that held an ACCESS EXCLUSIVE lock and stalled prod reads, and +// the reason no-data-mutation-in-fast-instance-command exists. This is an +// exception, not a precedent: new backfills go in a slow instance command. +/* oxlint-disable twenty/no-data-mutation-in-fast-instance-command */ import { QueryRunner } from 'typeorm'; import { RegisteredInstanceCommand } from 'src/engine/core-modules/upgrade/decorators/registered-instance-command.decorator';