Merge pull request #348 from fsolla/cursor/p5-p1-notification-push-commit-ed58

P5-P1 — Push de notificação só após commit da transação

docs/CHANGELOG-AGENTS.md

@@ -4,6 +4,8 @@ Conteúdo movido do `AGENTS.md` em 2026-07-30 (fatiamento do paradigma de agente
 
 ---
 
+**Recently resolved (2026-08-03):** **Pass 5 P1** — push de notificação de campanha após commit: `onPayloadTransactionCommit` em `withPayloadTransaction`; `createCampaignNotification` não usa mais `queueMicrotask` sob `req.transactionID`; `sendCampaignPushForNotification` soft-fail se a row sumiu. Guarda `notificationPushScheduleConventions`.
+
 **Recently resolved (2026-08-03):** **Pass 5 — auditoria de engenharia autônoma** — precheck solitário (pool desligado); delta desde Pass 4 = 627 arquivos / 520 commits; baseline verde (madge **859** arquivos). **0 P0**; **1 P1** — push de notificação via `queueMicrotask` sob `req.transactionID` corria antes do commit (`createCampaignNotification`) → after-commit registry + soft-fail. **0** `kind:agent-miss` abertas. Carry-forward P4-A…P4-L re-medido; ondas novas P5-A…P5-L em [`entrega-engenharia-p5.md`](plans/entrega-engenharia-p5.md).
 
 **Recently resolved (2026-08-03):** **OPS18** (Issue #296) — promote dual plan-issue: Action `plan-issue-ready-on-main-merge.yml` no merge lê `Related #N` e promove Issues ainda aguardando plano (`canPromotePlanIssue`); idempotente com `pnpm agent:ready`. Planos: [`plan-issue-promote-dual.md`](docs/plans/plan-issue-promote-dual.md), impl [`plan-issue-promote-dual-impl.md`](docs/plans/plan-issue-promote-dual-impl.md).

docs/GUARDRAILS.md

@@ -45,6 +45,6 @@ Ledger dos guardrails determinísticos colhidos de `kind:agent-miss` (fluxo `pnp
 
 ## Pass 5 — guardas adicionadas (não-miss)
 
-| Guarda                                                                                     | Origem | Classe | Mecanismo                                                                                                                                 | Status                       |
-| ------------------------------------------------------------------------------------------ | ------ | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------- |
-| Push de notificação de campanha só após commit da transação; send soft-fail se a row sumiu | P5-P1  | 3 + 5  | `onPayloadTransactionCommit` em `payloadTransaction.ts`; schedule em `createCampaignNotification`; pin em `notificationPushSchedule` unit | remediação Pass 5 (PR P5-P1) |
+| Guarda                                                                                     | Origem | Classe | Mecanismo                                                                                                                                         | Status      |
+| ------------------------------------------------------------------------------------------ | ------ | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- |
+| Push de notificação de campanha só após commit da transação; send soft-fail se a row sumiu | P5-P1  | 3 + 5  | `onPayloadTransactionCommit` em `payloadTransaction.ts`; schedule em `createCampaignNotification`; `notificationPushScheduleConventions` unit pin | vivo Pass 5 |

docs/TECH-DEBT.md

@@ -64,7 +64,7 @@ Ledger of known engineering debt. Items large enough to be scheduled work become
 | homeSearch int suites cloned ×7 (662 LOC of same-shaped `it('returns empty…'/'scopes…'/'rejects leaders…')` — `describe.each` over domains would own the shared contract | `tests/int/homeSearch*.int.spec.ts` | duplication | low | M | P3 | open — Pass 4 | |
 | Flake inventory: fixed timeouts (`CEREMONY_TIMEOUT=60s`, HomeActions `timeout: 15000` ×many, ColumnPicker `REFRESH=20s`), `Date.now()` markers in fixture names, e2e deep links now claimed (Pass 4 PR #93) but shared leased consent rows remain load-bearing | `tests/e2e/*`, `tests/int/municipalityPageData.int.spec.ts` | flake risk | low | M | P3 | open — Pass 4 (prefer event waits; allocate municipalities — both guarded since Pass 4) | |
 
-| **P5-P1 (remediated in-session):** `createCampaignNotification` scheduled push with `queueMicrotask` under `req.transactionID` — microtasks run before `withPayloadTransaction` commit, so push could fire for an uncommitted / rolled-back notification | `src/utilities/notification/createCampaignNotification.ts:38-40`; `src/utilities/payloadTransaction.ts:40-65` | correctness (txn ordering) | high — push for phantom notification; noisy failure on rollback | S | P1 | open — remediating Pass 5 (P5-P1 in-session) | P5-P1 |
+| **P5-P1 (remediated in-session):** `createCampaignNotification` scheduled push with `queueMicrotask` under `req.transactionID` — microtasks run before `withPayloadTransaction` commit, so push could fire for an uncommitted / rolled-back notification | `src/utilities/notification/createCampaignNotification.ts:38-40`; `src/utilities/payloadTransaction.ts:40-65` | correctness (txn ordering) | high — push for phantom notification; noisy failure on rollback | S | P1 | closed 2026-08-03 (Pass 5 P1) — after-commit registry + existence-safe send | P5-P1 |
 | Wizard/action href knowledge dual-switched (`wizardActionChain` chain + previous) and wide URL surface (`campaignActionRoutes` 252 LOC / 31 exports) | `src/lib/wizardActionChain.ts`; `src/lib/campaignActionRoutes.ts` | duplication + wide interface | medium — every new wizard step touches both switches | M | P2 | open — Pass 5 | P5-A |
 | Staff-detail wizard action id list re-spelled ×3 (activity / leadership / demand adapters) | `src/lib/activityQuickActions.ts:18-24`; `campaignQuickActionLeadership.ts:19-25`; `campaignQuickActionDemands.ts:14-20` | duplication | low | S | P2 | open — Pass 5 | P5-B |
 | `/campanha/acoes/[slug]` page is a 296-line auth+load+branch dispatcher | `src/app/(campaign)/campanha/(app)/acoes/[slug]/page.tsx` | bloater / divergent change | medium — every quick-action lands here | M | P2 | open — Pass 5 | P5-C |

src/utilities/notification/createCampaignNotification.ts

@@ -7,6 +7,7 @@ import type { Notification } from '@/payload-types'
 import type { Payload, PayloadRequest } from 'payload'
 
 import { sendCampaignPushForNotification } from '@/utilities/notification/sendCampaignPush'
+import { onPayloadTransactionCommit } from '@/utilities/payloadTransaction'
 
 export type NotificationWriteRequest = Pick<PayloadRequest, 'transactionID'>
 
@@ -17,6 +18,26 @@ type CreateCampaignNotificationInput = {
   municipalityID?: number | null
 }
 
+const scheduleCampaignNotificationPush = (
+  payload: Payload,
+  notificationID: number,
+  req?: NotificationWriteRequest,
+): void => {
+  const run = (): void => {
+    void sendCampaignPushForNotification(payload, notificationID)
+  }
+
+  // Pass 5 P1: never `queueMicrotask` under an open transaction — microtasks
+  // flush before `withPayloadTransaction` reaches `commitTransaction`.
+  const transactionID = req?.transactionID
+  if (typeof transactionID === 'string' || typeof transactionID === 'number') {
+    onPayloadTransactionCommit(transactionID, run)
+    return
+  }
+
+  queueMicrotask(run)
+}
+
 const createCampaignNotification = async (
   payload: Payload,
   input: CreateCampaignNotificationInput,
@@ -35,9 +56,7 @@ const createCampaignNotification = async (
     req,
   })
 
-  queueMicrotask(() => {
-    void sendCampaignPushForNotification(payload, notification.id)
-  })
+  scheduleCampaignNotificationPush(payload, notification.id, req)
 
   return notification
 }

src/utilities/notification/sendCampaignPush.ts

@@ -5,6 +5,7 @@ import 'server-only'
 import webpush from 'web-push'
 
 import type { NotificationPayload } from '@/lib/notificationContract'
+import type { Notification } from '@/payload-types'
 import type { Payload } from 'payload'
 
 const resolveVapidConfig = (): {
@@ -42,12 +43,19 @@ export const sendCampaignPushForNotification = async (
 ): Promise<void> => {
   if (!ensureVapidConfigured()) return
 
-  const notification = await payload.findByID({
-    collection: 'notification',
-    id: notificationID,
-    depth: 0,
-    overrideAccess: true,
-  })
+  // Soft-fail when the row is gone (rolled-back txn, or a deferred send that
+  // raced commit). Push is best-effort; never throw into an after-commit hook.
+  let notification: Notification
+  try {
+    notification = await payload.findByID({
+      collection: 'notification',
+      id: notificationID,
+      depth: 0,
+      overrideAccess: true,
+    })
+  } catch {
+    return
+  }
 
   const recipientID =
     typeof notification.recipient === 'number' ? notification.recipient : notification.recipient?.id
@@ -55,7 +63,6 @@ export const sendCampaignPushForNotification = async (
 
   const content = notification.payload as NotificationPayload
   if (!content?.title || !content.href) return
-
   const subscriptions = await payload.find({
     collection: 'pushSubscription',
     where: { user: { equals: recipientID } },

src/utilities/payloadTransaction.ts

@@ -19,11 +19,57 @@ export type PayloadTransactionOptions = {
 
 const DEFAULT_BEGIN_FAILURE_MESSAGE = 'Não foi possível iniciar a transação.'
 
+const afterCommitCallbacks = new Map<string, Array<() => void>>()
+
+const transactionKey = (transactionID: number | string): string => String(transactionID)
+
+/**
+ * Register a side effect to run only after `withPayloadTransaction` successfully
+ * commits. Callbacks are discarded on rollback. Used so notification push (and
+ * similar post-write work) cannot observe an uncommitted row or fire after a
+ * rollback — `queueMicrotask` runs before `commitTransaction` (Pass 5 P1).
+ *
+ * Only transactions opened by `withPayloadTransaction` flush this registry.
+ * Callers that pass a foreign/Payload-internal `transactionID` should not rely
+ * on this hook; prefer creating the side-effect write without that `req`, or
+ * open the write through `withPayloadTransaction`.
+ */
+export const onPayloadTransactionCommit = (
+  transactionID: number | string,
+  callback: () => void,
+): void => {
+  const key = transactionKey(transactionID)
+  const existing = afterCommitCallbacks.get(key)
+  if (existing) {
+    existing.push(callback)
+    return
+  }
+  afterCommitCallbacks.set(key, [callback])
+}
+
+const discardAfterCommitCallbacks = (transactionID: number | string): void => {
+  afterCommitCallbacks.delete(transactionKey(transactionID))
+}
+
+const runAfterCommitCallbacks = (transactionID: number | string): void => {
+  const key = transactionKey(transactionID)
+  const callbacks = afterCommitCallbacks.get(key) ?? []
+  afterCommitCallbacks.delete(key)
+  for (const callback of callbacks) {
+    try {
+      callback()
+    } catch {
+      // After-commit side effects must not fail the write caller.
+    }
+  }
+}
+
 const rollbackOrAggregate = async (
   payload: TransactionPayload,
   transactionID: number | string,
   originalError: unknown,
 ): Promise<never> => {
+  discardAfterCommitCallbacks(transactionID)
   try {
     await payload.db.rollbackTransaction(transactionID)
   } catch (rollbackError) {
@@ -62,5 +108,6 @@ export const withPayloadTransaction = async <Result>(
     return rollbackOrAggregate(payload, transactionID, error)
   }
 
+  runAfterCommitCallbacks(transactionID)
   return result
 }

tests/int/payloadTransaction.int.spec.ts

@@ -3,7 +3,7 @@
 import type { Payload } from 'payload'
 import { describe, expect, it, vi } from 'vitest'
 
-import { withPayloadTransaction } from '@/utilities/payloadTransaction'
+import { onPayloadTransactionCommit, withPayloadTransaction } from '@/utilities/payloadTransaction'
 
 import { stub } from '../helpers/stub'
 
@@ -131,4 +131,60 @@ describe('withPayloadTransaction', () => {
     expect((failure as AggregateError).errors).toEqual([commit, rollback])
     expect((failure as AggregateError & { cause?: unknown }).cause).toBe(commit)
   })
+
+  it('runs onPayloadTransactionCommit callbacks only after a successful commit', async () => {
+    const fixture = createPayload()
+    const afterCommit = vi.fn()
+
+    await withPayloadTransaction(fixture.payload, async ({ transactionID }) => {
+      onPayloadTransactionCommit(transactionID, afterCommit)
+      expect(afterCommit).not.toHaveBeenCalled()
+      return 'ok'
+    })
+
+    expect(fixture.commitTransaction).toHaveBeenCalledWith(17)
+    expect(afterCommit).toHaveBeenCalledTimes(1)
+  })
+
+  it('discards onPayloadTransactionCommit callbacks when the callback fails', async () => {
+    const fixture = createPayload()
+    const afterCommit = vi.fn()
+
+    await expect(
+      withPayloadTransaction(fixture.payload, async ({ transactionID }) => {
+        onPayloadTransactionCommit(transactionID, afterCommit)
+        throw new Error('callback falhou')
+      }),
+    ).rejects.toThrow('callback falhou')
+
+    expect(fixture.rollbackTransaction).toHaveBeenCalledWith(17)
+    expect(afterCommit).not.toHaveBeenCalled()
+  })
+
+  it('discards onPayloadTransactionCommit callbacks when commit fails', async () => {
+    const fixture = createPayload({ commitError: new Error('commit falhou') })
+    const afterCommit = vi.fn()
+
+    await expect(
+      withPayloadTransaction(fixture.payload, async ({ transactionID }) => {
+        onPayloadTransactionCommit(transactionID, afterCommit)
+        return 'ok'
+      }),
+    ).rejects.toThrow('commit falhou')
+
+    expect(afterCommit).not.toHaveBeenCalled()
+  })
+
+  it('swallows after-commit callback errors so the write caller still receives the result', async () => {
+    const fixture = createPayload()
+
+    const result = await withPayloadTransaction(fixture.payload, async ({ transactionID }) => {
+      onPayloadTransactionCommit(transactionID, () => {
+        throw new Error('push side-effect failed')
+      })
+      return 'ok'
+    })
+
+    expect(result).toBe('ok')
+  })
 })

tests/unit/notificationPushScheduleConventions.unit.spec.ts

@@ -0,0 +1,28 @@
+import { readFileSync } from 'node:fs'
+import { resolve } from 'node:path'
+import { describe, expect, it } from 'vitest'
+
+// Pass 5 P1: notification push must not be scheduled with `queueMicrotask` while
+// the create still rides `req.transactionID` — microtasks flush before
+// `withPayloadTransaction` commits. The create module must route txn-bound
+// sends through `onPayloadTransactionCommit`.
+describe('notification push schedule (Pass 5 P1)', () => {
+  const source = readFileSync(
+    resolve(process.cwd(), 'src/utilities/notification/createCampaignNotification.ts'),
+    'utf8',
+  )
+
+  it('registers push on withPayloadTransaction commit when a transactionID is present', () => {
+    expect(source).toMatch(/onPayloadTransactionCommit\s*\(/)
+    expect(source).toMatch(/req\?\.transactionID/)
+  })
+
+  it('does not schedule sendCampaignPush via queueMicrotask unconditionally', () => {
+    // Unconditional microtask around send was the P1 bug. A microtask only on
+    // the no-transaction branch remains allowed; the send call must not appear
+    // as the sole queueMicrotask body without a transactionID guard above it.
+    expect(source).not.toMatch(
+      /queueMicrotask\s*\(\s*\(\)\s*=>\s*\{\s*void sendCampaignPushForNotification/,
+    )
+  })
+})