Skip to content

[fix] Roll back partially-installed files when install() fails partway through #105

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

Merged
kmturley merged 1 commit into from
Jul 31, 2026
Merged

[fix] Roll back partially-installed files when install() fails partway through #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ Install a package by slug. Optionally including a version.
2. Otherwise (only installer-type files are available), if the program does not have Admin privileges, ask for elevated privileges to the filesystem before continuing
5. Download each matching file to a temporary directory.
6. Check the hash against the metadata sha256.
1. If hash and sha256 do not match, return error
1. If hash and sha256 do not match, remove any files/directories already moved into their final destination by an earlier iteration of steps 5-9 for this same install call, then return error. A package version's files should install as a single unit - a manager should never leave a package version partially installed (e.g. some of its files present in the final plugin/preset/project directory, others missing), since a later [scan](#scan-logic) would otherwise have no way to tell a partial, broken install apart from a complete one.
7. Check if the file type is installer
1. Run the installer process and wait for it to complete before continuing
2. When the process ends, run a local package scan to see if the installation finished correctly.
Expand Down
240 changes: 132 additions & 108 deletions src/classes/ManagerLocal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,128 +330,152 @@ export class ManagerLocal extends Manager {
versionNum,
);
dirCreate(dirDownloads);
for (const key in files) {
// Download file to temporary directory if not already downloaded.
const file: FileInterface = files[key];
const filePath: string = path.join(dirDownloads, path.basename(file.url));
if (!fileExists(filePath)) {
const fileBuffer: ArrayBuffer = await apiBuffer(file.url);
fileCreate(filePath, Buffer.from(fileBuffer));
}

// Check file hash matches expected hash.
const hash: string = await fileHash(filePath);
if (hash !== file.sha256) throw new Error(`${filePath} hash mismatch`);

// If installer, run the installer headless (without the user interface).
if (file.type === FileType.Installer) {
// Test time out if installing during tests.
if (isTests()) fileOpen(filePath);
else fileInstall(filePath);
// Currently we don't get a list of paths from the installer.
// Create empty directory and save package version information.
// Installers have to be manually uninstalled for now.
const dirTarget: string = path.join(this.typeDir, 'Installers', slug, versionNum);
dirCreate(dirTarget);
fileCreateJson(path.join(dirTarget, 'index.json'), pkgVersion);
}
// Every directory this call populates under `this.typeDir` (the live, user-facing install
// location that isPackageInstalled()/scan() look at) - tracked so that if a later file in
// this loop fails (hash mismatch, extraction error, ...), everything already installed for
// this call can be rolled back instead of leaving a partial install that looks installed but
// is actually missing files. Downloads/extraction happen in scratch temp directories outside
// typeDir and are deliberately left alone - see the download-caching note in
// ManagerLocal.test.ts.
const installedDirs = new Set<string>();
try {
for (const key in files) {
// Download file to temporary directory if not already downloaded.
const file: FileInterface = files[key];
const filePath: string = path.join(dirDownloads, path.basename(file.url));
if (!fileExists(filePath)) {
const fileBuffer: ArrayBuffer = await apiBuffer(file.url);
fileCreate(filePath, Buffer.from(fileBuffer));
}

// If archive, extract the archive to temporary directory, then move individual files.
if (file.type === FileType.Archive) {
const dirSource: string = path.join(
this.config.get('appDir') as string,
file.type,
this.type,
slug,
versionNum,
);
const dirSub: string = path.join(slug, versionNum);
let formatDir: Record<string, string> = pluginFormatDir;
if (this.type === RegistryType.Apps) formatDir = pluginFormatDir;
else if (this.type === RegistryType.Presets) formatDir = presetFormatDir;
else if (this.type === RegistryType.Projects) formatDir = projectFormatDir;
await archiveExtract(filePath, dirSource);

// Move entire directory, maintaining the same folder structure.
if (pkgVersion.type === PluginType.Sampler) {
const dirTarget: string = path.join(this.typeDir, 'Samplers', dirSub);
// Check file hash matches expected hash.
const hash: string = await fileHash(filePath);
if (hash !== file.sha256) throw new Error(`${filePath} hash mismatch`);

// If installer, run the installer headless (without the user interface).
if (file.type === FileType.Installer) {
// Test time out if installing during tests.
if (isTests()) fileOpen(filePath);
else fileInstall(filePath);
// Currently we don't get a list of paths from the installer.
// Create empty directory and save package version information.
// Installers have to be manually uninstalled for now.
const dirTarget: string = path.join(this.typeDir, 'Installers', slug, versionNum);
dirCreate(dirTarget);
dirMove(dirSource, dirTarget);
fileCreateJson(path.join(dirTarget, 'index.json'), pkgVersion);
} else {
// Check if archive contains installer files (pkg, dmg) that should be run
const allFiles = dirRead(`${dirSource}/**/*`).filter(f => !dirIs(f));
const installerFiles = allFiles.filter(f => {
const ext = path.extname(f).toLowerCase();
return ext === '.pkg' || ext === '.dmg';
});

if (installerFiles.length > 0) {
// Run installer files found in archive
for (const installerFile of installerFiles) {
if (isTests()) fileOpen(installerFile);
else fileInstall(installerFile);
}
// Create directory and save package info for installer
const dirTarget: string = path.join(this.typeDir, 'Installers', dirSub);
dirCreate(dirTarget);
fileCreateJson(path.join(dirTarget, 'index.json'), pkgVersion);
} else if (this.type === RegistryType.Plugins) {
// For plugins, move files into type-specific subdirectories
const filesMoved: string[] = filesMove(dirSource, this.typeDir, dirSub, formatDir);
if (filesMoved.length === 0) {
throw new Error(`No compatible files found to install for ${slug}`);
}
filesMoved.forEach((fileMoved: string) => {
const fileJson: string = path.join(path.dirname(fileMoved), 'index.json');
fileCreateJson(fileJson, pkgVersion);
});
} else {
// For apps/projects/presets, move entire directory without type subdirectories
const dirTarget: string = path.join(this.typeDir, dirSub);
installedDirs.add(dirTarget);
}

// If archive, extract the archive to temporary directory, then move individual files.
if (file.type === FileType.Archive) {
const dirSource: string = path.join(
this.config.get('appDir') as string,
file.type,
this.type,
slug,
versionNum,
);
const dirSub: string = path.join(slug, versionNum);
let formatDir: Record<string, string> = pluginFormatDir;
if (this.type === RegistryType.Apps) formatDir = pluginFormatDir;
else if (this.type === RegistryType.Presets) formatDir = presetFormatDir;
else if (this.type === RegistryType.Projects) formatDir = projectFormatDir;
await archiveExtract(filePath, dirSource);

// Move entire directory, maintaining the same folder structure.
if (pkgVersion.type === PluginType.Sampler) {
const dirTarget: string = path.join(this.typeDir, 'Samplers', dirSub);
dirCreate(dirTarget);
dirMove(dirSource, dirTarget);
fileCreateJson(path.join(dirTarget, 'index.json'), pkgVersion);
// Ensure executable permissions for likely executables inside moved app/project/preset
try {
const movedFiles = dirRead(path.join(dirTarget, '**', '*')).filter(f => !dirIs(f));
movedFiles.forEach((movedFile: string) => {
const ext = path.extname(movedFile).slice(1).toLowerCase();
if (['', 'elf', 'exe'].includes(ext)) {
try {
fileExec(movedFile);
} catch (err) {
this.log(`Failed to set exec on ${movedFile}:`, err);
}
}
installedDirs.add(dirTarget);
} else {
// Check if archive contains installer files (pkg, dmg) that should be run
const allFiles = dirRead(`${dirSource}/**/*`).filter(f => !dirIs(f));
const installerFiles = allFiles.filter(f => {
const ext = path.extname(f).toLowerCase();
return ext === '.pkg' || ext === '.dmg';
});

if (installerFiles.length > 0) {
// Run installer files found in archive
for (const installerFile of installerFiles) {
if (isTests()) fileOpen(installerFile);
else fileInstall(installerFile);
}
// Create directory and save package info for installer
const dirTarget: string = path.join(this.typeDir, 'Installers', dirSub);
dirCreate(dirTarget);
fileCreateJson(path.join(dirTarget, 'index.json'), pkgVersion);
installedDirs.add(dirTarget);
} else if (this.type === RegistryType.Plugins) {
// For plugins, move files into type-specific subdirectories
const filesMoved: string[] = filesMove(dirSource, this.typeDir, dirSub, formatDir);
if (filesMoved.length === 0) {
throw new Error(`No compatible files found to install for ${slug}`);
}
filesMoved.forEach((fileMoved: string) => {
const fileJson: string = path.join(path.dirname(fileMoved), 'index.json');
fileCreateJson(fileJson, pkgVersion);
// A single archive can contain multiple formats (e.g. VST3 and CLAP), moved into
// different formatDir subdirectories - track each one, not just the first.
installedDirs.add(path.dirname(fileMoved));
});
} catch (err) {
this.log('Error while setting executable permissions:', err);
}
// Also handle macOS .app bundles: set exec on binaries in Contents/MacOS
try {
const appDirs = dirRead(path.join(dirTarget, '**', '*.app')).filter(d => dirIs(d));
appDirs.forEach((appDir: string) => {
try {
const macosBinPattern = path.join(appDir, 'Contents', 'MacOS', '**', '*');
const macosFiles = dirRead(macosBinPattern).filter(f => !dirIs(f));
macosFiles.forEach((binFile: string) => {
} else {
// For apps/projects/presets, move entire directory without type subdirectories
const dirTarget: string = path.join(this.typeDir, dirSub);
dirCreate(dirTarget);
dirMove(dirSource, dirTarget);
fileCreateJson(path.join(dirTarget, 'index.json'), pkgVersion);
installedDirs.add(dirTarget);
// Ensure executable permissions for likely executables inside moved app/project/preset
try {
const movedFiles = dirRead(path.join(dirTarget, '**', '*')).filter(f => !dirIs(f));
movedFiles.forEach((movedFile: string) => {
const ext = path.extname(movedFile).slice(1).toLowerCase();
if (['', 'elf', 'exe'].includes(ext)) {
try {
fileExec(binFile);
fileExec(movedFile);
} catch (err) {
this.log(`Failed to set exec on app binary ${binFile}:`, err);
this.log(`Failed to set exec on ${movedFile}:`, err);
}
});
} catch (err) {
this.log(`Error scanning .app contents for ${appDir}:`, err);
}
});
} catch (err) {
this.log(err);
}
});
} catch (err) {
this.log('Error while setting executable permissions:', err);
}
// Also handle macOS .app bundles: set exec on binaries in Contents/MacOS
try {
const appDirs = dirRead(path.join(dirTarget, '**', '*.app')).filter(d => dirIs(d));
appDirs.forEach((appDir: string) => {
try {
const macosBinPattern = path.join(appDir, 'Contents', 'MacOS', '**', '*');
const macosFiles = dirRead(macosBinPattern).filter(f => !dirIs(f));
macosFiles.forEach((binFile: string) => {
try {
fileExec(binFile);
} catch (err) {
this.log(`Failed to set exec on app binary ${binFile}:`, err);
}
});
} catch (err) {
this.log(`Error scanning .app contents for ${appDir}:`, err);
}
});
} catch (err) {
this.log(err);
}
}
}
}
}
} catch (err) {
// Roll back everything this call already installed under typeDir before propagating -
// otherwise a package that failed partway through would be left looking installed (its
// version directory exists) while actually missing files, and neither isPackageInstalled()
// nor scan() would have any way to tell the difference.
for (const dir of installedDirs) dirDelete(dir);
throw err;
}
pkgVersion.installed = true;
return pkgVersion;
Expand Down
61 changes: 61 additions & 0 deletions tests/classes/ManagerLocal.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import { ConfigInterface } from '../../src/types/Config';
import { PackageVersion } from '../../src/types/Package';
import { Architecture } from '../../src/types/Architecture';
import { SystemType } from '../../src/types/SystemType';
import { FileType } from '../../src/types/FileType';
import { mockRegistrySync, omitDownloads } from '../testUtils';
import * as apiHelpers from '../../src/helpers/api';

const APP_DIR: string = 'test';
// Explicitly test-scoped rather than relying on Config.test.ts/ConfigLocal.test.ts to have
Expand Down Expand Up @@ -249,6 +251,65 @@ test('Install installer-only package still elevates when unprivileged', async ()
runCliAsAdminSpy.mockRestore();
});

test('Install rolls back already-installed files when a later file in the same version fails', async () => {
// Regression test for the "partial install" failure mode identified in review.md: a package
// version with two files, where the first installs successfully (creating and populating its
// target directory under typeDir) before the second fails its hash check. Without rollback,
// the first file's directory would be left behind, and isPackageInstalled()/scan() would treat
// this version as installed despite it actually being incomplete.
const slug = 'test-org/rollback-project';
const versionNum = '1.0.0';
const compatibleFile = {
architectures: [Architecture.Arm32, Architecture.Arm64, Architecture.X32, Architecture.X64],
size: 10,
systems: [{ type: SystemType.Linux }, { type: SystemType.Mac }, { type: SystemType.Win }],
type: FileType.Archive,
};
const pkgVersion: PackageVersion = {
...PROJECT,
files: [
{ ...compatibleFile, sha256: 'a'.repeat(64), url: 'https://example.invalid/rollback/file-one.zip' },
{ ...compatibleFile, sha256: 'b'.repeat(64), url: 'https://example.invalid/rollback/file-two.zip' },
],
};
const pkg = new Package(slug);
pkg.addVersion(versionNum, pkgVersion);

const manager = new ManagerLocal(RegistryType.Projects, CONFIG);
manager.addPackage(pkg);

const apiBufferSpy = vi.spyOn(apiHelpers, 'apiBuffer').mockResolvedValue(new Uint8Array([1, 2, 3]).buffer);
// The first file's hash matches what's configured above, so it clears the check and proceeds
// to install; the second never will, regardless of content - this forces the failure to land
// on the *second* file, after the first has already been fully installed.
const fileHashSpy = vi
.spyOn(fileHelpers, 'fileHash')
.mockResolvedValueOnce('a'.repeat(64))
.mockResolvedValueOnce('mismatched-hash');
// archiveExtract() itself isn't under test here - stub it to just produce a small real
// directory, so the real (unmocked) dirMove()/dirRead() logic downstream has something genuine
// to move for the first file.
const archiveExtractSpy = vi
.spyOn(fileHelpers, 'archiveExtract')
.mockImplementation(async (_filePath: string, dirPath: string) => {
dirCreate(dirPath);
fileCreateJson(path.join(dirPath, 'dummy.json'), { ok: true });
});

const dirTarget: string = path.join(CONFIG.projectsDir as string, slug, versionNum);

try {
await expect(manager.install(slug, versionNum)).rejects.toThrow('hash mismatch');

expect(dirExists(dirTarget)).toEqual(false);
expect(manager.isPackageInstalled(slug, versionNum)).toEqual(false);
} finally {
apiBufferSpy.mockRestore();
fileHashSpy.mockRestore();
archiveExtractSpy.mockRestore();
}
});

test('Install all installs every listed package', async () => {
const manager = new ManagerLocal(RegistryType.Plugins, CONFIG);
// Seed a single known package directly rather than sync()'ing the live registry - installAll()
Expand Down
Loading