diff --git a/docs/CHANGELOG-AGENTS.md b/docs/CHANGELOG-AGENTS.md index 020e4852..9842ce5d 100644 --- a/docs/CHANGELOG-AGENTS.md +++ b/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):** **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). **Recently resolved (2026-08-02):** **OPS17** (Issue #292) — ciclo plan-issue: gate pós-overview antes de Issue/PR; `agent:register --plan` nasce `blocked`; após plano em `main`, `pnpm agent:ready` promove a `ready`. Planos: [`plan-issue-lifecycle-ready.md`](docs/plans/plan-issue-lifecycle-ready.md), impl [`plan-issue-lifecycle-ready-impl.md`](docs/plans/plan-issue-lifecycle-ready-impl.md). Action dual no merge = OPS18 (#296). diff --git a/docs/GUARDRAILS.md b/docs/GUARDRAILS.md index 5a13d902..f1c4bed9 100644 --- a/docs/GUARDRAILS.md +++ b/docs/GUARDRAILS.md @@ -42,3 +42,9 @@ Ledger dos guardrails determinísticos colhidos de `kind:agent-miss` (fluxo `pnp ## Misses colhidas aguardando guarda (fila do harvest) (nenhuma — todas as 5 abertas em 2026-07-31 foram classificadas: 4 com guarda shippada + 1 com guarda já viva. Novas `kind:agent-miss` entram aqui até o próximo audit.) + +## 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`; `notificationPushScheduleConventions` unit pin | vivo Pass 5 | diff --git a/src/utilities/notification/createCampaignNotification.ts b/src/utilities/notification/createCampaignNotification.ts index 50347f82..f50d98fd 100644 --- a/src/utilities/notification/createCampaignNotification.ts +++ b/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 @@ -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 } diff --git a/src/utilities/notification/sendCampaignPush.ts b/src/utilities/notification/sendCampaignPush.ts index 364e4dc1..7546f0ad 100644 --- a/src/utilities/notification/sendCampaignPush.ts +++ b/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 => { 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 } }, diff --git a/src/utilities/payloadTransaction.ts b/src/utilities/payloadTransaction.ts index 45e2a251..091ca737 100644 --- a/src/utilities/payloadTransaction.ts +++ b/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 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 => { + discardAfterCommitCallbacks(transactionID) try { await payload.db.rollbackTransaction(transactionID) } catch (rollbackError) { @@ -62,5 +108,6 @@ export const withPayloadTransaction = async ( return rollbackOrAggregate(payload, transactionID, error) } + runAfterCommitCallbacks(transactionID) return result } diff --git a/tests/int/payloadTransaction.int.spec.ts b/tests/int/payloadTransaction.int.spec.ts index 49ee2504..2751b6fa 100644 --- a/tests/int/payloadTransaction.int.spec.ts +++ b/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') + }) }) diff --git a/tests/unit/notificationPushScheduleConventions.unit.spec.ts b/tests/unit/notificationPushScheduleConventions.unit.spec.ts new file mode 100644 index 00000000..c5d8f943 --- /dev/null +++ b/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/, + ) + }) +})