Skip to content

Record unreconciled authorization outcomes and audit events atomically - #895

Open
rudycelekli wants to merge 1 commit into
CopilotKit:mainfrom
rudycelekli:fix/audit-unreconciled-authorizations
Open

rudycelekli wants to merge 1 commit into
CopilotKit:mainfrom
rudycelekli:fix/audit-unreconciled-authorizations

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 30, 2026 •

Copy link
Copy Markdown

What this changes

When a vendor authorization answer cannot be reconciled, the attempt currently settles as unreconciled without a persisted audit event. Operators can inspect the attempt row, but the authorization audit trail cannot explain that ending.

Record mcp.authorization_unreconciled atomically with the winning terminal transition. The event correlates actor, server/app, action, holder and attempt creation time; it excludes the live authorization ID, consent URL and vendor account identifier. An audit insert failure rolls back the outcome and completion timestamp. A late or concurrent losing terminal callback emits no event.

This addresses the shared unreconciled transition prioritized in #876. It is a partial implementation: the other lifecycle audit gaps remain open. Successful completion and cancellation events keep their current behavior.

Where it runs

OpenBot is deployed as several server processes behind a load balancer, serving a whole company.
Consecutive requests from the same person reach different processes, and the process that answered a
WebSocket upgrade is rarely the one that answers the next call on that conversation.

State that outlives a single request therefore has to be shared, or the change works on one machine
and stops working the moment there are two, without saying so. That failure is worse than not
shipping the feature: it passes review, passes CI, passes a local demo, and only surfaces as a Bot
that forgets, a question nobody can answer, or a boundary that never fires.

Answer these even when the answer is "none":

  • New state that outlives a request? The new event is a row in the existing Postgres audit table, committed with the existing pending-authorization row. No process-local state or schema migration.
  • What happens on the second replica? Its callback competes for the same pending-authorization row. If another replica already settled it, the conditional update returns no row and this callback writes no event.
  • Anything serialised? The existing actor/auth-ID match and completed_at IS NULL conditional UPDATE retain first-terminal-wins under Postgres row locking. The winning update and audit insert share one transaction; this is not a check-then-write.
  • Anything fanned out to a browser? None. The existing audit reader reads the new persisted event; no socket protocol or fan-out is added.
  • New listener, port, or schedule? None.

Postgres is already there and is the default answer to all of the above: a table, a unique index, a
conditional update, LISTEN/NOTIFY for fan-out.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act. No vendor call or gateway decision path is changed.
  • New refusals and new failures each write a row. This adds the previously missing unreconciled-ending event, without introducing new refusals. If audit persistence fails, the transition rejects and rolls back instead of committing an unaudited ending.
  • Nothing new is trusted from the client that the server can resolve itself. Correlation comes from the updated attempt row and its server record, not a new client payload.

Changelog

  • A line in CHANGELOG.md under Unreleased describes the audit event and rollback behavior.

Proof

Native before/after proof uses Bun 1.4.3 and disposable PostgreSQL 17 with the real plugin store and persisted audit table:

  • Before the production change: four behavioral regression cases executed; three failed for the missing event/absent audit-write rollback, and the previously-settled control passed. The baseline did not fail from imports or test setup.
  • After the production change: the four new cases passed in the maintained transaction integration suite, including concurrent callbacks and rollback on injected audit insert failure. All 15 maintained transaction cases passed, including those four regressions. Eight server shards finished 6,169 passed, 18 skipped, and 0 failed. The broader bun run test:ci finished 11,017 passed, 214 skipped, and 0 failed across 886 files. Skipped live-vendor/deployment tests do not establish live acceptance.
  • bun run lint, typecheck, typecheck:tests, desktop/chart version checks, Drizzle consistency/drift checks, Linux startup control and bun run build passed. Final bun run format:check passed. Linux test/build evidence; final formatting check. The full-run tree differs only in formatter layout; both edited TypeScript files emit identical normalized JavaScript. Final source tree: 8cc20c84bd4bedf25eddd03ad9cceb25750bcaff. Platform/container matrices and upstream PR-head CI remain separate checks.

The change has no new visual surface. Native DB tests prove this terminal transition; they do not claim live vendor acceptance or resolution of every item in #876.

AI assistance: implementation, test drafting and independent patch review used AI coding assistants. The persisted behavior assertions and CI results above are executable evidence; this disclosure does not claim human-only authorship or a human review that has not occurred.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant