mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
refactor(core): Guard and reverse the agent id width migration
Addresses two review findings. Measure each column before narrowing it. Nine of the ten are held to their new width by a foreign key, but agent_chat_subscriptions.credentialId has none on purpose, so a subscription stays recordable after its credential is deleted. Its value arrives through an agent config field that zod bounds only with min(1). The integration refuses to start unless the id resolves to a real credential, so an oversized value should not exist, but nothing in the database enforces that. A column that would truncate now keeps its old width and logs why. This change is cosmetic and must never be the reason an instance fails to boot. Make the migration reversible. It only moves declared varchar bounds, so widening them back restores the previous schema with no data loss.
This commit is contained in:
parent
5968601c6e
commit
4d250c7732
|
|
@ -1,4 +1,4 @@
|
|||
import type { IrreversibleMigration, MigrationContext } from '../migration-types';
|
||||
import type { MigrationContext, ReversibleMigration } from '../migration-types';
|
||||
|
||||
/**
|
||||
* Narrows the agent id columns that were declared wider than the id column they
|
||||
|
|
@ -27,17 +27,33 @@ import type { IrreversibleMigration, MigrationContext } from '../migration-types
|
|||
* (a platform thread id, where varchar(255) is right — and probably where the
|
||||
* `credentialId` beside it got its width).
|
||||
*
|
||||
* On Postgres each step rewrites the table, so it holds an exclusive lock for
|
||||
* the length of the rewrite. Every narrowed column is fed from a column already
|
||||
* at or below its new width, so no value can be too long.
|
||||
* Nine of the ten columns are held to their new width by a foreign key, so no
|
||||
* value can be too long. `agent_chat_subscriptions.credentialId` is the
|
||||
* exception — deliberately no foreign key, so a subscription stays recordable
|
||||
* after its credential is deleted — and its value reaches us through an agent
|
||||
* config field that zod bounds only with `min(1)`. In practice the integration
|
||||
* refuses to start unless the id resolves to a real credential, but nothing in
|
||||
* the database enforces that, so every column is measured before it is narrowed
|
||||
* and one that would truncate is left at its old width with a warning. This
|
||||
* migration is cosmetic; it must never be the reason an instance fails to boot.
|
||||
*
|
||||
* On Postgres each step rewrites the table and holds an exclusive lock for the
|
||||
* length of the rewrite.
|
||||
*/
|
||||
const COLUMNS: Array<{ table: string; column: string; from: number; to: number }> = [
|
||||
interface WidthChange {
|
||||
table: string;
|
||||
column: string;
|
||||
from: number;
|
||||
to: number;
|
||||
}
|
||||
|
||||
const COLUMNS: WidthChange[] = [
|
||||
// Hold a `project.id`.
|
||||
{ table: 'agents', column: 'projectId', from: 255, to: 36 },
|
||||
{ table: 'agent_execution_threads', column: 'projectId', from: 255, to: 36 },
|
||||
// Holds an `agents.id`.
|
||||
{ table: 'agent_checkpoints', column: 'agentId', from: 255, to: 36 },
|
||||
// Holds a `credentials_entity.id`.
|
||||
// Holds a `credentials_entity.id`. The one column here without a foreign key.
|
||||
{ table: 'agent_chat_subscriptions', column: 'credentialId', from: 255, to: 36 },
|
||||
// Hold an `agents_threads.id`.
|
||||
{ table: 'agents_messages', column: 'threadId', from: 255, to: 128 },
|
||||
|
|
@ -49,33 +65,86 @@ const COLUMNS: Array<{ table: string; column: string; from: number; to: number }
|
|||
{ table: 'agents_observation_locks', column: 'observationScopeId', from: 255, to: 128 },
|
||||
];
|
||||
|
||||
export class AlignAgentIdColumnWidths1788853123113 implements IrreversibleMigration {
|
||||
async up({ isPostgres, isSqlite, runQuery, escape, tablePrefix }: MigrationContext) {
|
||||
if (isPostgres) {
|
||||
for (const { table, column, to } of COLUMNS) {
|
||||
await runQuery(
|
||||
`ALTER TABLE ${escape.tableName(table)} ALTER COLUMN ${escape.columnName(column)} TYPE VARCHAR(${to});`,
|
||||
);
|
||||
}
|
||||
} else if (isSqlite) {
|
||||
// SQLite does not enforce varchar limits, so only the declared schema
|
||||
// needs to change. Rewriting it in place avoids recreating tables that
|
||||
// other agent tables reference with ON DELETE CASCADE.
|
||||
await runQuery('PRAGMA writable_schema = 1;');
|
||||
try {
|
||||
for (const { table, column, from, to } of COLUMNS) {
|
||||
await runQuery(
|
||||
"UPDATE sqlite_master SET sql = replace(sql, :from, :to) WHERE type = 'table' AND name = :tableName",
|
||||
{
|
||||
from: `"${column}" varchar(${from})`,
|
||||
to: `"${column}" varchar(${to})`,
|
||||
tableName: `${tablePrefix}${table}`,
|
||||
},
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
await runQuery('PRAGMA writable_schema = 0;');
|
||||
}
|
||||
const reverse = (changes: WidthChange[]): WidthChange[] =>
|
||||
changes.map(({ table, column, from, to }) => ({ table, column, from: to, to: from }));
|
||||
|
||||
/**
|
||||
* Drops any change whose column already holds a value longer than the width it
|
||||
* would move to. Only narrowing can trip this, so widening skips the scan.
|
||||
*/
|
||||
async function applicable(
|
||||
{ runQuery, escape, logger, migrationName }: MigrationContext,
|
||||
changes: WidthChange[],
|
||||
): Promise<WidthChange[]> {
|
||||
const keep: WidthChange[] = [];
|
||||
|
||||
for (const change of changes) {
|
||||
const { table, column, from, to } = change;
|
||||
if (to >= from) {
|
||||
keep.push(change);
|
||||
continue;
|
||||
}
|
||||
|
||||
const rows = await runQuery<Array<{ longest: number | null }>>(
|
||||
`SELECT MAX(LENGTH(${escape.columnName(column)})) AS longest FROM ${escape.tableName(table)};`,
|
||||
);
|
||||
const longest = Number(rows[0]?.longest ?? 0);
|
||||
|
||||
if (longest > to) {
|
||||
logger.warn(
|
||||
`[${migrationName}] Leaving ${table}.${column} at varchar(${from}): it holds a value of ${longest} characters, which varchar(${to}) cannot store. Shorten or remove those rows and this column is aligned by the next release.`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
keep.push(change);
|
||||
}
|
||||
|
||||
return keep;
|
||||
}
|
||||
|
||||
async function setWidths(context: MigrationContext, changes: WidthChange[]): Promise<void> {
|
||||
const { isPostgres, isSqlite, runQuery, escape, tablePrefix } = context;
|
||||
const applied = await applicable(context, changes);
|
||||
|
||||
if (isPostgres) {
|
||||
for (const { table, column, to } of applied) {
|
||||
await runQuery(
|
||||
`ALTER TABLE ${escape.tableName(table)} ALTER COLUMN ${escape.columnName(column)} TYPE VARCHAR(${to});`,
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (!isSqlite) return;
|
||||
|
||||
// SQLite does not enforce varchar limits, so only the declared schema needs
|
||||
// to change. Rewriting it in place avoids recreating tables that other agent
|
||||
// tables reference with ON DELETE CASCADE.
|
||||
await runQuery('PRAGMA writable_schema = 1;');
|
||||
try {
|
||||
for (const { table, column, from, to } of applied) {
|
||||
await runQuery(
|
||||
"UPDATE sqlite_master SET sql = replace(sql, :from, :to) WHERE type = 'table' AND name = :tableName",
|
||||
{
|
||||
from: `"${column}" varchar(${from})`,
|
||||
to: `"${column}" varchar(${to})`,
|
||||
tableName: `${tablePrefix}${table}`,
|
||||
},
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
await runQuery('PRAGMA writable_schema = 0;');
|
||||
}
|
||||
}
|
||||
|
||||
export class AlignAgentIdColumnWidths1788853123113 implements ReversibleMigration {
|
||||
async up(context: MigrationContext) {
|
||||
await setWidths(context, COLUMNS);
|
||||
}
|
||||
|
||||
/** Widening back is lossless, so the narrowing is fully reversible. */
|
||||
async down(context: MigrationContext) {
|
||||
await setWidths(context, reverse(COLUMNS));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user