diff --git a/server/src/__tests__/agent-test-environment-routes.test.ts b/server/src/__tests__/agent-test-environment-routes.test.ts index 4b5cf7b39d4..7a9088665ed 100644 --- a/server/src/__tests__/agent-test-environment-routes.test.ts +++ b/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()), + adapterSettingsService: () => ({ getDisabledTypes: async () => [] }), +})); + vi.mock("../services/dot-runner-broker.js", () => ({ dotRunnerBroker: () => ({ enabled: async () => true, bindingForAgent: async () => null }), })); diff --git a/server/src/routes/adapters.ts b/server/src/routes/adapters.ts index c878285fc99..406b59f16a4 100644 --- a/server/src/routes/adapters.ts +++ b/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"); diff --git a/server/src/services/adapter-settings-local.test.ts b/server/src/services/adapter-settings-local.test.ts index 0dec321c0b7..03e56fcb941 100644 --- a/server/src/services/adapter-settings-local.test.ts +++ b/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) => 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) => callback(db) } as unknown as AdapterSettingsDb; + const first = adapterSettingsService(db); + const second = adapterSettingsService(db); + let failAudit!: (reason: Error) => void; + const auditWait = new Promise((_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(); + }); }); diff --git a/server/src/services/adapter-settings-postgres.test.ts b/server/src/services/adapter-settings-postgres.test.ts index bb778f7159b..299c4b14f35 100644 --- a/server/src/services/adapter-settings-postgres.test.ts +++ b/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))); diff --git a/server/src/services/adapter-settings.ts b/server/src/services/adapter-settings.ts index ca014254338..4c2081c5578 100644 --- a/server/src/services/adapter-settings.ts +++ b/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 = Promise.resolve(); type Transaction = Parameters[0]>[0]; export type AdapterSettingsDb = Pick; 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);