feat: Overhaul Docker infrastructure with multi-stage builds, add a cleanup script, and refactor the update service to combine update and requirement checks.

2026-01-17 16:20:33 +01:00
parent d7543d9f48
commit 17e636c4e5
9 changed files with 496 additions and 214 deletions
--- a/shared/modules/admin/update.service.ts
+++ b/shared/modules/admin/update.service.ts
@@ -1,4 +1,4 @@
-import { exec } from "child_process";
+import { exec, type ExecException } from "child_process";
 import { promisify } from "util";
 import { writeFile, readFile, unlink } from "fs/promises";
 import { Client, TextChannel } from "discord.js";
@@ -10,32 +10,69 @@ const execAsync = promisify(exec);

 // Constants
 const STALE_CONTEXT_MS = 10 * 60 * 1000; // 10 minutes
+const DEFAULT_TIMEOUT_MS = 30_000; // 30 seconds for git commands
+const INSTALL_TIMEOUT_MS = 120_000; // 2 minutes for dependency installation
+const BUILD_TIMEOUT_MS = 180_000; // 3 minutes for web build

+/**
+ * Execute a command with timeout protection
+ */
+async function execWithTimeout(
+    cmd: string,
+    timeoutMs: number = DEFAULT_TIMEOUT_MS
+): Promise<{ stdout: string; stderr: string }> {
+    return new Promise((resolve, reject) => {
+        const process = exec(cmd, (error: ExecException | null, stdout: string, stderr: string) => {
+            if (error) {
+                reject(error);
+            } else {
+                resolve({ stdout, stderr });
+            }
+        });

+        const timer = setTimeout(() => {
+            process.kill("SIGTERM");
+            reject(new Error(`Command timed out after ${timeoutMs}ms: ${cmd}`));
+        }, timeoutMs);
+
+        process.on("exit", () => clearTimeout(timer));
+    });
+}

 export class UpdateService {
    private static readonly CONTEXT_FILE = ".restart_context.json";
    private static readonly ROLLBACK_FILE = ".rollback_commit.txt";

+    // Cache for rollback state (set when we save, cleared on cleanup)
+    private static rollbackPointExists: boolean | null = null;
+
    /**
     * Check for available updates with detailed commit information
+     * Optimized: Parallel git commands and combined requirements check
     */
-    static async checkForUpdates(): Promise<UpdateInfo> {
-        const { stdout: branchName } = await execAsync("git rev-parse --abbrev-ref HEAD");
+    static async checkForUpdates(): Promise<UpdateInfo & { requirements: UpdateCheckResult }> {
+        // Get branch first (needed for subsequent commands)
+        const { stdout: branchName } = await execWithTimeout("git rev-parse --abbrev-ref HEAD");
        const branch = branchName.trim();

-        const { stdout: currentCommit } = await execAsync("git rev-parse --short HEAD");
+        // Parallel execution: get current commit while fetching
+        const [currentResult] = await Promise.all([
+            execWithTimeout("git rev-parse --short HEAD"),
+            execWithTimeout(`git fetch origin ${branch} --prune`) // Only fetch current branch
+        ]);
+        const currentCommit = currentResult.stdout.trim();

-        await execAsync("git fetch --all");
+        // After fetch completes, get remote info in parallel
+        const [latestResult, logResult, diffResult] = await Promise.all([
+            execWithTimeout(`git rev-parse --short origin/${branch}`),
+            execWithTimeout(`git log HEAD..origin/${branch} --format="%h|%s|%an" --no-merges`),
+            execWithTimeout(`git diff HEAD..origin/${branch} --name-only`)
+        ]);

-        const { stdout: latestCommit } = await execAsync(`git rev-parse --short origin/${branch}`);
+        const latestCommit = latestResult.stdout.trim();

-        // Get commit log with author info
-        const { stdout: logOutput } = await execAsync(
-            `git log HEAD..origin/${branch} --format="%h|%s|%an" --no-merges`
-        );
-
-        const commits: CommitInfo[] = logOutput
+        // Parse commit log
+        const commits: CommitInfo[] = logResult.stdout
            .trim()
            .split("\n")
            .filter(line => line.length > 0)
@@ -44,51 +81,64 @@ export class UpdateService {
                return { hash: hash || "", message: message || "", author: author || "" };
            });

+        // Parse changed files and analyze requirements in one pass
+        const changedFiles = diffResult.stdout.trim().split("\n").filter(f => f.length > 0);
+        const requirements = this.analyzeChangedFiles(changedFiles);
+
        return {
            hasUpdates: commits.length > 0,
            branch,
-            currentCommit: currentCommit.trim(),
-            latestCommit: latestCommit.trim(),
+            currentCommit,
+            latestCommit,
            commitCount: commits.length,
-            commits
+            commits,
+            requirements
        };
    }

    /**
-     * Analyze what the update requires
+     * Analyze changed files to determine update requirements
+     * Extracted for reuse and clarity
+     */
+    private static analyzeChangedFiles(changedFiles: string[]): UpdateCheckResult {
+        const needsRootInstall = changedFiles.some(file =>
+            file === "package.json" || file === "bun.lock"
+        );
+
+        const needsWebInstall = changedFiles.some(file =>
+            file === "web/package.json" || file === "web/bun.lock"
+        );
+
+        // Only rebuild web if essential source files changed
+        const needsWebBuild = changedFiles.some(file =>
+            file.match(/^web\/src\/(components|pages|lib|index)/) ||
+            file === "web/build.ts" ||
+            file === "web/tailwind.config.ts" ||
+            file === "web/tsconfig.json"
+        );
+
+        const needsMigrations = changedFiles.some(file =>
+            file.includes("schema.ts") || file.startsWith("drizzle/")
+        );
+
+        return {
+            needsRootInstall,
+            needsWebInstall,
+            needsWebBuild,
+            needsMigrations,
+            changedFiles
+        };
+    }
+
+    /**
+     * @deprecated Use checkForUpdates() which now includes requirements
+     * Kept for backwards compatibility
     */
    static async checkUpdateRequirements(branch: string): Promise<UpdateCheckResult> {
        try {
-            const { stdout } = await execAsync(`git diff HEAD..origin/${branch} --name-only`);
+            const { stdout } = await execWithTimeout(`git diff HEAD..origin/${branch} --name-only`);
            const changedFiles = stdout.trim().split("\n").filter(f => f.length > 0);
-
-            const needsRootInstall = changedFiles.some(file =>
-                file === "package.json" || file === "bun.lock"
-            );
-
-            const needsWebInstall = changedFiles.some(file =>
-                file === "web/package.json" || file === "web/bun.lock"
-            );
-
-            // Detect if web source files changed (requires rebuild)
-            const needsWebBuild = changedFiles.some(file =>
-                file.startsWith("web/src/") ||
-                file === "web/build.ts" ||
-                file === "web/tailwind.config.ts" ||
-                file === "web/tsconfig.json"
-            );
-
-            const needsMigrations = changedFiles.some(file =>
-                file.includes("schema.ts") || file.startsWith("drizzle/")
-            );
-
-            return {
-                needsRootInstall,
-                needsWebInstall,
-                needsWebBuild,
-                needsMigrations,
-                changedFiles
-            };
+            return this.analyzeChangedFiles(changedFiles);
        } catch (e) {
            console.error("Failed to check update requirements:", e);
            return {
@@ -129,9 +179,10 @@ export class UpdateService {
     * Save the current commit for potential rollback
     */
    static async saveRollbackPoint(): Promise<string> {
-        const { stdout } = await execAsync("git rev-parse HEAD");
+        const { stdout } = await execWithTimeout("git rev-parse HEAD");
        const commit = stdout.trim();
        await writeFile(this.ROLLBACK_FILE, commit);
+        this.rollbackPointExists = true; // Cache the state
        return commit;
    }

@@ -141,8 +192,9 @@ export class UpdateService {
    static async rollback(): Promise<{ success: boolean; message: string }> {
        try {
            const rollbackCommit = await readFile(this.ROLLBACK_FILE, "utf-8");
-            await execAsync(`git reset --hard ${rollbackCommit.trim()}`);
+            await execWithTimeout(`git reset --hard ${rollbackCommit.trim()}`);
            await unlink(this.ROLLBACK_FILE);
+            this.rollbackPointExists = false;
            return { success: true, message: `Rolled back to ${rollbackCommit.trim().substring(0, 7)}` };
        } catch (e) {
            return {
@@ -154,12 +206,18 @@ export class UpdateService {

    /**
     * Check if a rollback point exists
+     * Uses cache when available to avoid file system access
     */
    static async hasRollbackPoint(): Promise<boolean> {
+        if (this.rollbackPointExists !== null) {
+            return this.rollbackPointExists;
+        }
        try {
            await readFile(this.ROLLBACK_FILE, "utf-8");
+            this.rollbackPointExists = true;
            return true;
        } catch {
+            this.rollbackPointExists = false;
            return false;
        }
    }
@@ -168,26 +226,32 @@ export class UpdateService {
     * Perform the git update
     */
    static async performUpdate(branch: string): Promise<void> {
-        await execAsync(`git reset --hard origin/${branch}`);
+        await execWithTimeout(`git reset --hard origin/${branch}`);
    }

    /**
     * Install dependencies for specified projects
+     * Optimized: Parallel installation
     */
    static async installDependencies(options: { root: boolean; web: boolean }): Promise<string> {
-        const outputs: string[] = [];
+        const tasks: Promise<{ label: string; output: string }>[] = [];

        if (options.root) {
-            const { stdout } = await execAsync("bun install");
-            outputs.push(`📦 Root: ${stdout.trim() || "Done"}`);
+            tasks.push(
+                execWithTimeout("bun install", INSTALL_TIMEOUT_MS)
+                    .then(({ stdout }) => ({ label: "📦 Root", output: stdout.trim() || "Done" }))
+            );
        }

        if (options.web) {
-            const { stdout } = await execAsync("cd web && bun install");
-            outputs.push(`🌐 Web: ${stdout.trim() || "Done"}`);
+            tasks.push(
+                execWithTimeout("cd web && bun install", INSTALL_TIMEOUT_MS)
+                    .then(({ stdout }) => ({ label: "🌐 Web", output: stdout.trim() || "Done" }))
+            );
        }

-        return outputs.join("\n");
+        const results = await Promise.all(tasks);
+        return results.map(r => `${r.label}: ${r.output}`).join("\n");
    }

    /**
@@ -228,7 +292,7 @@ export class UpdateService {
            }

            const result = await this.executePostRestartTasks(context, channel);
-            await this.notifyPostRestartResult(channel, result, context);
+            await this.notifyPostRestartResult(channel, result);
            await this.cleanupContext();
        } catch (e) {
            console.error("Failed to handle post-restart context:", e);
@@ -301,21 +365,33 @@ export class UpdateService {
            }
        };

-        // 1. Install Dependencies if needed
+        // 1. Install Dependencies if needed (PARALLELIZED)
        if (context.installDependencies) {
            try {
                progress.currentStep = "install";
                await updateProgress();

-                const { stdout: rootOutput } = await execAsync("bun install");
-                const { stdout: webOutput } = await execAsync("cd web && bun install");
+                // Parallel installation of root and web dependencies
+                const [rootResult, webResult] = await Promise.all([
+                    execWithTimeout("bun install", INSTALL_TIMEOUT_MS)
+                        .then(({ stdout }) => ({ success: true, output: stdout.trim() || "Done" }))
+                        .catch(err => ({ success: false, output: err instanceof Error ? err.message : String(err) })),
+                    execWithTimeout("cd web && bun install", INSTALL_TIMEOUT_MS)
+                        .then(({ stdout }) => ({ success: true, output: stdout.trim() || "Done" }))
+                        .catch(err => ({ success: false, output: err instanceof Error ? err.message : String(err) }))
+                ]);

-                result.installOutput = `📦 Root: ${rootOutput.trim() || "Done"}\n🌐 Web: ${webOutput.trim() || "Done"}`;
+                result.installSuccess = rootResult.success && webResult.success;
+                result.installOutput = `📦 Root: ${rootResult.output}\n🌐 Web: ${webResult.output}`;
                progress.installDone = true;
+
+                if (!result.installSuccess) {
+                    console.error("Dependency Install Failed:", result.installOutput);
+                }
            } catch (err: unknown) {
                result.installSuccess = false;
                result.installOutput = err instanceof Error ? err.message : String(err);
-                progress.installDone = true; // Mark as done even on failure
+                progress.installDone = true;
                console.error("Dependency Install Failed:", err);
            }
        }
@@ -326,7 +402,7 @@ export class UpdateService {
                progress.currentStep = "build";
                await updateProgress();

-                const { stdout } = await execAsync("cd web && bun run build");
+                const { stdout } = await execWithTimeout("cd web && bun run build", BUILD_TIMEOUT_MS);
                result.webBuildOutput = stdout.trim() || "Build completed successfully";
                progress.buildDone = true;
            } catch (err: unknown) {
@@ -343,7 +419,7 @@ export class UpdateService {
                progress.currentStep = "migrate";
                await updateProgress();

-                const { stdout } = await execAsync("bun x drizzle-kit migrate");
+                const { stdout } = await execWithTimeout("bun x drizzle-kit migrate", DEFAULT_TIMEOUT_MS);
                result.migrationOutput = stdout;
                progress.migrateDone = true;
            } catch (err: unknown) {
@@ -368,9 +444,9 @@ export class UpdateService {

    private static async notifyPostRestartResult(
        channel: TextChannel,
-        result: PostRestartResult,
-        context: RestartContext
+        result: PostRestartResult
    ): Promise<void> {
+        // Use cached rollback state - we just saved it before restart
        const hasRollback = await this.hasRollbackPoint();
        await channel.send(getPostRestartEmbed(result, hasRollback));
    }
@@ -381,5 +457,6 @@ export class UpdateService {
        } catch {
            // File may not exist, ignore
        }
+        // Don't clear rollback cache here - rollback file persists
    }
 }