Merge atomic environment qualification preservation

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

* codex/runner-native-platform-prerequisites:
  fix(environments): preserve measured proof atomically on metadata saves

server/src/__tests__/environment-instance-routes.test.ts

@@ -21,6 +21,7 @@ const mockEnvironmentService = vi.hoisted(() => ({
   list: vi.fn(),
   getById: vi.fn(),
   create: vi.fn(),
+  update: vi.fn(),
 }));
 
 const mockEnvironmentCustomImageService = vi.hoisted(() => ({
@@ -139,6 +140,16 @@ describe("environment instance routes", () => {
     expect(res.body.details?.code ?? res.body.code).toBe("environment_platform_probe_reserved");
     expect(mockEnvironmentService.create).not.toHaveBeenCalled();
   });
+  it.each([null, { userNote: "replacement" }])("passes explicit user metadata replacement through without copying an old probe (%j)", async metadata => {
+    const existing = createEnvironment({ metadata: { userNote: "old", paperclipSandboxPlatformProbes: { "company-1": { v: 1, marker: "stale" } } } });
+    mockEnvironmentService.getById.mockResolvedValue(existing);
+    mockEnvironmentService.update.mockResolvedValue({ ...existing, metadata });
+    const app = createApp({ type: "board", source: "local_implicit", isInstanceAdmin: true });
+    const res = await request(app).patch("/api/environments/env-1").send({ metadata });
+    expect(res.status).toBe(200);
+    expect(mockEnvironmentService.update).toHaveBeenCalledExactlyOnceWith("env-1", { metadata }, { db: expect.anything() });
+    expect(res.body.metadata).toEqual(metadata);
+  });
   beforeEach(() => {
     mockIssueService.clearExecutionWorkspaceEnvironmentSelection.mockReset();
     mockProjectService.clearExecutionWorkspaceEnvironmentSelection.mockReset();
@@ -148,6 +159,7 @@ describe("environment instance routes", () => {
     mockEnvironmentService.list.mockReset();
     mockEnvironmentService.getById.mockReset();
     mockEnvironmentService.create.mockReset();
+    mockEnvironmentService.update.mockReset();
     Object.values(mockEnvironmentCustomImageService).forEach((mock) => mock.mockReset());
     mockEnvironmentCustomImageService.getOverview.mockResolvedValue({
       activeTemplate: null,

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

@@ -1,6 +1,8 @@
 import { randomUUID } from "node:crypto";
 import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
-import { and, eq } from "drizzle-orm";
+import { and, eq, type SQL } from "drizzle-orm";
+import type { UpdateEnvironment } from "@paperclipai/shared";
+import { SANDBOX_PLATFORM_PROBE_METADATA_KEY } from "../services/environment-custom-image-runtime.js";
 import {
   activityLog,
   agents,
@@ -130,6 +132,65 @@ describeEmbeddedPostgres("environmentService leases", () => {
     return companyId;
   }
 
+  it("clears user metadata to null when no private platform proof exists", async () => {
+    const created = await svc.create({ name: "User metadata clear", driver: "sandbox", config: { provider: "daytona" }, metadata: { userNote: "remove" } });
+    expect((await svc.update(created.id, { metadata: null }))?.metadata).toBeNull();
+  });
+
+  it.each(["clear", "replace", "unrelated", "status", "config", "driver"] as const)(
+    "handles %s saves atomically after a concurrent platform probe record", async operation => {
+      const companyId = await seedCompany();
+      const config = { provider: "daytona", image: `registry.example.test/runtime@sha256:${"a".repeat(64)}` };
+      const staleProof = { v: 1, marker: "old" };
+      const freshProof = { v: 1, marker: "new" };
+      const created = await svc.create({ name: "Concurrent metadata", driver: "sandbox", config,
+        metadata: { userNote: "old", _paperclipManagedArchiveToken: "old-archive-token",
+          [SANDBOX_PLATFORM_PROBE_METADATA_KEY]: { [companyId]: staleProof } } });
+      const patches: Record<typeof operation, UpdateEnvironment> = {
+        clear: { metadata: null },
+        replace: { metadata: { userNote: "replacement", [SANDBOX_PLATFORM_PROBE_METADATA_KEY]: { [companyId]: staleProof } } },
+        unrelated: { description: "unrelated save" },
+        status: { status: "active" },
+        config: { config: { ...config, image: `registry.example.test/runtime@sha256:${"b".repeat(64)}` } },
+        driver: { driver: "ssh", config: { host: "fixture.example.test", username: "fixture" } },
+      };
+      // Interleave the real probe UPDATE after the save is prepared but before its
+      // real UPDATE executes. This reproduces the stale route-read/write window.
+      const delayedWriteDb = {
+        update(table: typeof environments) {
+          const prepared = db.update(table);
+          return {
+            set(values: Parameters<typeof prepared.set>[0]) {
+              const statement = prepared.set(values);
+              return {
+                where(condition: SQL) {
+                  const pending = statement.where(condition);
+                  return {
+                    async returning() {
+                      expect(await svc.recordSandboxPlatformProbe({
+                        environmentId: created.id, companyId, config, proof: freshProof,
+                      })).toBe(true);
+                      return pending.returning();
+                    },
+                  };
+                },
+              };
+            },
+          };
+        },
+      };
+      const saved = await svc.update(created.id, patches[operation], { db: delayedWriteDb as never });
+      if (operation === "config" || operation === "driver") {
+        expect(saved?.metadata).not.toHaveProperty(SANDBOX_PLATFORM_PROBE_METADATA_KEY);
+      } else {
+        expect(saved?.metadata?.[SANDBOX_PLATFORM_PROBE_METADATA_KEY]).toEqual({ [companyId]: freshProof });
+      }
+      if (operation === "clear") expect(saved?.metadata).toEqual({ [SANDBOX_PLATFORM_PROBE_METADATA_KEY]: { [companyId]: freshProof } });
+      else expect(saved?.metadata?.userNote).toBe(operation === "replace" ? "replacement" : "old");
+      if (operation !== "unrelated") expect(saved?.metadata).not.toHaveProperty("_paperclipManagedArchiveToken");
+    },
+  );
+
   it("acquires and releases a lease for a run", async () => {
     const { companyId, environmentId, runId } = await seedEnvironment();
 

server/src/routes/environments.ts

@@ -1216,14 +1216,6 @@ export function environmentRoutes(
     // Persist the config change and its binding rows atomically: a binding
     // ref that fails validation (e.g. a deleted secret) must roll the whole
     // save back instead of leaving the config re-pointed with stale bindings.
-    if (changesProviderTarget || patch.metadata !== undefined) {
-      const metadata = { ...(patch.metadata ?? existing.metadata ?? {}) };
-      if (changesProviderTarget) delete metadata[SANDBOX_PLATFORM_PROBE_METADATA_KEY];
-      else if (existing.metadata?.[SANDBOX_PLATFORM_PROBE_METADATA_KEY] !== undefined) {
-        metadata[SANDBOX_PLATFORM_PROBE_METADATA_KEY] = existing.metadata[SANDBOX_PLATFORM_PROBE_METADATA_KEY];
-      }
-      patch.metadata = metadata;
-    }
     const environment = await db.transaction(async (tx) => {
       const updated = await svc.update(existing.id, patch, { db: tx });
       if (!updated) return null;

server/src/services/environments.ts

@@ -1,7 +1,7 @@
 import { randomUUID } from "node:crypto";
 
 import { SANDBOX_PLATFORM_PROBE_METADATA_KEY } from "./environment-custom-image-runtime.js";
-import { and, desc, eq, inArray, isNull, ne, sql } from "drizzle-orm";
+import { and, desc, eq, inArray, isNull, ne, sql, type SQL } from "drizzle-orm";
 import type { Db } from "@paperclipai/db";
 import {
   agents,
@@ -1135,20 +1135,30 @@ export function environmentService(db: Db) {
       if ("envVars" in patch && patch.envVars !== undefined) {
         values.envVars = (patch.envVars ?? {}) as Record<string, unknown>;
       }
+      const changesTarget = patch.config !== undefined || patch.driver !== undefined;
+      let metadataUpdate: SQL | undefined;
       if (patch.metadata !== undefined) {
-        values.metadata = withoutManagedEnvironmentArchiveToken(patch.metadata ?? null);
-      } else if (patch.status !== undefined) {
-        const existingMetadata = await writeDb
-          .select({ metadata: environments.metadata })
-          .from(environments)
-          .where(eq(environments.id, id))
-          .then((rows) => rows[0]?.metadata ?? null);
-        values.metadata = withoutManagedEnvironmentArchiveToken(existingMetadata);
+        const userMetadata = withoutManagedEnvironmentArchiveToken(patch.metadata ?? null);
+        if (userMetadata) delete userMetadata[SANDBOX_PLATFORM_PROBE_METADATA_KEY];
+        if (changesTarget) {
+          values.metadata = userMetadata;
+        } else {
+          const replacement = userMetadata === null ? sql`null::jsonb` : sql`${JSON.stringify(userMetadata)}::jsonb`;
+          // Preserve the current private subtree in the same statement. A route's
+          // earlier read must never replace a newer concurrent probe receipt.
+          metadataUpdate = sql`case when ${environments.metadata} ? ${SANDBOX_PLATFORM_PROBE_METADATA_KEY}
+            then coalesce(${replacement}, '{}'::jsonb) || jsonb_build_object(
+              ${SANDBOX_PLATFORM_PROBE_METADATA_KEY}::text, ${environments.metadata}->${SANDBOX_PLATFORM_PROBE_METADATA_KEY})
+            else ${replacement} end`;
+        }
+      } else if (changesTarget || patch.status !== undefined) {
+        metadataUpdate = sql`${environments.metadata} - ${MANAGED_ENVIRONMENT_ARCHIVE_TOKEN_METADATA_KEY}`;
+        if (changesTarget) metadataUpdate = sql`${metadataUpdate} - ${SANDBOX_PLATFORM_PROBE_METADATA_KEY}`;
       }
 
       const row = await writeDb
         .update(environments)
-        .set(values)
+        .set({ ...values, ...(metadataUpdate ? { metadata: metadataUpdate } : {}) })
         .where(eq(environments.id, id))
         .returning()
         .then((rows) => rows[0] ?? null)