fix: restore cached packages in apt install order (#228)

Restore extracted archives sorted by filename, so packages whose install
scripts depend on files from other packages could break. Restore now
follows the entry order in manifest_all.json, which comes from the apt
install output, and fails explicitly when an archive is missing.

Refs #196

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

src/cache.ts

@@ -8,7 +8,6 @@ import * as path from "node:path";
 import { ActionPackageNames, findInstallScript } from "./packages.js";
 import { Manifest, ManifestEntry } from "./manifest.js";
 import {
-  deserializePackageName,
   CommandRunner,
   PackageInfo,
   PackageName,
@@ -116,10 +115,7 @@ export class Cache {
     this.logger = logger;
   }
 
-  private async runInstallScripts(archiveFilename: string): Promise<void> {
-    const serializePackageName = archiveFilename.replace(/\.tar$/, "");
-    const packageName = deserializePackageName(serializePackageName)!;
-
+  private async runInstallScripts(packageName: PackageName): Promise<void> {
     const runScript = async (
       packageName: PackageName,
       scriptType: "preinst" | "postinst",
@@ -186,22 +182,37 @@ export class Cache {
       await logFileError(`Manifest file not found at ${manifestPath}`);
       return undefined;
     }
-    const archives = await fs
-      .readdir(this.path)
-      .then((entries: string[]) =>
-        entries.filter((entry) => entry.endsWith(".tar")),
-      )
-      .then((entries: string[]) => entries.sort());
-
-    if (archives.length === 0) {
+    const manifestAllPath = path.join(this.path, MANIFEST_ALL_FILENAME);
+    const manifestAll = await Manifest.readFromFile(manifestAllPath).catch(
+      () => undefined,
+    );
+    if (!manifestAll || manifestAll.entries.length === 0) {
       await logFileError(
-        `No archive files found in cache directory ${this.path}`,
+        `Manifest of all packages not found or empty at ${manifestAllPath}`,
       );
       return undefined;
     }
 
-    for (const archiveFilename of archives) {
-      const archivePath = path.join(this.path, archiveFilename);
+    // Restore in the order apt installed the packages (i.e. the order of the
+    // manifest entries) and not by archive filename. Install scripts can depend
+    // on files from packages installed before them (see issue #196).
+    const archivePaths = manifestAll.entries.map((entry) => ({
+      packageName: entry.packageName,
+      archivePath: path.join(this.path, entry.packageName.serialize() + ".tar"),
+    }));
+    for (const { archivePath } of archivePaths) {
+      if (
+        !(await fs
+          .access(archivePath, fs.constants.R_OK)
+          .then(() => true)
+          .catch(() => false))
+      ) {
+        await logFileError(`Archive file not found at ${archivePath}`);
+        return undefined;
+      }
+    }
+
+    for (const { packageName, archivePath } of archivePaths) {
       await tar.extract({
         cwd: "/",
         file: archivePath,
@@ -209,7 +220,7 @@ export class Cache {
       });
 
       if (executeInstallScripts) {
-        await this.runInstallScripts(archiveFilename);
+        await this.runInstallScripts(packageName);
       }
     }
 

test/cache-restore.test.ts

@@ -0,0 +1,106 @@
+import fs from "node:fs";
+import path from "node:path";
+import * as ghcache from "@actions/cache";
+import * as tar from "tar";
+import { createPackageName } from "ts-apt";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import winston from "winston";
+
+import {
+  Cache,
+  CacheKey,
+  CACHE_DEFAULT_DIRNAME,
+  MANIFEST_ALL_FILENAME,
+  MANIFEST_MAIN_FILENAME,
+} from "../src/cache.js";
+import { Manifest, ManifestEntry } from "../src/manifest.js";
+import { ActionPackageNames } from "../src/packages.js";
+
+// Cache resolves its directory from os.homedir(), so point it at a temp dir.
+const { HOME_DIR } = await vi.hoisted(async () => {
+  const nodeFs = await import("node:fs");
+  const nodeOs = await import("node:os");
+  const nodePath = await import("node:path");
+  return {
+    HOME_DIR: nodeFs.mkdtempSync(
+      nodePath.join(nodeOs.tmpdir(), "cache-restore-test-"),
+    ),
+  };
+});
+
+vi.mock("node:os", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:os")>();
+  return { ...actual, homedir: () => HOME_DIR };
+});
+vi.mock("@actions/cache", () => ({ restoreCache: vi.fn() }));
+vi.mock("tar", () => ({ extract: vi.fn(), create: vi.fn() }));
+
+const CACHE_DIR = path.join(HOME_DIR, CACHE_DEFAULT_DIRNAME);
+const REQUESTED_PACKAGE = createPackageName("app", "1.0");
+// Install order from apt, deliberately not alphabetical.
+const INSTALL_ORDER = [
+  createPackageName("zlib1g", "1.2"),
+  createPackageName("libfoo", "2.0"),
+  REQUESTED_PACKAGE,
+];
+const KEY = new CacheKey(
+  "v1",
+  "1",
+  "x86_64",
+  ActionPackageNames.fromInput(REQUESTED_PACKAGE.serialize()),
+);
+
+const archivePath = (packageName: { serialize(): string }) =>
+  path.join(CACHE_DIR, packageName.serialize() + ".tar");
+
+async function writeCache(archivedPackages = INSTALL_ORDER): Promise<void> {
+  fs.mkdirSync(CACHE_DIR, { recursive: true });
+  const created = new Date(0);
+  await new Manifest(
+    created,
+    [new ManifestEntry(REQUESTED_PACKAGE)],
+    KEY,
+  ).writeToFile(path.join(CACHE_DIR, MANIFEST_MAIN_FILENAME));
+  await new Manifest(
+    created,
+    INSTALL_ORDER.map((pkg) => new ManifestEntry(pkg)),
+    KEY,
+  ).writeToFile(path.join(CACHE_DIR, MANIFEST_ALL_FILENAME));
+  archivedPackages.forEach((pkg) => fs.writeFileSync(archivePath(pkg), ""));
+}
+
+function createCache(): Cache {
+  const logger = winston.createLogger({ silent: true });
+  return new Cache({ run: vi.fn() } as never, logger);
+}
+
+describe("Cache.loadAndRestore", () => {
+  beforeEach(() => {
+    vi.mocked(ghcache.restoreCache).mockResolvedValue(KEY.hash);
+  });
+
+  afterEach(() => {
+    fs.rmSync(CACHE_DIR, { recursive: true, force: true });
+    vi.clearAllMocks();
+  });
+
+  it("loadAndRestore cache hit extracts archives in install order", async () => {
+    await writeCache();
+
+    const manifest = await createCache().loadAndRestore(KEY, false);
+
+    expect(manifest).toBeDefined();
+    expect(
+      vi.mocked(tar.extract).mock.calls.map(([options]) => options?.file),
+    ).toEqual(INSTALL_ORDER.map(archivePath));
+  });
+
+  it("loadAndRestore archive missing returns undefined without extracting", async () => {
+    await writeCache(INSTALL_ORDER.slice(1));
+
+    const manifest = await createCache().loadAndRestore(KEY, false);
+
+    expect(manifest).toBeUndefined();
+    expect(tar.extract).not.toHaveBeenCalled();
+  });
+});