Record unreconciled authorization outcomes and audit events atomically

CHANGELOG.md

@@ -8,6 +8,10 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+- Authorization attempts whose vendor answer cannot be reconciled now leave a safe, correlated
+  audit event when they settle as unreconciled. If that event cannot be saved, the attempt remains
+  unsettled rather than losing the audit trail.
+
 **Before upgrading.** Sixteen things change for an existing deployment:
 - Bots now share one computer per person instead of each having its own. Migration
   `0069_isolated_bots` adds `agent_profiles.isolated`, off for every existing Bot. With the

server/src/audit.ts

@@ -217,6 +217,8 @@ export const auditEventTypes = [
    * thing they are entitled to see a record of. Carries the scope the vendor actually granted.
    */
   "mcp.account_connected",
+  /** A vendor authorization answer could not be reconciled with its pending attempt. */
+  "mcp.authorization_unreconciled",
   /*
    * One person's connector access retired, by them or on their behalf.
    *

server/src/plugins/plugin-store/context.ts

@@ -1,6 +1,7 @@
 import { connectAppRefusal } from "@openbot/shared/connect-prompt";
 import { and, asc, eq, inArray, isNull, sql } from "drizzle-orm";
-import { recordAuditEvent } from "../../audit";
+import { createAuditStore, recordAuditEvent } from "../../audit";
+import type { Database } from "../../db/client";
 import {
   type CredentialExecutor,
   CredentialUnusableError,
@@ -370,10 +371,11 @@ export function createStoreContext(options: PluginStoreOptions) {
         );
 
   /**
-   * The one statement behind `failAuthorizationAttempt`, `markAuthorizationUnreconciled` and
-   * `markAuthorizationWithdrawn`, so the three verdicts cannot drift into three row shapes.
-   * `completed_at` is truncated to milliseconds for the reason `claimAuthorizationAttempt`
-   * truncates `created_at`.
+   * The shared transition behind `failAuthorizationAttempt`, `markAuthorizationUnreconciled` and
+   * `markAuthorizationWithdrawn`, so the verdicts cannot drift into three row shapes.
+   * An unreconciled transition and its safe audit record commit together; a failed audit leaves
+   * the attempt unsettled. `completed_at` is truncated to milliseconds for the reason
+   * `claimAuthorizationAttempt` truncates `created_at`.
    *
    * CONDITIONAL ON `completed_at IS NULL`, EXACTLY AS THE COMPLETION ABOVE IS, BECAUSE A SETTLED
    * ATTEMPT IS A RECORD AND NOT A DRAFT. Without that condition this UPDATE matched on
@@ -404,21 +406,56 @@ export function createStoreContext(options: PluginStoreOptions) {
     input: { actorId: string; authId: string },
     outcome: "failed" | "unreconciled" | "withdrawn",
   ) => {
-    const written = await database
-      .update(pendingAuthorizations)
-      .set({
-        completedAt: sql`date_trunc('milliseconds', now())`,
-        outcome,
-      })
-      .where(
-        and(
-          eq(pendingAuthorizations.actorId, input.actorId),
-          eq(pendingAuthorizations.authId, input.authId),
-          isNull(pendingAuthorizations.completedAt),
-        ),
-      )
-      .returning({ actorId: pendingAuthorizations.actorId });
-    return written.length > 0;
+    return await database.transaction(async (transaction) => {
+      const [attempt] = await transaction
+        .update(pendingAuthorizations)
+        .set({
+          completedAt: sql`date_trunc('milliseconds', now())`,
+          outcome,
+        })
+        .where(
+          and(
+            eq(pendingAuthorizations.actorId, input.actorId),
+            eq(pendingAuthorizations.authId, input.authId),
+            isNull(pendingAuthorizations.completedAt),
+          ),
+        )
+        .returning({
+          serverId: pendingAuthorizations.serverId,
+          action: pendingAuthorizations.action,
+          holder: pendingAuthorizations.holder,
+          createdAt: pendingAuthorizations.createdAt,
+        });
+      if (!attempt) return false;
+      if (outcome === "unreconciled") {
+        const [server] = await transaction
+          .select({ url: mcpServers.url })
+          .from(mcpServers)
+          .where(eq(mcpServers.id, attempt.serverId))
+          .limit(1);
+        const toolkit = server
+          ? toolkitOfFor(brokerProvider, server.url)
+          : null;
+        await recordAuditEvent(
+          createAuditStore(transaction as unknown as Database),
+          {
+            eventType: "mcp.authorization_unreconciled",
+            targetType: "mcp_server",
+            targetId: toolkit ?? attempt.serverId,
+            payload: {
+              actor: input.actorId,
+              server: toolkit,
+              serverId: attempt.serverId,
+              action: attempt.action,
+              holder: attempt.holder,
+              attemptStartedAt: attempt.createdAt.toISOString(),
+              outcome,
+            },
+          },
+        );
+      }
+      return true;
+    });
   };
 
   /**

server/tests/arcade-completion-transaction.integration.test.ts

@@ -573,3 +573,79 @@ describe("failAuthorizationAttempt / markAuthorizationUnreconciled", () => {
     expect([withdrew, unreconciled]).toEqual([false, false]);
   });
 });
+
+describe("unreconciled authorization audit", () => {
+  const eventsFor = async (actorId: string) => {
+    const rows = await database
+      .select()
+      .from(auditEvents)
+      .where(
+        and(
+          eq(auditEvents.eventType, "mcp.authorization_unreconciled"),
+          eq(auditEvents.targetId, serverId),
+        ),
+      );
+    return rows.filter((row) => row.payload.actor === actorId);
+  };
+
+  test("unreconciled audit: persists a safe event correlated with the closed attempt", async () => {
+    const a = await actor("audit-unreconciled");
+    const authId = "ar_must_not_reach_the_audit_payload";
+    const attempt = await attemptWith(a, authId);
+    expect(
+      await store.markAuthorizationUnreconciled({ actorId: a, authId }),
+    ).toBe(true);
+    const events = await eventsFor(a);
+    expect(events).toHaveLength(1);
+    expect(events[0]?.payload).toEqual({
+      actor: a,
+      server: null,
+      serverId,
+      action: "Google.SendEmail",
+      holder: "person",
+      attemptStartedAt: attempt.createdAt.toISOString(),
+      outcome: "unreconciled",
+    });
+    expect(JSON.stringify(events)).not.toContain(authId);
+    expect((await attemptRow(a)).outcome).toBe("unreconciled");
+  });
+
+  test("unreconciled audit: two concurrent terminal callbacks record one event", async () => {
+    const a = await actor("audit-concurrent");
+    const authId = "ar_concurrent_unreconciled";
+    await attemptWith(a, authId);
+    const outcomes = await Promise.all([
+      store.markAuthorizationUnreconciled({ actorId: a, authId }),
+      store.markAuthorizationUnreconciled({ actorId: a, authId }),
+    ]);
+    expect(outcomes.sort()).toEqual([false, true]);
+    expect(await eventsFor(a)).toHaveLength(1);
+  });
+
+  test("unreconciled audit: an audit insert failure leaves the attempt unsettled", async () => {
+    const a = await actor("audit-rollback");
+    const authId = "ar_rollback_unreconciled";
+    await attemptWith(a, authId);
+    await expect(
+      brokenAuditStore.markAuthorizationUnreconciled({ actorId: a, authId }),
+    ).rejects.toThrow("injected audit failure");
+    const after = await attemptRow(a);
+    expect(after.completedAt).toBeNull();
+    expect(after.outcome).toBeNull();
+    expect(await eventsFor(a)).toHaveLength(0);
+  });
+
+  test("unreconciled audit: a previously settled attempt emits no false event", async () => {
+    const a = await actor("audit-already-settled");
+    const authId = "ar_already_failed";
+    await attemptWith(a, authId);
+    expect(await store.failAuthorizationAttempt({ actorId: a, authId })).toBe(
+      true,
+    );
+    expect(
+      await store.markAuthorizationUnreconciled({ actorId: a, authId }),
+    ).toBe(false);
+    expect(await eventsFor(a)).toHaveLength(0);
+    expect((await attemptRow(a)).outcome).toBe("failed");
+  });
+});