mirror of
https://github.com/fsolla/teqo.git
synced 2026-11-13 15:13:07 +00:00
fix(P5-P1): schedule campaign notification push after txn commit
queueMicrotask under req.transactionID raced withPayloadTransaction commit. Register after-commit callbacks, soft-fail missing notification rows on send, and pin the schedule with a convention unit spec.
This commit is contained in:
parent
e20bbd2718
commit
fb75f7235b
7 changed files with 176 additions and 11 deletions
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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 |
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 } },
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
28
tests/unit/notificationPushScheduleConventions.unit.spec.ts
Normal file
28
tests/unit/notificationPushScheduleConventions.unit.spec.ts
Normal file
|
|
@ -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/,
|
||||
)
|
||||
})
|
||||
})
|
||||
Loading…
Reference in a new issue