From 23b00a1c20e5e7f3347d0864099c3037337d159c Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Wed, 20 May 2026 14:37:15 +0200 Subject: [PATCH] fix(shutdown): keep the current volume status after shutdown cleanup --- .../lifecycle/__tests__/shutdown.test.ts | 4 +- app/server/modules/lifecycle/shutdown.ts | 2 +- .../volumes/__tests__/volumes.service.test.ts | 41 +++++++++++++++++-- app/server/modules/volumes/volume.service.ts | 16 ++++---- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/app/server/modules/lifecycle/__tests__/shutdown.test.ts b/app/server/modules/lifecycle/__tests__/shutdown.test.ts index 76a80474..11dbed84 100644 --- a/app/server/modules/lifecycle/__tests__/shutdown.test.ts +++ b/app/server/modules/lifecycle/__tests__/shutdown.test.ts @@ -58,7 +58,7 @@ describe("shutdown", () => { expect(updated!.status).toBe("mounted"); }); - test("keeps legacy controller-local fallback unmount on shutdown", async () => { + test("keeps mounted status while running the legacy controller-local fallback unmount on shutdown", async () => { config.flags.enableLocalAgent = false; const events: string[] = []; vi.spyOn(Scheduler, "stop").mockImplementation(async () => { @@ -85,6 +85,6 @@ describe("shutdown", () => { expect(events).toEqual(["scheduler.stop", "agents.stop"]); expect(runVolumeCommand).not.toHaveBeenCalled(); expect(updated).toBeDefined(); - expect(updated!.status).toBe("unmounted"); + expect(updated!.status).toBe("mounted"); }); }); diff --git a/app/server/modules/lifecycle/shutdown.ts b/app/server/modules/lifecycle/shutdown.ts index 53c14356..ba01018a 100644 --- a/app/server/modules/lifecycle/shutdown.ts +++ b/app/server/modules/lifecycle/shutdown.ts @@ -18,7 +18,7 @@ export const shutdown = async () => { for (const volume of volumes) { const { status, error } = await withContext({ organizationId: volume.organizationId }, () => - volumeService.unmountVolume(volume.shortId), + volumeService.unmountVolume(volume.shortId, { persistStatus: false }), ).catch((error) => ({ status: "error" as const, error: toMessage(error) })); logger.info(`Volume ${volume.name} unmount status: ${status}${error ? `, error: ${error}` : ""}`); diff --git a/app/server/modules/volumes/__tests__/volumes.service.test.ts b/app/server/modules/volumes/__tests__/volumes.service.test.ts index 9372df16..f401ba26 100644 --- a/app/server/modules/volumes/__tests__/volumes.service.test.ts +++ b/app/server/modules/volumes/__tests__/volumes.service.test.ts @@ -152,11 +152,38 @@ describe("volumeService.mountVolume", () => { }); }); +describe("volumeService.unmountVolume", () => { + test("persists the unmounted status for normal unmount requests", async () => { + const { organizationId, user } = await createTestSession(); + const volume = await createTestVolume({ organizationId, status: "mounted", agentId: "agent-1" }); + agentManagerMock.runVolumeCommand.mockResolvedValueOnce({ + name: "volume.unmount", + result: { status: "unmounted" }, + }); + + await withContext({ organizationId, userId: user.id }, async () => { + const result = await volumeService.unmountVolume(volume.shortId); + + expect(result.status).toBe("unmounted"); + expect(agentManagerMock.runVolumeCommand).toHaveBeenCalledWith( + volume.agentId, + expect.objectContaining({ name: "volume.unmount", volume: expect.objectContaining({ id: volume.id }) }), + ); + }); + + const updatedVolume = await db.query.volumesTable.findFirst({ where: { id: volume.id } }); + expect(updatedVolume?.status).toBe("unmounted"); + }); +}); + describe("volumeService.ensureHealthyVolume", () => { test("returns ready when the mounted volume passes its health check", async () => { const { organizationId, user } = await createTestSession(); const volume = await createTestVolume({ organizationId, status: "mounted", agentId: "agent-1" }); - agentManagerMock.runVolumeCommand.mockResolvedValue({ name: "volume.checkHealth", result: { status: "mounted" } }); + agentManagerMock.runVolumeCommand.mockResolvedValue({ + name: "volume.checkHealth", + result: { status: "mounted" }, + }); await withContext({ organizationId, userId: user.id }, async () => { const result = await volumeService.ensureHealthyVolume(volume.shortId); @@ -169,14 +196,22 @@ describe("volumeService.ensureHealthyVolume", () => { expect(agentManagerMock.runVolumeCommand).toHaveBeenCalledOnce(); expect(agentManagerMock.runVolumeCommand).toHaveBeenCalledWith( volume.agentId, - expect.objectContaining({ name: "volume.checkHealth", volume: expect.objectContaining({ id: volume.id }) }), + expect.objectContaining({ + name: "volume.checkHealth", + volume: expect.objectContaining({ id: volume.id }), + }), ); }); }); test("auto-remounts when the mounted volume fails its health check", async () => { const { organizationId, user } = await createTestSession(); - const volume = await createTestVolume({ organizationId, status: "mounted", autoRemount: true, agentId: "agent-1" }); + const volume = await createTestVolume({ + organizationId, + status: "mounted", + autoRemount: true, + agentId: "agent-1", + }); agentManagerMock.runVolumeCommand .mockResolvedValueOnce({ name: "volume.checkHealth", result: { status: "error", error: "stale mount" } }) .mockResolvedValueOnce({ name: "volume.unmount", result: { status: "unmounted" } }) diff --git a/app/server/modules/volumes/volume.service.ts b/app/server/modules/volumes/volume.service.ts index 6e7b9c2b..bdb843ed 100644 --- a/app/server/modules/volumes/volume.service.ts +++ b/app/server/modules/volumes/volume.service.ts @@ -174,7 +174,7 @@ const mountVolume = async (shortId: ShortId) => { return { error, status }; }; -const unmountVolume = async (shortId: ShortId) => { +const unmountVolume = async (shortId: ShortId, options?: { persistStatus?: boolean }) => { const organizationId = getOrganizationId(); const volume = await findVolume(shortId); @@ -184,13 +184,15 @@ const unmountVolume = async (shortId: ShortId) => { const { status, error } = await runVolumeBackendCommand(volume, "volume.unmount"); - await db - .update(volumesTable) - .set({ status }) - .where(and(eq(volumesTable.id, volume.id), eq(volumesTable.organizationId, organizationId))); + if (options?.persistStatus !== false) { + await db + .update(volumesTable) + .set({ status }) + .where(and(eq(volumesTable.id, volume.id), eq(volumesTable.organizationId, organizationId))); - if (status === "unmounted") { - serverEvents.emit("volume:unmounted", { organizationId, volumeName: volume.name }); + if (status === "unmounted") { + serverEvents.emit("volume:unmounted", { organizationId, volumeName: volume.name }); + } } return { error, status };