Skip to content

feat: automatic db migrations #816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

feat: automatic db migrations #816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
96b825e
fix: detect if table exists on migration, add retry count
RohitR311 Oct 2, 2025
516b90d
fix: migration import path
RohitR311 Oct 2, 2025
3a43fff
fix: make retryCount optional field
RohitR311 Oct 2, 2025
0ffd66f
feat: run migration on backend pull
RohitR311 Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions server/docker-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,15 @@ wait_for_postgres() {
# Wait for PostgreSQL to be ready
wait_for_postgres

# Run the application with migrations before startup
NODE_OPTIONS="--max-old-space-size=4096" node -e "require('./server/src/db/migrate')().then(() => { console.log('Migration process completed.'); })"
# Run database migrations using npm script
echo "Running database migrations..."
npm run migrate

# Run the server normally
if [ $? -eq 0 ]; then
echo "✅ Migrations completed successfully!"
else
echo "⚠️ Migration failed, but continuing to start server..."
fi
Comment on lines +29 to +37
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider failing fast on migration errors.

The script continues to start the server even if migrations fail (line 36). This could lead to runtime errors if the schema is out of sync with the application code (e.g., missing retryCount column).

Consider changing the behavior to exit on migration failure to prevent cascading errors:

 if [ $? -eq 0 ]; then
   echo "✅ Migrations completed successfully!"
 else
-  echo "⚠️  Migration failed, but continuing to start server..."
+  echo "❌ Migration failed. Exiting to prevent runtime errors."
+  exit 1
 fi

Alternatively, if continuing on failure is intentional (e.g., for local development), document the rationale in a comment.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Run database migrations using npm script
echo "Running database migrations..."
npm run migrate
# Run the server normally
if [ $? -eq 0 ]; then
echo "✅ Migrations completed successfully!"
else
echo "⚠️ Migration failed, but continuing to start server..."
fi
# Run database migrations using npm script
echo "Running database migrations..."
npm run migrate
if [ $? -eq 0 ]; then
echo "✅ Migrations completed successfully!"
else
echo "❌ Migration failed. Exiting to prevent runtime errors."
exit 1
fi
🤖 Prompt for AI Agents 
In server/docker-entrypoint.sh around lines 29 to 37, the script currently
continues to start the server when migrations fail which can cause runtime
errors; update the else branch to fail fast by exiting with a non-zero code
(e.g., exit 1) so the container stops on migration failure, or if the intent is
to allow startup despite migration errors (local/dev), add a clear comment
explaining that choice and why it's safe and desired.


# Run the server normally
exec "$@"
17 changes: 4 additions & 13 deletions server/src/db/migrate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
'use strict';

import { execSync } from 'child_process';
import path from 'path';
import { fileURLToPath } from 'url';
import db from './models/index.js';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
const { execSync } = require('child_process');
const path = require('path');

async function runMigrations() {
try {
console.log('Testing database connection...');
await db.sequelize.authenticate();
console.log('Database connection established successfully.');

console.log('Running database migrations...');
execSync('npx sequelize-cli db:migrate', {
execSync('npx sequelize-cli db:migrate', {
stdio: 'inherit',
cwd: path.resolve(__dirname, '../../..')
cwd: path.resolve(__dirname, '../../..')
});
console.log('Migrations completed successfully');
return true;
Expand Down
20 changes: 20 additions & 0 deletions server/src/db/migrations/20250202000000-add-retry-count.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';

module.exports = {
up: async (queryInterface, Sequelize) => {
const tableInfo = await queryInterface.describeTable('run');

// Only add the column if it doesn't exist
if (!tableInfo.retryCount) {
await queryInterface.addColumn('run', 'retryCount', {
type: Sequelize.INTEGER,
allowNull: true,
defaultValue: 0,
});
}
},

down: async (queryInterface, Sequelize) => {
await queryInterface.removeColumn('run', 'retryCount');
}
Comment on lines +17 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Make the down migration idempotent.

The down migration unconditionally removes the retryCount column without checking if it exists. This is inconsistent with the guarded patterns in other migrations (e.g., 20250327111003-add-airtable-columns.js and 20250527105655-add-webhooks.js) and will fail if the column was never added or was already removed.

Apply this diff to make the down migration idempotent:

 down: async (queryInterface, Sequelize) => {
-  await queryInterface.removeColumn('run', 'retryCount');
+  const tableInfo = await queryInterface.describeTable('run');
+  if (tableInfo.retryCount) {
+    await queryInterface.removeColumn('run', 'retryCount');
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
down: async (queryInterface, Sequelize) => {
await queryInterface.removeColumn('run', 'retryCount');
}
down: async (queryInterface, Sequelize) => {
const tableInfo = await queryInterface.describeTable('run');
if (tableInfo.retryCount) {
await queryInterface.removeColumn('run', 'retryCount');
}
}
🤖 Prompt for AI Agents 
In server/src/db/migrations/20250202000000-add-retry-count.js around lines 17 to
19, the down migration unconditionally calls queryInterface.removeColumn('run',
'retryCount') which can fail if the column does not exist; update the down
migration to first inspect the table (e.g., using
queryInterface.describeTable('run') or a similar metadata call) and only call
removeColumn if the returned description shows retryCount exists, so the down
migration becomes idempotent and mirrors the guarded pattern used in other
migrations.

};
30 changes: 22 additions & 8 deletions server/src/db/migrations/20250327111003-add-airtable-columns.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,28 @@ module.exports = {
// Remove Airtable related columns
return queryInterface.sequelize.transaction(async (transaction) => {
try {
// Remove columns in reverse order
await queryInterface.removeColumn('robot', 'airtable_refresh_token', { transaction });
await queryInterface.removeColumn('robot', 'airtable_access_token', { transaction });
await queryInterface.removeColumn('robot', 'airtable_table_id', { transaction });
await queryInterface.removeColumn('robot', 'airtable_table_name', { transaction });
await queryInterface.removeColumn('robot', 'airtable_base_name', { transaction });
await queryInterface.removeColumn('robot', 'airtable_base_id', { transaction });

const tableInfo = await queryInterface.describeTable('robot', { transaction });

// Remove columns in reverse order, only if they exist
if (tableInfo.airtable_refresh_token) {
await queryInterface.removeColumn('robot', 'airtable_refresh_token', { transaction });
}
if (tableInfo.airtable_access_token) {
await queryInterface.removeColumn('robot', 'airtable_access_token', { transaction });
}
if (tableInfo.airtable_table_id) {
await queryInterface.removeColumn('robot', 'airtable_table_id', { transaction });
}
if (tableInfo.airtable_table_name) {
await queryInterface.removeColumn('robot', 'airtable_table_name', { transaction });
}
if (tableInfo.airtable_base_name) {
await queryInterface.removeColumn('robot', 'airtable_base_name', { transaction });
}
if (tableInfo.airtable_base_id) {
await queryInterface.removeColumn('robot', 'airtable_base_id', { transaction });
}

return Promise.resolve();
} catch (error) {
return Promise.reject(error);
Expand Down
52 changes: 35 additions & 17 deletions server/src/db/migrations/20250527105655-add-webhooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,44 @@

module.exports = {
async up(queryInterface, Sequelize) {
await queryInterface.addColumn('robot', 'webhooks', {
type: Sequelize.JSONB,
allowNull: true,
defaultValue: null,
comment: 'Webhook configurations for the robot'
});
const tableInfo = await queryInterface.describeTable('robot');

// Optional: Add an index for better query performance if you plan to search within webhook data
await queryInterface.addIndex('robot', {
fields: ['webhooks'],
using: 'gin', // GIN index for JSONB columns
name: 'robot_webhooks_gin_idx'
});
// Only add the column if it doesn't exist
if (!tableInfo.webhooks) {
await queryInterface.addColumn('robot', 'webhooks', {
type: Sequelize.JSONB,
allowNull: true,
defaultValue: null,
comment: 'Webhook configurations for the robot'
});
}

// Check if index exists before adding
const indexes = await queryInterface.showIndex('robot');
const indexExists = indexes.some(index => index.name === 'robot_webhooks_gin_idx');

if (!indexExists && tableInfo.webhooks) {
await queryInterface.addIndex('robot', {
fields: ['webhooks'],
using: 'gin', // GIN index for JSONB columns
name: 'robot_webhooks_gin_idx'
});
}
},

async down(queryInterface, Sequelize) {
// Remove the index first
await queryInterface.removeIndex('robot', 'robot_webhooks_gin_idx');

// Then remove the column
await queryInterface.removeColumn('robot', 'webhooks');
// Check if index exists before removing
const indexes = await queryInterface.showIndex('robot');
const indexExists = indexes.some(index => index.name === 'robot_webhooks_gin_idx');

if (indexExists) {
await queryInterface.removeIndex('robot', 'robot_webhooks_gin_idx');
}

// Check if column exists before removing
const tableInfo = await queryInterface.describeTable('robot');
if (tableInfo.webhooks) {
await queryInterface.removeColumn('robot', 'webhooks');
}
}
};
2 changes: 1 addition & 1 deletion server/src/models/Run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Run extends Model<RunAttributes, RunCreationAttributes> implements RunAttr
public runByAPI!: boolean;
public serializableOutput!: Record<string, any[]>;
public binaryOutput!: Record<string, any>;
public retryCount!: number;
public retryCount?: number;
}

Run.init(
Expand Down