From e4499c395f3dce5b276b964f33f24575d070e03d Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Sat, 9 May 2026 11:26:34 +0200 Subject: [PATCH] chore: pr feedbacks --- app/server/modules/agents/agents-manager.ts | 8 +++++-- .../lifecycle/__tests__/shutdown.test.ts | 22 +++++++++++++++---- app/server/modules/volumes/volume.service.ts | 3 +-- apps/agent/src/commands/volume.test.ts | 2 +- .../src/volume-host/__tests__/cleanup.test.ts | 8 +++---- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/app/server/modules/agents/agents-manager.ts b/app/server/modules/agents/agents-manager.ts index 9e63d9bd..e93fe451 100644 --- a/app/server/modules/agents/agents-manager.ts +++ b/app/server/modules/agents/agents-manager.ts @@ -90,7 +90,9 @@ const getActiveBackupRun = (jobId: string, scheduleId: string, eventName: string } if (activeBackupRun.scheduleShortId !== scheduleId) { - logger.warn(`Ignoring ${eventName} for job ${jobId} due to schedule mismatch ${scheduleId} from agent ${agentId}`); + logger.warn( + `Ignoring ${eventName} for job ${jobId} due to schedule mismatch ${scheduleId} from agent ${agentId}`, + ); return null; } @@ -308,7 +310,9 @@ export const startLocalAgent = async () => { await spawnLocalAgentProcess(runtime); if (!runtime.agentManager) { - return; + throw new Error( + `startLocalAgent spawned ${LOCAL_AGENT_ID} via spawnLocalAgentProcess, but runtime.agentManager is missing; waitForAgentReady cannot check readiness`, + ); } if (!(await runtime.agentManager.waitForAgentReady(LOCAL_AGENT_ID))) { diff --git a/app/server/modules/lifecycle/__tests__/shutdown.test.ts b/app/server/modules/lifecycle/__tests__/shutdown.test.ts index afb523ba..76a80474 100644 --- a/app/server/modules/lifecycle/__tests__/shutdown.test.ts +++ b/app/server/modules/lifecycle/__tests__/shutdown.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { Scheduler } from "../../../core/scheduler"; import * as bootstrapModule from "../bootstrap"; import { agentManager } from "../../agents/agents-manager"; @@ -12,8 +12,14 @@ const loadShutdownModule = async () => { return import(moduleUrl.href); }; +let originalEnableLocalAgent: boolean; + +beforeEach(() => { + originalEnableLocalAgent = config.flags.enableLocalAgent; +}); + afterEach(() => { - config.flags.enableLocalAgent = true; + config.flags.enableLocalAgent = originalEnableLocalAgent; vi.restoreAllMocks(); }); @@ -28,7 +34,7 @@ describe("shutdown", () => { }); const runVolumeCommand = vi.spyOn(agentManager, "runVolumeCommand"); - await createTestVolume({ + const volume = await createTestVolume({ name: "Shutdown test volume", config: { backend: "directory", @@ -47,6 +53,9 @@ describe("shutdown", () => { expect(events).toEqual(["scheduler.stop", "agents.stop"]); expect(runVolumeCommand).not.toHaveBeenCalled(); + const updated = await db.query.volumesTable.findFirst({ where: { id: volume.id } }); + expect(updated).toBeDefined(); + expect(updated!.status).toBe("mounted"); }); test("keeps legacy controller-local fallback unmount on shutdown", async () => { @@ -58,6 +67,9 @@ describe("shutdown", () => { vi.spyOn(bootstrapModule, "stopApplicationRuntime").mockImplementation(async () => { events.push("agents.stop"); }); + const runVolumeCommand = vi.spyOn(agentManager, "runVolumeCommand").mockImplementation(async () => { + throw new Error("runVolumeCommand should not be called during fallback shutdown"); + }); const volume = await createTestVolume({ name: "Fallback shutdown test volume", @@ -71,6 +83,8 @@ describe("shutdown", () => { const updated = await db.query.volumesTable.findFirst({ where: { id: volume.id } }); expect(events).toEqual(["scheduler.stop", "agents.stop"]); - expect(updated?.status).toBe("unmounted"); + expect(runVolumeCommand).not.toHaveBeenCalled(); + expect(updated).toBeDefined(); + expect(updated!.status).toBe("unmounted"); }); }); diff --git a/app/server/modules/volumes/volume.service.ts b/app/server/modules/volumes/volume.service.ts index 7eaf091b..570bc3d3 100644 --- a/app/server/modules/volumes/volume.service.ts +++ b/app/server/modules/volumes/volume.service.ts @@ -70,7 +70,6 @@ const volumeForAgent = async (volume: Volume): Promise => ({ const volumeForHost = async (volume: Volume): Promise => ({ ...volume, - shortId: volume.shortId, config: await decryptVolumeConfig(volume.config), provisioningId: volume.provisioningId ?? null, }); @@ -211,7 +210,7 @@ const getVolume = async (shortId: ShortId) => { let statfs: Partial = {}; if (volume.status === "mounted") { statfs = await withTimeout( - !shouldUseControllerLocalVolumeFallback(volume) + shouldRunViaAgent(volume) ? runVolumeCommand(volume.agentId, { name: "volume.statfs", volume: await volumeForAgent(volume), diff --git a/apps/agent/src/commands/volume.test.ts b/apps/agent/src/commands/volume.test.ts index 938a11aa..28c3fbdf 100644 --- a/apps/agent/src/commands/volume.test.ts +++ b/apps/agent/src/commands/volume.test.ts @@ -36,7 +36,7 @@ afterEach(() => { }); const runVolumeCommand = async (payload: VolumeCommandPayload) => { - const outboundMessages: string[] = []; + const outboundMessages: AgentWireMessage[] = []; const context = fromPartial({ offerOutbound: (message: AgentWireMessage) => Effect.sync(() => { diff --git a/apps/agent/src/volume-host/__tests__/cleanup.test.ts b/apps/agent/src/volume-host/__tests__/cleanup.test.ts index 1b191a5f..b80e3492 100644 --- a/apps/agent/src/volume-host/__tests__/cleanup.test.ts +++ b/apps/agent/src/volume-host/__tests__/cleanup.test.ts @@ -7,8 +7,8 @@ let tempRoot: string | undefined; let mockMountPoints: string[] = []; afterEach(async () => { - vi.doUnmock("./constants"); - vi.doUnmock("./fs"); + vi.doUnmock("../constants"); + vi.doUnmock("../fs"); vi.resetModules(); mockMountPoints = []; if (tempRoot) { @@ -20,8 +20,8 @@ afterEach(async () => { const loadCleanup = async () => { vi.resetModules(); tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-agent-cleanup-")); - vi.doMock("./constants", () => ({ VOLUME_MOUNT_BASE: tempRoot })); - vi.doMock("./fs", () => ({ + vi.doMock("../constants", () => ({ VOLUME_MOUNT_BASE: tempRoot })); + vi.doMock("../fs", () => ({ readMountInfo: async () => mockMountPoints.map((mountPoint) => ({ mountPoint, fstype: "fuse.sshfs" })), }));