From d48914858a350a379ed77e8dfa52a3c851a1934b Mon Sep 17 00:00:00 2001 From: Francisco Solla Date: Mon, 24 Aug 2026 22:27:32 -0300 Subject: [PATCH] =?UTF-8?q?fix(campaignInvite):=20C121=20=E2=80=94=20unifi?= =?UTF-8?q?ca=20chave=20de=20lock=20da=20reden=C3=A7=C3=A3o=20com=20contac?= =?UTF-8?q?t-ficha=20(Closes=20#885)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/changelog/2026-08-24-c121.md | 1 + ...-unificar-chave-invite-redemption-ficha.md | 63 ++++++++++++++----- src/utilities/campaignInviteRedemption.ts | 38 +++++++++-- src/utilities/campaignInviteRepository.ts | 7 --- tests/int/campaignInvite.int.spec.ts | 60 ++++++++++++++---- 5 files changed, 127 insertions(+), 42 deletions(-) create mode 100644 docs/changelog/2026-08-24-c121.md diff --git a/docs/changelog/2026-08-24-c121.md b/docs/changelog/2026-08-24-c121.md new file mode 100644 index 00000000..cd0b7d7f --- /dev/null +++ b/docs/changelog/2026-08-24-c121.md @@ -0,0 +1 @@ +**C121 — chave de lock da redenção de convite unificada com o lock de ficha `contact-ficha:` (2026-08-24):** débito colhido no fechamento do C120 (#663): a redenção de convite serializava com `invite-redemption-contact:` enquanto o find-or-create (C120) usa `contact-ficha:` — fluxos que resolviam a MESMA ficha por primários diferentes não se serializavam e um telefone digitado sumia. **Renomeada** a chave para `contact-ficha:` via `acquireContactFichaLock` (já existia em `contactPhoneLocks.ts`); **removido** o helper `acquireCampaignInviteRedemptionContactLock` (`campaignInviteRepository.ts:127-132`, único caller). **Realinhada a ordem ao C120** (telefones/account em batch PRIMEIRO, ficha POR ÚLTIMO e sozinha) ao mover a aquisição para DEPOIS dos phone/account-locks e ANTES do RMW do contact — cobre a mesma janela (pg_advisory_xact_lock é transaction-scoped) e **evita o deadlock** que o simples renome (mesma ordem) introduziria sob a corrida (redenção ficha→phone vs create phone→ficha na mesma chave). **Re-leitura da ficha DENTRO do lock** em ambos os fluxos (login `campaignInviteRedemption.ts` + autofill), preservando a semântica de reorder do primário mas com phones frescos — senão o append concorrente do create seria dropado pelo full-replace stale. **Incluído o path de autofill** (antes sem lock de ficha) para fechar 100% a corrida. Sem migration (schema intocado). **Spec int `tests/int/campaignInvite.int.spec.ts`** revalidado: `invite-redemption-contact:`→`contact-ficha:`, teste de ordem reescrito para ficha por último, e novo teste de serialização do autofill no `contact-ficha`. /simplify (2 revisores): MAJOR (RMW stale pré-lock) e minor (teste de autofill) ambos resolvidos inline; nenhum débito aberto. Closes #885. diff --git a/docs/plans/c121-unificar-chave-invite-redemption-ficha.md b/docs/plans/c121-unificar-chave-invite-redemption-ficha.md index 768a805f..aaff04cd 100644 --- a/docs/plans/c121-unificar-chave-invite-redemption-ficha.md +++ b/docs/plans/c121-unificar-chave-invite-redemption-ficha.md @@ -1,6 +1,6 @@ # Impl: C121 — unificar a chave de lock da redenção de convite com o lock de ficha -Status: rascunho +Status: implementado (GATE aprovou opção B + re-leitura dentro do lock + autofill) Atualizado em: 2026-08-24 Issue: #C121 (débito registrado em `pnpm agent:register` pelo fechamento do C120) Intenção: débito capturado no fechamento do C120 (ver `docs/plans/c120-race-append-telefone-ficha-impl.md` §Componentes, "Invite redemption") @@ -21,30 +21,55 @@ Appetite restante: ~1 dia ## Abordagem recomendada -Trocar `invite-redemption-contact:` → `contact-ficha:` na redenção -(`campaignInviteRepository.ts:127-132` / `campaignInviteRedemption.ts:66-145`), -usando `acquireContactFichaLock`. O lock já está em `contactPhoneLocks.ts` desde -o C120; aqui é só o call-site novo + revalidar as asserções **literais** do -spec `tests/int/campaignInvite.int.spec.ts:799-898` (que ancora a chave antiga). +Trocar `invite-redemption-contact:` → `contact-ficha:` na redenção usando +`acquireContactFichaLock` (já existe em `contactPhoneLocks.ts` desde o C120). O +spec `tests/int/campaignInvite.int.spec.ts:799-898` ancora a chave antiga e deve +ser revalidado. + +**Encontrado na exploração (decisão no GATE):** o create-flow do C120 adquire os +locks na ordem `phone-keys PRIMEIRO → contact-ficha POR ÚLTIMO` +(`contactIdentity.ts:58`,`:81`, re-lendo os phones DENTRO do lock de ficha). A +redenção de login hoje adquire o lock de contato/ficha PRIMEIRO +(`campaignInviteRedemption.ts:67`) e os phone/account-keys DEPOIS (`:201`). +`pg_advisory_xact_lock` é transaction-scoped, então o lock cobre a RMW do +contact (`:232`) — a premissa do plano ("mesma janela") se confirma. MAS, se eu +só renomear a chave (mantendo a ordem atual), redenção e create passam a +contender pela MESMA chave `contact-ficha:` em ordem INVERSA → **novo +deadlock** sob a corrida (redenção segura ficha→phone; create segura +phone→ficha). Esse deadlock NÃO existe hoje (chaves diferentes). **Opções consideradas:** -- **A — reusar `acquireContactFichaLock` trocando a chave (recomendada):** 1 - call-site + revalidação do spec; alinha com o C120. -- **B — manter chaves separadas e documentar (rejeitada):** é exatamente o gap - que o C120 abriu — dois locks de entidade de contato que não se serializam. -- **Rejeitadas além:** nenhuma (sem alternativa de mecanismo; a semântica de - advisory lock é a mesma já aceita no C120). +- **A — renomear a chave MANTENDO a ordem atual (literal do plano original):** + 1 call-site + revalidação do spec; passa no teste de ordem existente + (`:872-901`, que exige o lock de contato ANTES de `invite-redemption-user`). + CUSTO: introduz o deadlock acima sob a corrida rara. **(não recomendada após + exploração)** +- **B — renomear a chave E realinhar a ordem ao C120 (recomendada):** mover a + aquisição de `acquireContactFichaLock` para DEPOIS dos phone/account-locks + (`:201`) e ANTES da RMW do contact (`:232`), removendo-a de + `findReusableLeadershipAccount` (`:67`). Ficha passa a ser adquirida por + ÚLTIMO, igual ao create → sem deadlock, mesma janela de proteção. CUSTO: o + teste de ordem `:872-901` precisa ser reescrito para a nova ordem (a chave de + ficha agora vem DEPOIS de `invite-redemption-user`). +- **C — manter chaves separadas (rejeitada):** é exatamente o gap que o C120 + abriu — dois locks de contato que não se serializam. ### Componentes / mudanças - **`src/utilities/campaignInviteRedemption.ts`** — trocar a chave para - `contact-ficha:`. -- **`src/utilities/campaignInviteRepository.ts:127-132`** — a public key skin de - `` (semântica atual) aponta para `contact-ficha:`. -- **`tests/int/campaignInvite.int.spec.ts:799-898`** — espera literal da chave - atualizada. + `contact-ficha:`; na opção B, mover a aquisição para depois de `:201`. +- **`src/utilities/campaignInviteRepository.ts:127-132`** — `acquireCampaignInviteRedemptionContactLock` + (único caller: `:67`) é removida; o call-site passa a usar `acquireContactFichaLock`. +- **`tests/int/campaignInvite.int.spec.ts:799-898`** — `invite-redemption-contact:` + → `contact-ficha:`; na opção B, reescrever `:872-901` para a nova ordem. - **Migration:** sem migration. **Access/Consent:** n/a. **UI:** n/a. +- **Observação de escopo (GATE):** o path de AUTO-FILL (`redeemCampaignInviteAutofillRecord`, + `:85-158`) hoje NÃO adquire nenhum lock de ficha — só phone-keys (`:126`). Ele + não usa `invite-redemption-contact` e está FORA das linhas citadas no plano. + Para fechar 100% a corrida de telefone entre redenção e create, o auto-fill + também deveria adquirir `acquireContactFichaLock` (após `:126`, antes `:138`). + Decisão de escopo no GATE. ## Fases verificáveis @@ -75,6 +100,10 @@ spec `tests/int/campaignInvite.int.spec.ts:799-898` (que ancora a chave antiga). - Spec ancora a chave literal → atualizar junto na mesma entrega (sem isso o PR falha com a asserção velha, nunca com falsos verdes). +- **Deadlock por ordem de lock (encontrado na exploração):** só renomear a + chave mantendo a ordem atual cria corrida-deadlock com o create do C120 + (chaves opostas em `contact-ficha:`). Mitigação: opção B (realinhar + ordem ao C120). **Decisão no GATE.** - Redenção é single-use (token linka uma conta); a coincidência com um create-append da mesma ficha é rara — mitigada pelo lock compartilhado. diff --git a/src/utilities/campaignInviteRedemption.ts b/src/utilities/campaignInviteRedemption.ts index 47667137..cdf0b7d1 100644 --- a/src/utilities/campaignInviteRedemption.ts +++ b/src/utilities/campaignInviteRedemption.ts @@ -14,13 +14,12 @@ import type { CampaignUser, Contact } from '@/payload-types' import { requireLeadershipConsent, resolveInviteConsent } from '@/utilities/campaignConsent' import { acquireCampaignInviteAccountLocks, - acquireCampaignInviteRedemptionContactLock, consumeCampaignInvite, findSameContactAccountIDs, INVALID_CAMPAIGN_INVITE_MESSAGE, requireCampaignInvitePostgres, } from '@/utilities/campaignInviteRepository' -import { contactPhoneLockKeys } from '@/utilities/contactPhoneLocks' +import { acquireContactFichaLock, contactPhoneLockKeys } from '@/utilities/contactPhoneLocks' import type { PayloadTransactionRequest } from '@/utilities/payloadTransaction' import { withPayloadTransaction } from '@/utilities/payloadTransaction' @@ -64,7 +63,6 @@ const findReusableLeadershipAccount = async ( leadership: { contact: unknown; user?: unknown }, ): Promise => { const contactID = requireRelationshipId(leadership.contact) - await acquireCampaignInviteRedemptionContactLock(payload, req, contactID) let accountID = relationshipId(leadership.user) if (accountID === null) { @@ -132,13 +130,28 @@ export const redeemCampaignInviteAutofillRecord = async ( ), ), ) + // C121 — serialize the contact RMW on the C120 ficha lock (`contact-ficha:`), + // acquired AFTER the phone/account locks to honor that order and avoid a deadlock. + await acquireContactFichaLock(payload, req, contactID) + // C121 (bypass) — re-read phones inside the ficha lock: the read above was + // pre-lock, so a concurrent create-append would otherwise be dropped here. + const currentPhones = ( + await payload.findByID({ + collection: 'contact', + id: contactID, + depth: 0, + select: { phones: { value: true } }, + overrideAccess: true, + req, + }) + ).phones // C111 — the phone is a contact channel, not a unique person identity: // the ficha this invite anchors is known (leadership.contact), so the // typed number may legitimately match another ficha's. await payload.update({ collection: 'contact', id: contactID, - data: profileContactData(data, originalContact.phones), + data: profileContactData(data, currentPhones), depth: 0, overrideAccess: true, req, @@ -208,6 +221,21 @@ export const redeemCampaignInviteLoginRecord = async ( `account-username:${data.phone}`, ...(account ? [`invite-redemption-user:${account.id}`] : []), ]) + // C121 — serialize the contact RMW on the C120 ficha lock (`contact-ficha:`), + // acquired AFTER the phone/account locks to honor that order and avoid a deadlock. + await acquireContactFichaLock(payload, req, originalContact.id) + // C121 (bypass) — re-read phones inside the ficha lock: the read above was + // pre-lock, so a concurrent create-append would otherwise be dropped here. + const currentPhones = ( + await payload.findByID({ + collection: 'contact', + id: originalContact.id, + depth: 0, + select: { phones: { value: true } }, + overrideAccess: true, + req, + }) + ).phones // C111 — the ficha this invite anchors is known; the typed phone may be // shared with another ficha. The account side stays fail-closed below: // `username` is the login key and remains DB-unique, so the second @@ -232,7 +260,7 @@ export const redeemCampaignInviteLoginRecord = async ( await payload.update({ collection: 'contact', id: originalContact.id, - data: profileContactData(data, originalContact.phones), + data: profileContactData(data, currentPhones), depth: 0, overrideAccess: true, req, diff --git a/src/utilities/campaignInviteRepository.ts b/src/utilities/campaignInviteRepository.ts index cae15e55..713dd449 100644 --- a/src/utilities/campaignInviteRepository.ts +++ b/src/utilities/campaignInviteRepository.ts @@ -124,13 +124,6 @@ export const findSameContactAccountIDs = async ( .filter((value): value is number => typeof value === 'number') } -export const acquireCampaignInviteRedemptionContactLock = ( - payload: Payload, - req: PayloadTransactionRequest, - contactID: number, -): Promise => - acquireTextAdvisoryLocks(payload, req, [`invite-redemption-contact:${contactID}`]) - export const acquireCampaignInviteAccountLocks = ( payload: Payload, req: PayloadTransactionRequest, diff --git a/tests/int/campaignInvite.int.spec.ts b/tests/int/campaignInvite.int.spec.ts index d408efb2..93156e6a 100644 --- a/tests/int/campaignInvite.int.spec.ts +++ b/tests/int/campaignInvite.int.spec.ts @@ -796,12 +796,12 @@ describe('campaign invite domain', () => { ) itWithInviteConsent( - 'waits on the exact invite-redemption-contact key before login redemption mutates data', + 'waits on the exact contact-ficha key before login redemption mutates data', async () => { const actions = await import('@/app/(campaign)/campanha/actions/invite') const coordinator = await campaignFixtures().createCampaignUser('coordinator') const fixture = await createLinkedLoginInvite(coordinator) - const key = `invite-redemption-contact:${fixture.contact.id}` + const key = `contact-ficha:${fixture.contact.id}` const originalAccountName = fixture.account.name const originalContactName = fixture.contact.name @@ -850,11 +850,7 @@ describe('campaign invite domain', () => { password: campaignFixtures().value('senha-nova'), consentAccepted: true, }), - whileWaiting: async (waiterPID) => { - await expectExactAdvisoryLockGranted( - `invite-redemption-contact:${fixture.contact.id}`, - waiterPID, - ) + whileWaiting: async () => { await expectLoginInviteUnchanged({ accountID: fixture.account.id, accountName: fixture.account.name, @@ -870,7 +866,7 @@ describe('campaign invite domain', () => { ) itWithInviteConsent( - 'orders all login redemption keys before waiting on the exact invite-redemption-user key', + 'orders all login redemption keys before waiting on the exact contact-ficha key', async () => { const actions = await import('@/app/(campaign)/campanha/actions/invite') const coordinator = await campaignFixtures().createCampaignUser('coordinator') @@ -878,10 +874,11 @@ describe('campaign invite domain', () => { const usernameKey = `account-username:${fixture.contact.phone}` const contactPhoneKey = `contact-phone:${fixture.contact.phone}` const userKey = `invite-redemption-user:${fixture.account.id}` + const fichaKey = `contact-ficha:${fixture.contact.id}` await expect( withBlockedInviteAction({ - key: userKey, + key: fichaKey, start: () => actions.redeemCampaignInviteLogin({ token: fixture.invite.token, @@ -894,10 +891,7 @@ describe('campaign invite domain', () => { await Promise.all([ expectExactAdvisoryLockGranted(usernameKey, waiterPID), expectExactAdvisoryLockGranted(contactPhoneKey, waiterPID), - expectExactAdvisoryLockGranted( - `invite-redemption-contact:${fixture.contact.id}`, - waiterPID, - ), + expectExactAdvisoryLockGranted(userKey, waiterPID), ]) await expectLoginInviteUnchanged({ accountID: fixture.account.id, @@ -913,6 +907,46 @@ describe('campaign invite domain', () => { }, ) + itWithInviteConsent( + 'waits on the exact contact-ficha key before autofill redemption mutates data', + async () => { + const actions = await import('@/app/(campaign)/campanha/actions/invite') + const coordinator = await campaignFixtures().createCampaignUser('coordinator') + const municipality = await campaignFixtures().getMunicipality() + const { contact, leadership } = await createInviteLeadershipGraph( + coordinator, + municipality.id, + ) + const invite = await createInviteThroughAction(coordinator, { + leadership: leadership.id, + kind: 'autopreenchimento', + }) + const key = `contact-ficha:${contact.id}` + const originalContactName = contact.name + + await expect( + withBlockedInviteAction({ + key, + start: () => + actions.redeemCampaignInviteAutofill({ + token: invite.token, + name: campaignFixtures().value('Nome após contato'), + phone: contact.phone, + consentAccepted: true, + }), + whileWaiting: async () => { + const unchanged = await payload.findByID({ + collection: 'contact', + id: contact.id, + depth: 0, + }) + expect(unchanged.name).toBe(originalContactName) + }, + }), + ).resolves.toEqual({ ok: true }) + }, + ) + itWithInviteConsent( 'consumes an autofill invite once under concurrent redemption and writes only the whitelist', async (consent) => {