diff --git a/src/commands/apps/destroy.ts b/src/commands/apps/destroy.ts index 229ce44128..e60abf68fc 100644 --- a/src/commands/apps/destroy.ts +++ b/src/commands/apps/destroy.ts @@ -2,7 +2,7 @@ import {Command, flags} from '@heroku-cli/command' import * as color from '@heroku/heroku-cli-util/color' import {Args, ux} from '@oclif/core' -import * as git from '../../lib/ci/git.js' +import {gitService} from '../../lib/ci/git.js' import ConfirmCommand from '../../lib/confirm-command.js' export default class Destroy extends Command { @@ -35,13 +35,15 @@ export default class Destroy extends Command { * you want, and they can all point to the same url. * The only requirement is that the "name" is unique. */ - if (git.inGitRepo()) { + if (gitService.inGitRepo()) { // delete git remotes pointing to this app - const remotes = await git.listRemotes() - await Promise.all([ - remotes.get(git.gitUrl(app))?.map(({name}) => git.rmRemote(name)), - remotes.get(git.sshGitUrl(app))?.map(({name}) => git.rmRemote(name)), + const remotes = await gitService.listRemotes() + // Deduplicate remote names (same name appears for fetch and push) + const names = new Set([ + ...(remotes.get(gitService.gitUrl(app))?.map(({name}) => name) ?? []), + ...(remotes.get(gitService.sshGitUrl(app))?.map(({name}) => name) ?? []), ]) + await Promise.all([...names].map(name => gitService.rmRemote(name))) } ux.action.stop() diff --git a/src/lib/ci/git.ts b/src/lib/ci/git.ts index 394e090af3..6d369e64bb 100644 --- a/src/lib/ci/git.ts +++ b/src/lib/ci/git.ts @@ -162,7 +162,6 @@ async function createRemote(remote: string, url: string) { return null } -// GitService class for easier testing/stubbing export class GitService { async createArchive(ref: string) { return createArchive(ref) @@ -172,9 +171,29 @@ export class GitService { return githubRepository() } + gitUrl(app?: string) { + return gitUrl(app) + } + + inGitRepo() { + return inGitRepo() + } + + async listRemotes() { + return listRemotes() + } + async readCommit(commit: string) { return readCommit(commit) } + + async rmRemote(remote: string) { + return rmRemote(remote) + } + + sshGitUrl(app: string) { + return sshGitUrl(app) + } } // Export a shared instance for use across commands diff --git a/test/unit/commands/apps/destroy.unit.test.ts b/test/unit/commands/apps/destroy.unit.test.ts index fb5bc8d662..ff8c427308 100644 --- a/test/unit/commands/apps/destroy.unit.test.ts +++ b/test/unit/commands/apps/destroy.unit.test.ts @@ -1,8 +1,10 @@ import {runCommand} from '@heroku-cli/test-utils' import {expect} from 'chai' import nock from 'nock' +import {createSandbox} from 'sinon' import Destroy from '../../../../src/commands/apps/destroy.js' +import {gitService} from '../../../../src/lib/ci/git.js' describe('apps:destroy', function () { let api: nock.Scope @@ -43,4 +45,73 @@ describe('apps:destroy', function () { expect(error?.message).to.include('No app specified.') }) + + describe('git remote cleanup', function () { + const sandbox = createSandbox() + + afterEach(function () { + sandbox.restore() + }) + + it('removes duplicate git remotes without error (issue #3677)', async function () { + api + .get('/apps/myapp').reply(200, {name: 'myapp'}) + .delete('/apps/myapp').reply(200) + + const rmRemoteCalls: string[] = [] + + // Stub gitService methods + sandbox.stub(gitService, 'inGitRepo').returns(true) + // Return a map with duplicate entries (fetch + push for same remote) + const mockRemotes = new Map([ + ['https://git.heroku.com/myapp.git', [ + {kind: '(fetch)', name: 'heroku'}, + {kind: '(push)', name: 'heroku'}, + ]], + ]) + sandbox.stub(gitService, 'listRemotes').resolves(mockRemotes) + sandbox.stub(gitService, 'gitUrl').returns('https://git.heroku.com/myapp.git') + sandbox.stub(gitService, 'sshGitUrl').returns('git@git.heroku.com:myapp.git') + sandbox.stub(gitService, 'rmRemote').callsFake(async (name: string) => { + rmRemoteCalls.push(name) + }) + + await runCommand(Destroy, ['--app', 'myapp', '--confirm', 'myapp']) + + // Verify rmRemote was called exactly once (deduplication worked) + expect(rmRemoteCalls.length).to.equal(1) + expect(rmRemoteCalls[0]).to.equal('heroku') + }) + + it('removes multiple different remotes', async function () { + api + .get('/apps/myapp').reply(200, {name: 'myapp'}) + .delete('/apps/myapp').reply(200) + + const rmRemoteCalls: string[] = [] + + sandbox.stub(gitService, 'inGitRepo').returns(true) + // Multiple remotes with duplicates (fetch + push for each) + const mockRemotes = new Map([ + ['https://git.heroku.com/myapp.git', [ + {kind: '(fetch)', name: 'heroku'}, + {kind: '(push)', name: 'heroku'}, + {kind: '(fetch)', name: 'production'}, + {kind: '(push)', name: 'production'}, + ]], + ]) + sandbox.stub(gitService, 'listRemotes').resolves(mockRemotes) + sandbox.stub(gitService, 'gitUrl').returns('https://git.heroku.com/myapp.git') + sandbox.stub(gitService, 'sshGitUrl').returns('git@git.heroku.com:myapp.git') + sandbox.stub(gitService, 'rmRemote').callsFake(async (name: string) => { + rmRemoteCalls.push(name) + }) + + await runCommand(Destroy, ['--app', 'myapp', '--confirm', 'myapp']) + + // Verify both remotes were removed exactly once each + expect(rmRemoteCalls.length).to.equal(2) + expect(rmRemoteCalls).to.have.members(['heroku', 'production']) + }) + }) })