fix(inbox): disable the inbox atomically and simplify its webhook tests

[waleedlatif1]

Follow-up audit of #6431 and #6436. Two findings, one behavioral and one test-quality.

Atomic disable

disableInbox deleted the webhook row and cleared the workspace inbox columns as two independent statements in a Promise.all. That was tolerable before, but workspace.inbox_provider_id is now uniquely indexed, which turns a half-applied disable into a fatal state: the workspace keeps the id of an AgentMail inbox that has already been deleted, that address becomes free at the provider, and the next workspace to claim the same username gets the same id back — so its enableInbox fails on the unique constraint and surfaces a raw Postgres error to the admin.

Narrow (needs a partial failure plus username reuse) and self-inflicted, but it is a state the index newly makes unrecoverable, so both writes now run in one transaction. The AgentMail API deletes still happen before it, so no external I/O is held inside the transaction.

Test simplification

• Dropped a ~35-line hand-rolled table fixture that schemaMock from @sim/testing already provides; the file now uses the repo idiom vi.mock('@sim/db', () => ({ ...dbChainMock, ...schemaMock })).
• Dropped a helper whose four queueTableRows calls were three-quarters no-ops — the shared chain mock already resolves unqueued chains to an empty array — along with its TSDoc, which was wrong about why the calls were needed.
• The unknown-inbox test previously queued an empty workspace result, which made it pass against the pre-fix code too. It now asserts the routed inbox id reaches the lookup, so it discriminates.

Verified by checking out the pre-fix route from main and running the suite against it: 6 of 7 tests fail. The seventh is the oversized-body test, which guards the shared body cap that predates all of this and correctly passes on both.

Audit findings that needed no change

Recorded so they are on file rather than re-derived later:

• No legitimate delivery that previously succeeded now fails. Traced enable, rename, entitlement lapse, and both partial-write windows. In the one window that looked like a regression (webhook row written, workspace row not yet updated) the old code verified the signature and then failed its inboxProviderId mismatch check — same 401, same provider retry.
• The unverified inbox id influences nothing but secret selection. After the verification gate it is never read again; every downstream field comes from the post-verification parse. All 401 paths return an identical status and body, and both branches pay the same indexed round-trip, so there is no enumeration oracle.
• Removing the old post-verification mismatch check is provably redundant — there is now a single JSON.parse, so the routing id and the message id are the same object property.
• Every writer of inbox_provider_id audited: only enableInbox and disableInbox. Workspace creation and forking both omit the column, and no script-migration touches it.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 8, 2026 8:39pm

[cursor]

PR Summary

Medium Risk
The transactional disable fixes a real consistency edge case around the unique inbox_provider_id index; scope is limited to inbox lifecycle and test refactors.

Overview
disableInbox no longer runs webhook deletion and workspace inbox column clears in parallel. Those DB writes now share a single transaction, so workspace.inboxProviderId cannot stay set after the webhook row is removed—a half-applied disable could strand a unique provider id and block another workspace from enabling the same AgentMail address.

The AgentMail webhook/inbox API deletes still run before the transaction, so external calls are not held inside it.

Webhook route tests drop a local table fixture and use shared schemaMock from @sim/testing. The accepted-delivery case queues only workspace and allowed-sender rows (empty inbox-task chains rely on the mock default). The unknown-inbox case now asserts the lookup uses the payload’s inbox_id, not just an empty workspace queue.

^{Reviewed by Cursor Bugbot for commit a1cef31. Configure here.}

[greptile-apps]

Greptile Summary

The PR makes the database portion of inbox disablement atomic and simplifies AgentMail webhook tests while strengthening the unknown-inbox routing assertion.

• Runs webhook-row deletion and workspace inbox-field clearing in one transaction.
• Replaces hand-built table fixtures with the shared schema mock.
• Removes redundant empty-result setup from accepted-delivery tests.
• Verifies that an unknown routed inbox ID reaches the workspace lookup.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable correctness, security, or test-quality regressions identified.

The transaction uses the repository’s supported callback pattern and transaction handle for both database writes, while the simplified tests preserve the required queued results and rely on the shared mock’s documented empty-array default.

Important Files Changed

Filename	Overview
apps/sim/lib/mothership/inbox/lifecycle.ts	Correctly groups the two related database mutations into one transaction without placing external API calls inside it.
apps/sim/app/api/webhooks/agentmail/route.test.ts	Correctly adopts shared database schema mocks, removes redundant queued results, and adds a discriminating lookup assertion.

Sequence Diagram

sequenceDiagram
  participant D as disableInbox
  participant A as AgentMail
  participant DB as Postgres
  D->>A: Delete remote webhook and inbox
  A-->>D: Deletion attempts settle
  D->>DB: Begin transaction
  D->>DB: Delete webhook row
  D->>DB: Clear workspace inbox fields
  alt Both writes succeed
    DB-->>D: Commit
  else Either write fails
    DB-->>D: Roll back both writes
  end

Loading

_{Reviews (1): Last reviewed commit: "fix(inbox): disable the inbox atomically..." | Re-trigger Greptile}