From 208926a6eb5375f9d2e91eecb58c85dc28e86c77 Mon Sep 17 00:00:00 2001 From: Josh Pinkney Date: Wed, 16 Apr 2025 18:59:58 -0400 Subject: [PATCH 1/3] fix(amazonq): improve lsp cleanup logic --- packages/core/src/shared/lsp/utils/cleanup.ts | 15 ++++++++++++++- .../src/test/shared/lsp/utils/cleanup.test.ts | 8 ++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/core/src/shared/lsp/utils/cleanup.ts b/packages/core/src/shared/lsp/utils/cleanup.ts index 83b58e2bb8f..3f8c27cd81d 100644 --- a/packages/core/src/shared/lsp/utils/cleanup.ts +++ b/packages/core/src/shared/lsp/utils/cleanup.ts @@ -23,7 +23,11 @@ function isDelisted(manifestVersions: LspVersion[], targetVersion: string): bool * @param downloadDirectory * @returns array of deleted versions. */ -export async function cleanLspDownloads(manifestVersions: LspVersion[], downloadDirectory: string): Promise { +export async function cleanLspDownloads( + latestInstalledVersion: string, + manifestVersions: LspVersion[], + downloadDirectory: string +): Promise { const downloadedVersions = await getDownloadedVersions(downloadDirectory) const [delistedVersions, remainingVersions] = partition(downloadedVersions, (v: string) => isDelisted(manifestVersions, v) @@ -40,6 +44,15 @@ export async function cleanLspDownloads(manifestVersions: LspVersion[], download } for (const v of sort(remainingVersions).slice(0, -2)) { + /** + * When switching between different manifests, the following edge case can occur: + * A newly downloaded version might chronologically be older than all previously downloaded versions, + * even though it's marked as the latest version in its own manifest. + * In such cases, we skip the cleanup process to preserve this version. Otherwise we will get an EPIPE error + */ + if (v === latestInstalledVersion) { + continue + } await fs.delete(path.join(downloadDirectory, v), { force: true, recursive: true }) deletedVersions.push(v) } diff --git a/packages/core/src/test/shared/lsp/utils/cleanup.test.ts b/packages/core/src/test/shared/lsp/utils/cleanup.test.ts index 98f37fff28f..1bbb472f916 100644 --- a/packages/core/src/test/shared/lsp/utils/cleanup.test.ts +++ b/packages/core/src/test/shared/lsp/utils/cleanup.test.ts @@ -41,7 +41,7 @@ describe('cleanLSPDownloads', function () { it('keeps two newest versions', async function () { await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath) - const deleted = await cleanLspDownloads([], installationDir.fsPath) + const deleted = await cleanLspDownloads('2.1.1', [], installationDir.fsPath) const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename) assert.strictEqual(result.length, 2) @@ -53,6 +53,7 @@ describe('cleanLSPDownloads', function () { it('deletes delisted versions', async function () { await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath) const deleted = await cleanLspDownloads( + '2.1.1', [{ serverVersion: '1.1.1', isDelisted: true, targets: [] }], installationDir.fsPath ) @@ -67,6 +68,7 @@ describe('cleanLSPDownloads', function () { it('handles case where less than 2 versions are not delisted', async function () { await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath) const deleted = await cleanLspDownloads( + '', [ { serverVersion: '1.1.1', isDelisted: true, targets: [] }, { serverVersion: '2.1.1', isDelisted: true, targets: [] }, @@ -83,7 +85,7 @@ describe('cleanLSPDownloads', function () { it('handles case where less than 2 versions exist', async function () { await fakeInstallVersions(['1.0.0'], installationDir.fsPath) - const deleted = await cleanLspDownloads([], installationDir.fsPath) + const deleted = await cleanLspDownloads('1.0.0', [], installationDir.fsPath) const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename) assert.strictEqual(result.length, 1) @@ -93,6 +95,7 @@ describe('cleanLSPDownloads', function () { it('does not install delisted version when no other option exists', async function () { await fakeInstallVersions(['1.0.0'], installationDir.fsPath) const deleted = await cleanLspDownloads( + '1.0.0', [{ serverVersion: '1.0.0', isDelisted: true, targets: [] }], installationDir.fsPath ) @@ -105,6 +108,7 @@ describe('cleanLSPDownloads', function () { it('ignores invalid versions', async function () { await fakeInstallVersions(['1.0.0', '.DS_STORE'], installationDir.fsPath) const deleted = await cleanLspDownloads( + '1.0.0', [{ serverVersion: '1.0.0', isDelisted: true, targets: [] }], installationDir.fsPath ) From 72d9c32762d3059fec65a966cb163e5959984687 Mon Sep 17 00:00:00 2001 From: Josh Pinkney Date: Wed, 16 Apr 2025 19:08:47 -0400 Subject: [PATCH 2/3] oops --- packages/core/src/shared/lsp/baseLspInstaller.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/src/shared/lsp/baseLspInstaller.ts b/packages/core/src/shared/lsp/baseLspInstaller.ts index 12b5cdb9341..6c37e0b13fd 100644 --- a/packages/core/src/shared/lsp/baseLspInstaller.ts +++ b/packages/core/src/shared/lsp/baseLspInstaller.ts @@ -51,7 +51,11 @@ export abstract class BaseLspInstaller Date: Wed, 16 Apr 2025 19:09:59 -0400 Subject: [PATCH 3/3] fix --- packages/core/src/test/shared/lsp/utils/cleanup.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/test/shared/lsp/utils/cleanup.test.ts b/packages/core/src/test/shared/lsp/utils/cleanup.test.ts index 1bbb472f916..ec2f5a32059 100644 --- a/packages/core/src/test/shared/lsp/utils/cleanup.test.ts +++ b/packages/core/src/test/shared/lsp/utils/cleanup.test.ts @@ -68,7 +68,7 @@ describe('cleanLSPDownloads', function () { it('handles case where less than 2 versions are not delisted', async function () { await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath) const deleted = await cleanLspDownloads( - '', + '1.0.1', [ { serverVersion: '1.1.1', isDelisted: true, targets: [] }, { serverVersion: '2.1.1', isDelisted: true, targets: [] },