From 61387f16c2d6a9a40c34b8bd1e5d367735619f9c Mon Sep 17 00:00:00 2001 From: Timothy Lowrimore Date: Thu, 8 Oct 2026 13:42:31 -0600 Subject: [PATCH 1/5] fix: ensures dbname is always the last arg provided to pg_dump --- src/commands/pg/pull.ts | 11 ++++-- test/unit/commands/pg/pull.unit.test.ts | 52 +++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/commands/pg/pull.ts b/src/commands/pg/pull.ts index c4347fdbef..d48bc309f3 100644 --- a/src/commands/pg/pull.ts +++ b/src/commands/pg/pull.ts @@ -60,7 +60,7 @@ export default class Pull extends Command { const target = await maybeTunnel(targetIn) const exclude = exclusions.map(e => ('--exclude-table-data=' + e)).join(' ') - const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku', ...connArgs(source, true)] + const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku'] if (exclude !== '') dumpFlags.push(exclude) @@ -74,7 +74,7 @@ export default class Pull extends Command { } if (source.password) dumpOptions.env.PGPASSWORD = source.password - const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner', ...connArgs(target)] + const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner'] const restoreOptions: childProcess.SpawnOptions & {env: NodeJS.ProcessEnv} = { env: {...env}, @@ -83,8 +83,11 @@ export default class Pull extends Command { } if (target.password) restoreOptions.env.PGPASSWORD = target.password - const pgDump = childProcess.spawn('pg_dump', dumpFlags, dumpOptions) - const pgRestore = childProcess.spawn('pg_restore', restoreFlags, restoreOptions) + const dumpArgs = dumpFlags.concat(connArgs(source, true)) + const restoreArgs = restoreFlags.concat(connArgs(target)) + + const pgDump = childProcess.spawn('pg_dump', dumpArgs, dumpOptions) + const pgRestore = childProcess.spawn('pg_restore', restoreArgs, restoreOptions) await spawnPipe(pgDump, pgRestore) diff --git a/test/unit/commands/pg/pull.unit.test.ts b/test/unit/commands/pg/pull.unit.test.ts index fea49e6f72..c360794ffd 100644 --- a/test/unit/commands/pg/pull.unit.test.ts +++ b/test/unit/commands/pg/pull.unit.test.ts @@ -88,6 +88,58 @@ describe('pg:pull', function () { expect(stderr).to.eq('') }) + describe('with --exclude-table-data', function () { + beforeEach(function () { + spawnStub.withArgs('pg_dump', match.array, match.any).returns({ + on: exitHandler, + stdout: { + pipe() {}, + }, + }) + spawnStub.withArgs('pg_restore', match.array, match.any).returns({ + on: exitHandler, + stdin: { + end() {}, + }, + }) + }) + + const getDumpArgs = (): string[] => { + const dumpCall = spawnStub.getCalls().find(call => call.args[0] === 'pg_dump') + expect(dumpCall, 'pg_dump should have been spawned').to.not.eq(undefined) + return dumpCall!.args[1] as string[] + } + + it('passes the exclusion and keeps the database name as the last pg_dump argument', async () => { + await runCommand(Cmd, [ + 'postgres-1', + 'localdb', + '-a', + 'myapp', + '--exclude-table-data=logs', + ]) + + const args = getDumpArgs() + expect(args).to.include('--exclude-table-data=logs') + expect(args[args.length - 1]).to.eq('mydb') + }) + + it('keeps the database name last when excluding multiple tables', async () => { + await runCommand(Cmd, [ + 'postgres-1', + 'localdb', + '-a', + 'myapp', + '--exclude-table-data=logs;events', + ]) + + const args = getDumpArgs() + expect(args.some(arg => arg.includes('--exclude-table-data=logs'))).to.eq(true) + expect(args.some(arg => arg.includes('--exclude-table-data=events'))).to.eq(true) + expect(args[args.length - 1]).to.eq('mydb') + }) + }) + skipOnWindows('opens an SSH tunnel and runs pg_dump for bastion databases', async () => { db.bastionHost = 'bastion-host' db.bastionKey = 'super-private-key' From bb80b01fd0d06fcea865c69d5553899d5f4bc16d Mon Sep 17 00:00:00 2001 From: Timothy Lowrimore Date: Thu, 8 Oct 2026 14:00:34 -0600 Subject: [PATCH 2/5] clarifying comment --- src/commands/pg/pull.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/commands/pg/pull.ts b/src/commands/pg/pull.ts index d48bc309f3..ad2706e31a 100644 --- a/src/commands/pg/pull.ts +++ b/src/commands/pg/pull.ts @@ -83,6 +83,8 @@ export default class Pull extends Command { } if (target.password) restoreOptions.env.PGPASSWORD = target.password + // Ensures the database name is always the last value in the args. + // This is necessary for the windows pg_dump and pg_restore binaries. const dumpArgs = dumpFlags.concat(connArgs(source, true)) const restoreArgs = restoreFlags.concat(connArgs(target)) From 1a7036d7425b762ce6fd0ffc5dccba2036dda54a Mon Sep 17 00:00:00 2001 From: Timothy Lowrimore Date: Thu, 8 Oct 2026 14:31:08 -0600 Subject: [PATCH 3/5] fixes the issue in pg:push command --- src/commands/pg/push.ts | 13 ++++-- test/unit/commands/pg/push.unit.test.ts | 58 +++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/commands/pg/push.ts b/src/commands/pg/push.ts index 881264167b..4d65749a67 100644 --- a/src/commands/pg/push.ts +++ b/src/commands/pg/push.ts @@ -56,7 +56,7 @@ export default class Push extends Command { const target = await maybeTunnel(targetIn) const exclude = exclusions.map(e => ('--exclude-table-data=' + e)).join(' ') - const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku', ...connArgs(source, true)] + const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku'] if (exclude !== '') dumpFlags.push(exclude) @@ -70,7 +70,7 @@ export default class Push extends Command { } if (source.password) dumpOptions.env.PGPASSWORD = source.password - const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner', ...connArgs(target)] + const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner'] const restoreOptions: childProcess.SpawnOptions & {env: NodeJS.ProcessEnv} = { env: {...env}, @@ -79,8 +79,13 @@ export default class Push extends Command { } if (target.password) restoreOptions.env.PGPASSWORD = target.password - const pgDump = childProcess.spawn('pg_dump', dumpFlags, dumpOptions) - const pgRestore = childProcess.spawn('pg_restore', restoreFlags, restoreOptions) + // Ensures the database name is always the last value in the args. + // This is necessary for the windows pg_dump and pg_restore binaries. + const dumpArgs = dumpFlags.concat(connArgs(source, true)) + const restoreArgs = restoreFlags.concat(connArgs(target)) + + const pgDump = childProcess.spawn('pg_dump', dumpArgs, dumpOptions) + const pgRestore = childProcess.spawn('pg_restore', restoreArgs, restoreOptions) await spawnPipe(pgDump, pgRestore) diff --git a/test/unit/commands/pg/push.unit.test.ts b/test/unit/commands/pg/push.unit.test.ts index 97534a7478..31c1c25dbd 100644 --- a/test/unit/commands/pg/push.unit.test.ts +++ b/test/unit/commands/pg/push.unit.test.ts @@ -186,6 +186,64 @@ describe('pg:push', function () { expect(stderr).to.eq('') }) + describe('with --exclude-table-data', function () { + beforeEach(function () { + spawnStub.withArgs('pg_dump', match.array, match.any).returns({ + on: exitHandler, + stdout: { + pipe() {}, + }, + }) + spawnStub.withArgs('pg_restore', match.array, match.any).returns({ + on: exitHandler, + stdin: { + end() {}, + }, + }) + }) + + const getSpawnArgs = (command: string): string[] => { + const call = spawnStub.getCalls().find(call => call.args[0] === command) + expect(call, `${command} should have been spawned`).to.not.eq(undefined) + return call!.args[1] as string[] + } + + it('passes the exclusion and keeps the database name as the last pg_dump argument', async () => { + await runCommand(Cmd, [ + 'localdb', + 'postgres-1', + '-a', + 'myapp', + '--exclude-table-data=logs', + ]) + + const dumpArgs = getSpawnArgs('pg_dump') + expect(dumpArgs).to.include('--exclude-table-data=logs') + expect(dumpArgs[dumpArgs.length - 1]).to.eq('localdb') + + const restoreArgs = getSpawnArgs('pg_restore') + expect(restoreArgs[restoreArgs.length - 1]).to.eq('mydb') + }) + + it('keeps the database name last when excluding multiple tables', async () => { + await runCommand(Cmd, [ + 'localdb', + 'postgres-1', + '-a', + 'myapp', + '--exclude-table-data=logs;events', + ]) + + const dumpArgs = getSpawnArgs('pg_dump') + expect(dumpArgs.some(arg => arg.includes('--exclude-table-data=logs'))).to.eq(true) + expect(dumpArgs.some(arg => arg.includes('--exclude-table-data=events'))).to.eq(true) + expect(dumpArgs[dumpArgs.length - 1]).to.eq('localdb') + + const restoreArgs = getSpawnArgs('pg_restore') + expect(restoreArgs[restoreArgs.length - 1]).to.eq('mydb') + }) + }) + skipOnWindows('exits non-zero when there is an error', async () => { const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku', 'localdb'] const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner', '-U', 'jeff', '-h', 'herokai.com', '-p', '5432', '-d', 'mydb'] From 0be47cd8c8c834b6c7c56010b0c3b8059844ea2b Mon Sep 17 00:00:00 2001 From: Timothy Lowrimore Date: Thu, 8 Oct 2026 14:34:32 -0600 Subject: [PATCH 4/5] linter fixes --- test/unit/commands/pg/pull.unit.test.ts | 6 +++--- test/unit/commands/pg/push.unit.test.ts | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/unit/commands/pg/pull.unit.test.ts b/test/unit/commands/pg/pull.unit.test.ts index c360794ffd..8aeed1f00c 100644 --- a/test/unit/commands/pg/pull.unit.test.ts +++ b/test/unit/commands/pg/pull.unit.test.ts @@ -106,7 +106,7 @@ describe('pg:pull', function () { const getDumpArgs = (): string[] => { const dumpCall = spawnStub.getCalls().find(call => call.args[0] === 'pg_dump') - expect(dumpCall, 'pg_dump should have been spawned').to.not.eq(undefined) + expect(dumpCall, 'pg_dump should have been spawned').to.not.eq() return dumpCall!.args[1] as string[] } @@ -121,7 +121,7 @@ describe('pg:pull', function () { const args = getDumpArgs() expect(args).to.include('--exclude-table-data=logs') - expect(args[args.length - 1]).to.eq('mydb') + expect(args.at(-1)).to.eq('mydb') }) it('keeps the database name last when excluding multiple tables', async () => { @@ -136,7 +136,7 @@ describe('pg:pull', function () { const args = getDumpArgs() expect(args.some(arg => arg.includes('--exclude-table-data=logs'))).to.eq(true) expect(args.some(arg => arg.includes('--exclude-table-data=events'))).to.eq(true) - expect(args[args.length - 1]).to.eq('mydb') + expect(args.at(-1)).to.eq('mydb') }) }) diff --git a/test/unit/commands/pg/push.unit.test.ts b/test/unit/commands/pg/push.unit.test.ts index 31c1c25dbd..60c881641d 100644 --- a/test/unit/commands/pg/push.unit.test.ts +++ b/test/unit/commands/pg/push.unit.test.ts @@ -204,7 +204,7 @@ describe('pg:push', function () { const getSpawnArgs = (command: string): string[] => { const call = spawnStub.getCalls().find(call => call.args[0] === command) - expect(call, `${command} should have been spawned`).to.not.eq(undefined) + expect(call, `${command} should have been spawned`).to.not.eq() return call!.args[1] as string[] } @@ -219,10 +219,10 @@ describe('pg:push', function () { const dumpArgs = getSpawnArgs('pg_dump') expect(dumpArgs).to.include('--exclude-table-data=logs') - expect(dumpArgs[dumpArgs.length - 1]).to.eq('localdb') + expect(dumpArgs.at(-1)).to.eq('localdb') const restoreArgs = getSpawnArgs('pg_restore') - expect(restoreArgs[restoreArgs.length - 1]).to.eq('mydb') + expect(restoreArgs.at(-1)).to.eq('mydb') }) it('keeps the database name last when excluding multiple tables', async () => { @@ -237,10 +237,10 @@ describe('pg:push', function () { const dumpArgs = getSpawnArgs('pg_dump') expect(dumpArgs.some(arg => arg.includes('--exclude-table-data=logs'))).to.eq(true) expect(dumpArgs.some(arg => arg.includes('--exclude-table-data=events'))).to.eq(true) - expect(dumpArgs[dumpArgs.length - 1]).to.eq('localdb') + expect(dumpArgs.at(-1)).to.eq('localdb') const restoreArgs = getSpawnArgs('pg_restore') - expect(restoreArgs[restoreArgs.length - 1]).to.eq('mydb') + expect(restoreArgs.at(-1)).to.eq('mydb') }) }) From 5db9f71949daa8005248c39951c5e84da8df5a86 Mon Sep 17 00:00:00 2001 From: Timothy Lowrimore Date: Thu, 8 Oct 2026 14:48:48 -0600 Subject: [PATCH 5/5] linter rule broke typechecker; disabling said linter rule --- eslint.config.js | 1 + test/unit/commands/pg/pull.unit.test.ts | 2 +- test/unit/commands/pg/push.unit.test.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 459500c36a..1b6b09526b 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -19,6 +19,7 @@ export default [ 'unicorn/no-array-for-each': 'warn', 'unicorn/no-array-reduce': 'warn', 'unicorn/no-empty-file': 'warn', + 'unicorn/no-useless-undefined': 'warn', 'unicorn/prefer-event-target': 'warn', 'unicorn/prefer-spread': 'warn', }, diff --git a/test/unit/commands/pg/pull.unit.test.ts b/test/unit/commands/pg/pull.unit.test.ts index 8aeed1f00c..cd42b17d37 100644 --- a/test/unit/commands/pg/pull.unit.test.ts +++ b/test/unit/commands/pg/pull.unit.test.ts @@ -106,7 +106,7 @@ describe('pg:pull', function () { const getDumpArgs = (): string[] => { const dumpCall = spawnStub.getCalls().find(call => call.args[0] === 'pg_dump') - expect(dumpCall, 'pg_dump should have been spawned').to.not.eq() + expect(dumpCall, 'pg_dump should have been spawned').to.not.eq(undefined) return dumpCall!.args[1] as string[] } diff --git a/test/unit/commands/pg/push.unit.test.ts b/test/unit/commands/pg/push.unit.test.ts index 60c881641d..270661fafe 100644 --- a/test/unit/commands/pg/push.unit.test.ts +++ b/test/unit/commands/pg/push.unit.test.ts @@ -204,7 +204,7 @@ describe('pg:push', function () { const getSpawnArgs = (command: string): string[] => { const call = spawnStub.getCalls().find(call => call.args[0] === command) - expect(call, `${command} should have been spawned`).to.not.eq() + expect(call, `${command} should have been spawned`).to.not.eq(undefined) return call!.args[1] as string[] }