test: cover concurrent setup expiry changes and startup ownership

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/__tests__/environment-custom-images-service.test.ts

@@ -1,3 +1,4 @@
+import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
 import { registerCustomImageCleanupWork } from "../services/custom-image-cleanup-work.js";
 import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js";
 import { idleWorkSnapshot } from "../services/task-admission.js";
@@ -30,7 +31,7 @@ import {
   startEmbeddedPostgresTestDatabase,
 } from "./helpers/embedded-postgres.js";
 
-const externalTestDatabase = process.env.PAPERCLIP_CUSTOM_IMAGE_TEST_DATABASE_URL;
+const externalTestDatabase = resolveExternalTestDatabaseUrl("PAPERCLIP_CUSTOM_IMAGE_TEST_DATABASE_URL");
 const embeddedPostgresSupport = externalTestDatabase ? { supported: true, reason: undefined } : await getEmbeddedPostgresTestSupport();
 const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
 
@@ -214,7 +215,6 @@ describeEmbeddedPostgres("environmentCustomImageService", () => {
     const errors: unknown[] = [];
 
     beforeEach(async () => {
-      // Open the pool before fake time can advance connection handshakes.
       await Promise.all(Array.from({ length: 6 }, () => db.execute(sql`select pg_sleep(0.01)`)));
       draining = false;
       errors.length = 0;
@@ -304,6 +304,33 @@ describeEmbeddedPostgres("environmentCustomImageService", () => {
       await quiet();
     });
 
+    it("retains a producer commit that arrives during the deadline read", async () => {
+      await start(); const created = await create(); await settle();
+      await coordinator!.stop();
+      let release!: () => void, reading!: () => void;
+      const gate = new Promise<void>(resolve => { release = resolve; });
+      const entered = new Promise<void>(resolve => { reading = resolve; });
+      const readDeadline = service.nextSetupCleanupAt;
+      vi.spyOn(service, "nextSetupCleanupAt").mockImplementationOnce(async () => {
+        const stale = await readDeadline();
+        reading(); await gate;
+        return stale;
+      });
+      const starting = start();
+      await entered;
+      const earlier = new Date(Date.now() + 5 * 60_000);
+      try {
+        provider.call.mockResolvedValueOnce({ providerLeaseId: created.session.providerLeaseId,
+          status: "waiting_for_user", expiresAt: earlier.toISOString() } as never);
+        await service.refreshSetupSession({ sessionId: created.session.id });
+      } finally { release(); }
+      await starting; await settle();
+      await vi.waitFor(() => expect(coordinator!.nextWakeAt()).toBe(earlier.getTime()));
+      await settle(5 * 60_000);
+      await vi.waitFor(() => expect(cancellations()).toHaveLength(1));
+      await quiet();
+    });
+
     it("reconstructs a saved expiry after restart", async () => {
       await start(); const created = await create(); await settle();
       await coordinator!.stop();
@@ -324,7 +351,6 @@ describeEmbeddedPostgres("environmentCustomImageService", () => {
         await vi.waitFor(() => expect(errors).toHaveLength(1)); errors.length = 0;
         draining = true;
       }
-      // Let the committed signal reconcile before observing passive drain.
       await settle();
       const reads = vi.spyOn(db, "select");
       await settle(2 * 60 * 60_000);

server/src/__tests__/server-startup-feedback-export.test.ts

@@ -129,6 +129,7 @@ const {
   };
   const issueThreadInteractionServiceFactoryMock = vi.fn(() => issueThreadInteractionServiceMock);
   const environmentCustomImagesServiceMock = {
+    nextSetupCleanupAt: vi.fn(async (): Promise<number | null> => null),
     cleanupExpiredSetupSessions: vi.fn(async () => ({ scanned: 0, timedOut: 0, failed: 0 })),
   };
   const environmentCustomImagesServiceFactoryMock = vi.fn(() => environmentCustomImagesServiceMock);
@@ -843,7 +844,8 @@ describe("startServer feedback export wiring", () => {
       expect(issueThreadInteractionServiceMock.sweepMergedPullRequestConfirmations).toHaveBeenCalledTimes(1);
       expect(executionWorkspaceServiceMock.sweepTerminalWorkspaces).toHaveBeenCalledTimes(1);
       expect(routineServiceMock.tickScheduledTriggers).toHaveBeenCalledTimes(1);
-      expect(environmentCustomImagesServiceMock.cleanupExpiredSetupSessions).toHaveBeenCalledTimes(2);
+      expect(environmentCustomImagesServiceMock.cleanupExpiredSetupSessions).toHaveBeenCalledTimes(1);
+      expect(deliveryWorkRegisterMock.mock.calls.some(([queue]) => queue === DELIVERY_QUEUES.customImageExpiry)).toBe(true);
     } finally {
       setIntervalSpy.mockRestore();
     }
@@ -960,10 +962,10 @@ describe("startServer feedback export wiring", () => {
     try {
       await expect(startServer()).resolves.toBeDefined();
       await settleStartupHousekeeping();
-      expect(vi.mocked(logger.error)).toHaveBeenCalledWith(
-        expect.objectContaining({ startupStep: "custom_image_setup_cleanup" }),
-        "startup housekeeping step failed",
-      );
+      const registration = deliveryWorkRegisterMock.mock.calls.find(([queue]) => queue === DELIVERY_QUEUES.customImageExpiry);
+      expect(registration).toBeDefined();
+      expect(await registration![1].hasPending!()).toBe(true);
+      expect(environmentCustomImagesServiceMock.nextSetupCleanupAt).not.toHaveBeenCalled();
       // The scheduler starts only after every housekeeping step settled.
       expect(heartbeatIntervals).toHaveLength(1);
     } finally {

server/src/services/custom-image-cleanup-work.ts

@@ -3,7 +3,6 @@ import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.
 import { DELIVERY_QUEUES } from "./delivery-work-notifications.js";
 import type { environmentCustomImageService } from "./environment-custom-images.js";
 
-/** Existing setup rows own expiry; provider operations remain in their service. */
 export function registerCustomImageCleanupWork(
   coordinator: ReturnType<typeof createDeliveryWorkCoordinator>,
   service: Pick<ReturnType<typeof environmentCustomImageService>, "cleanupExpiredSetupSessions" | "nextSetupCleanupAt">,

server/src/services/environment-custom-images.ts

@@ -291,8 +291,6 @@ export function environmentCustomImageService(
   const environments = environmentService(db);
   const secrets = secretService(db);
 
-  // This service owns all setup-session state and expiry changes. Keep wakes
-  // inside the existing outer transaction, including historical cleanup hints.
   function writeSetup<T>(write: (tx: Db) => PromiseLike<T>): Promise<T> {
     return db.transaction(async tx => {
       await notifyDeliveryWork(tx, DELIVERY_QUEUES.customImageExpiry);