From 02cb8ceebd19ed7469df3b4c6a7348081b0be50b Mon Sep 17 00:00:00 2001 From: 3m1n3nc3 Date: Sun, 5 Jul 2026 08:02:19 +0100 Subject: [PATCH] fix(cli): rollback --step default + surface query error causes Two CLI regressions found by running the built CLI against Postgres: - migrate:rollback with no --step failed with '--step must be a positive integer'. musket returns an empty string (not undefined) for a declared but unpassed value option, so Number('') = 0 tripped the validation. Treat null/undefined/'' alike as 'not provided' (default: the last batch). The test option mocks hid this by returning undefined for unset options. - The db command printed only the generic 'Raw query execution failed for the Kysely adapter' wrapper, hiding the real error. It now appends the underlying cause (e.g. 'relation "x" does not exist'). Tests: rollback with step='' rolls back the last batch; db appends the cause behind a wrapper error. --- src/cli/commands/DbCommand.ts | 18 ++++++++- src/cli/commands/MigrateRollbackCommand.ts | 5 ++- tests/base/db-command.spec.ts | 18 +++++++++ tests/base/migrate-database.spec.ts | 47 ++++++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/cli/commands/DbCommand.ts b/src/cli/commands/DbCommand.ts index 0590683..e52416b 100644 --- a/src/cli/commands/DbCommand.ts +++ b/src/cli/commands/DbCommand.ts @@ -52,12 +52,28 @@ export class DbCommand extends Command { bindings: bindings as DatabaseValue[], })) as DatabaseRow[] } catch (error) { - return void this.error(`Error: ${error instanceof Error ? error.message : String(error)}`) + return void this.error(`Error: ${this.describeError(error)}`) } this.render(rows) } + /** + * Builds a readable message from an adapter error, appending the underlying + * cause (e.g. the PostgreSQL error) so failures aren't hidden behind the + * generic "Raw query execution failed" wrapper. + */ + private describeError(error: unknown): string { + const message = error instanceof Error ? error.message : String(error) + const cause = (error as { cause?: unknown }).cause + const causeMessage = + cause instanceof Error ? cause.message : typeof cause === 'string' ? cause : undefined + + return causeMessage && !message.includes(causeMessage) + ? `${message} (${causeMessage})` + : message + } + /** * Resolves the SQL to run. Priority: the positional argument, then `--file`, * then an interactive editor prompt (so a bare `arkorm db` lets you compose a diff --git a/src/cli/commands/MigrateRollbackCommand.ts b/src/cli/commands/MigrateRollbackCommand.ts index 48cb620..cdc870c 100644 --- a/src/cli/commands/MigrateRollbackCommand.ts +++ b/src/cli/commands/MigrateRollbackCommand.ts @@ -76,8 +76,11 @@ export class MigrateRollbackCommand extends Command { // rolls back the single most recent batch (the group of migrations from the // last `migrate` run); `--step=N` rolls back the last N batches. Targets come // back ordered for rollback — the reverse of the order they were applied. + // musket yields an empty string for a declared-but-unpassed value option, so + // treat null/undefined/'' all as "not provided" (default: one batch). const stepOption = this.option('step') - const stepCount = stepOption == null ? 1 : Number(stepOption) + const stepProvided = stepOption != null && String(stepOption).trim() !== '' + const stepCount = stepProvided ? Number(stepOption) : 1 if (!Number.isFinite(stepCount) || stepCount <= 0 || !Number.isInteger(stepCount)) return void this.error('Error: --step must be a positive integer.') diff --git a/tests/base/db-command.spec.ts b/tests/base/db-command.spec.ts index ffeff06..ccfeab6 100644 --- a/tests/base/db-command.spec.ts +++ b/tests/base/db-command.spec.ts @@ -214,6 +214,24 @@ describe('DbCommand (raw SQL)', () => { expect(io.errorLines.some((line) => line.includes('syntax error'))).toBe(true) }) + it('appends the underlying cause behind a generic wrapper error', async () => { + const wrapper = new Error('Raw query execution failed for the Kysely adapter.') + ;(wrapper as { cause?: unknown }).cause = new Error('relation "widgets" does not exist') + const adapter = { + rawQuery: async () => { + throw wrapper + }, + } + + const io = await runDb(adapter, {}, { sql: 'select * from widgets' }) + + expect( + io.errorLines.some( + (line) => line.includes('Raw query execution failed') && line.includes('does not exist'), + ), + ).toBe(true) + }) + it('loads the adapter from arkormx.config when none is pre-set (regression)', async () => { // No getConfig stub: the command must load the project config itself and // resolve the adapter from it — the original bug returned "no adapter". diff --git a/tests/base/migrate-database.spec.ts b/tests/base/migrate-database.spec.ts index 0b2e780..793f7ce 100644 --- a/tests/base/migrate-database.spec.ts +++ b/tests/base/migrate-database.spec.ts @@ -318,6 +318,53 @@ describe('database-backed migration command fallback', () => { expect(adapter.state.migrations).toHaveLength(0) }) + it('treats an empty --step string as the default last batch (regression)', async () => { + // musket yields '' for a declared-but-unpassed value option like `--step=`; + // the command must not read that as step 0. + const workspace = makeTempDir('arkormx-db-rollback-empty-step-') + process.chdir(workspace) + + const migrationsDir = join(workspace, 'database', 'migrations') + mkdirSync(migrationsDir, { recursive: true }) + + const migrationBaseImport = `${originalCwd.replace(/\\/g, '/')}/src/database/Migration.ts` + writeFileSync( + join(migrationsDir, 'CreateGadgets.ts'), + [ + `import { Migration } from '${migrationBaseImport}'`, + '', + 'export class CreateGadgets extends Migration {', + " async up (schema) { schema.createTable('gadgets', (table) => { table.id() }) }", + " async down (schema) { schema.dropTable('gadgets') }", + '}', + '', + ].join('\n'), + ) + + const adapter = createNoopAdapter() as DatabaseAdapter & { + state: AppliedMigrationsState + executed: SchemaOperation[][] + } + + configureArkormRuntime(() => ({}), { adapter, paths: { migrations: migrationsDir } }) + + const app = new CliApp() + + const migrate = new MigrateCommand(app, new Kernel(app)) + ;(migrate as unknown as { app: CliApp }).app = app + attachCommandIo(migrate as unknown as any, { all: true }) + await migrate.handle() + + const rollback = new MigrateRollbackCommand(app, new Kernel(app)) + ;(rollback as unknown as { app: CliApp }).app = app + const io = attachCommandIo(rollback as unknown as any, { step: '' }) + await rollback.handle() + + expect(io.errorLines).toHaveLength(0) + expect(io.successLines.some((line) => line.includes('Rolled back 1 migration(s).'))).toBe(true) + expect(adapter.state.migrations).toHaveLength(0) + }) + it('offers to create the configured database before adapter-backed migrate runs', async () => { const workspace = makeTempDir('arkormx-db-migrate-create-database-') process.chdir(workspace)