fix(adapters): preserve save results when audit handling fails

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

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

@@ -103,6 +103,11 @@ vi.mock("../services/instance-settings.js", () => ({
   instanceSettingsService: () => mockInstanceSettingsService,
 }));
 
+vi.mock("../services/adapter-settings.js", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../services/adapter-settings.js")>()),
+  adapterSettingsService: () => ({ getDisabledTypes: async () => [] }),
+}));
+
 vi.mock("../services/dot-runner-broker.js", () => ({
   dotRunnerBroker: () => ({ enabled: async () => true, bindingForAgent: async () => null }),
 }));

server/src/routes/adapters.ts

@@ -526,7 +526,13 @@ export function adapterRoutes(options: {
         }, publications);
       }
     });
-    for (const publication of publications) publishActivity(publication);
+    for (const publication of publications) {
+      try {
+        publishActivity(publication);
+      } catch (error) {
+        logger.error({ err: error, companyId: publication.companyId }, "failed to publish adapter settings activity");
+      }
+    }
 
     if (changed) {
       logger.info({ type: adapterType, disabled }, "Adapter enabled/disabled");

server/src/services/adapter-settings-local.test.ts

@@ -4,7 +4,7 @@ import path from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 vi.mock("../adapters/registry.js", () => ({ listServerAdapters: () => [{ type: "codex_local" }, { type: "claude_local" }] }));
-import { adapterSettingsService } from "./adapter-settings.js";
+import { adapterSettingsService, type AdapterSettingsDb } from "./adapter-settings.js";
 import { getDisabledAdapterTypes, setAdapterDisabled } from "./adapter-plugin-store.js";
 import { parseAdapterRegistryEnv } from "./adapter-registry-config.js";
 
@@ -48,4 +48,40 @@ describe("local adapter settings compatibility", () => {
     expect(read).not.toHaveBeenCalled();
     expect(getDisabledAdapterTypes()).toEqual(["codex_local"]);
   });
+
+  it("restores the saved choice when its audit fails and audits a later retry", async () => {
+    const transaction = vi.fn(async (callback: (tx: AdapterSettingsDb) => Promise<unknown>) => callback(db));
+    const db = { transaction } as unknown as AdapterSettingsDb;
+    const service = adapterSettingsService(db);
+    const audit = vi.fn().mockRejectedValueOnce(new Error("audit unavailable")).mockResolvedValue(undefined);
+
+    await expect(service.setDisabled("codex_local", true, audit)).rejects.toThrow("audit unavailable");
+    expect(await service.getDisabledTypes()).toEqual([]);
+    expect(JSON.parse(fs.readFileSync(path.join(directory, "adapter-settings.json"), "utf8"))).toEqual({ disabledTypes: [] });
+    expect(await service.setDisabled("codex_local", true, audit)).toBe(true);
+    expect(await service.getDisabledTypes()).toEqual(["codex_local"]);
+    expect(audit).toHaveBeenCalledTimes(2);
+    expect(transaction).toHaveBeenCalledTimes(2);
+  });
+
+  it("keeps a failed audit rollback from overwriting a concurrent save", async () => {
+    const db = { transaction: async (callback: (tx: AdapterSettingsDb) => Promise<unknown>) => callback(db) } as unknown as AdapterSettingsDb;
+    const first = adapterSettingsService(db);
+    const second = adapterSettingsService(db);
+    let failAudit!: (reason: Error) => void;
+    const auditWait = new Promise<void>((_resolve, reject) => { failAudit = reject; });
+    const audit = vi.fn(() => auditWait);
+    const rejected = expect(first.setDisabled("codex_local", true, audit)).rejects.toThrow("audit unavailable");
+    await vi.waitFor(() => expect(audit).toHaveBeenCalledOnce());
+    const nextAudit = vi.fn(async () => undefined);
+    const next = second.setDisabled("codex_local", true, nextAudit);
+    await Promise.resolve();
+    expect(nextAudit).not.toHaveBeenCalled();
+
+    failAudit(new Error("audit unavailable"));
+    await rejected;
+    expect(await next).toBe(true);
+    expect(await first.getDisabledTypes()).toEqual(["codex_local"]);
+    expect(nextAudit).toHaveBeenCalledOnce();
+  });
 });

server/src/services/adapter-settings-postgres.test.ts

@@ -28,6 +28,7 @@ import { adapterSettingsService, initializeAdapterSettings } from "./adapter-set
 import { adapterRoutes } from "../routes/adapters.js";
 import { errorHandler } from "../middleware/error-handler.js";
 import { resolveNewAgentRunnerForCompany } from "./agent-runner-selection.js";
+import * as activities from "./activity-log.js";
 
 const externalDatabaseUrl = process.env.PAPERCLIP_ADAPTER_SETTINGS_TEST_DATABASE_URL;
 const support = externalDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport();
@@ -60,6 +61,7 @@ const support = externalDatabaseUrl ? { supported: true } : await getEmbeddedPos
     vi.stubEnv("PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN", "synthetic-test-token");
   });
   afterEach(async () => {
+    vi.restoreAllMocks();
     vi.unstubAllEnvs();
     await fs.rm(directory, { recursive: true, force: true });
   });
@@ -88,6 +90,17 @@ const support = externalDatabaseUrl ? { supported: true } : await getEmbeddedPos
     expect(legacy.set).not.toHaveBeenCalled();
   });
 
+  it("acknowledges the committed save when its live activity event fails", async () => {
+    await service().seedIfAbsent({ disabledTypes: [] });
+    const publication = vi.spyOn(activities, "publishActivity").mockImplementation(() => { throw new Error("event unavailable"); });
+    const response = await request(app()).patch("/adapters/codex_local").send({ disabled: true });
+    expect(response.status, JSON.stringify(response.body)).toBe(200);
+    expect(publication).toHaveBeenCalled();
+    expect(await adapterSettingsService(peer, managed).getDisabledTypes()).toEqual(["codex_local"]);
+    const events = await peer.select().from(activityLog).where(eq(activityLog.action, "instance.adapter.updated"));
+    expect(events.some((event) => event.details?.adapterType === "codex_local")).toBe(true);
+  });
+
   it("preserves simultaneous first writes and concurrent removals from other connections", async () => {
     const writers = [service(), adapterSettingsService(peer, managed)];
     await Promise.all(Array.from({ length: 12 }, (_, index) => writers[index % 2].setDisabled(`adapter_${index}`, true)));

server/src/services/adapter-settings.ts

@@ -12,6 +12,7 @@ import { parseAdapterRegistryEnv } from "./adapter-registry-config.js";
 import { isCloudManagedInstance } from "./cloud-instance.js";
 
 const SINGLETON_KEY = "adapter-settings:v1";
+let localSettingsMutation: Promise<unknown> = Promise.resolve();
 type Transaction = Parameters<Parameters<Db["transaction"]>[0]>[0];
 export type AdapterSettingsDb = Pick<Db | Transaction, "select" | "insert" | "update" | "transaction">;
 export type StoredAdapterSettings = { version: 1; revision: number; disabledTypes: string[] };
@@ -75,9 +76,22 @@ export function adapterSettingsService(db: AdapterSettingsDb | undefined, option
         throw forbidden("Adapter availability is managed by the hosting operator", { code: "settings_operator_managed" });
       }
       if (!managed) {
-        const changed = setAdapterDisabled(type, disabled);
-        if (changed && db) await afterChange?.(db, { version: 1, revision: 0, disabledTypes: getDisabledAdapterTypes() });
-        return changed;
+        const mutation = localSettingsMutation.then(async () => {
+          const previous = getDisabledAdapterTypes().includes(type);
+          const changed = setAdapterDisabled(type, disabled);
+          if (!changed || !db || !afterChange) return changed;
+          try {
+            await db.transaction(async (tx) => {
+              await afterChange(tx, { version: 1, revision: 0, disabledTypes: getDisabledAdapterTypes() });
+            });
+          } catch (error) {
+            setAdapterDisabled(type, previous);
+            throw error;
+          }
+          return true;
+        });
+        localSettingsMutation = mutation.catch(() => undefined);
+        return mutation;
       }
       return requireDb().transaction(async (tx) => {
         await seedIfAbsent({ disabledTypes: [] }, tx);