feat(lint): forbid data mutations in fast instance command up() (#21547)

## 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)_  <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>  --------- Co-authored-by: Claude <noreply@anthropic.com>
2026-06-14 20:44:54 +02:00
parent 47b31acc21
commit 0baa333809
5 changed files with 259 additions and 0 deletions
@@ -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,
@@ -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' }],
+    },
+  ],
+});
@@ -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 },
+          });
+        }
+      },
+    };
+  },
+});
@@ -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",
      {
@@ -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';