Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion docs/contact-auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,17 @@ message is folded into the memory thread like any other unanswered one.
- **denied** → the `denyMessage` (when set) goes to the customer under the same `stillOurs` fence and
persona token every gate message uses; the conversation is opened for humans (+ team) when
`handoffEnabled`; a pt-BR private note tells the operator, with the `reason` code when one came.
That note carries what is **not** on the operator's screen, so what it says about the customer's
copy depends on what the customer actually got: when the copy was delivered the note says nothing
about it (the message is one line above; it keeps the reason code, which is the invisible part),
and it speaks up in the three cases where nothing reached the customer — no `denyMessage`
configured, the notice cooldown withholding a repeat, or nothing arriving at all (the send failed,
or the ownership fence stood it down; the runtime sees one boolean for both, so the note reports
the result and never names a cause it does not know).
- **error** → nothing to the customer, no handoff (transient by contract: the next message retries),
a private note + a `warn` flow line.
a private note + a `warn` flow line. This note and the **no_identity** one are unchanged by the
above: both outcomes are silent to the customer by design, so there is no copy to describe and
"o agente não respondeu automaticamente" is simply true there.
- **no_identity** (no phone, email or identifier) → nothing to the customer (the deny copy would
mislead an unidentified web visitor), but the conversation IS opened for humans when
`handoffEnabled`: a contact the gate can never authorize would otherwise stay pending and
Expand Down
59 changes: 56 additions & 3 deletions src/modules/chatwoot/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2285,6 +2285,19 @@ const CONTACT_AUTH_ERROR_LABELS: Record<string, string> = {
// anything the customer wrote. This is also the ONE place the endpoint's own reason surfaces: the
// note sits in the operator's Chatwoot, on the conversation it is about, unlike the execution log
// that alert channels read.
// What the CUSTOMER got on this refusal, which the note has to describe correctly. Only a `denied`
// verdict can send a copy at all: `no_identity` and `error` are silent to the customer by design.
export type ContactAuthCopyOutcome =
// The refusal notice reached the customer.
| "sent"
// No `denyMessage` configured: the customer got nothing, on purpose.
| "none"
// Configured, but another refusal on this conversation holds the notice window.
| "suppressed"
// Configured and attempted, but nothing reached the customer: the send failed, or the
// ownership fence stood it down.
| "failed";

export function contactAuthNoteText(
verdict: {
outcome: ContactAuthOutcome;
Expand All @@ -2293,6 +2306,8 @@ export function contactAuthNoteText(
endpointReason?: string;
},
handedOff: boolean,
// Defaults to `none` so the sentence stays true for a caller that sends no copy.
copy: ContactAuthCopyOutcome = "none",
): string {
const handoffLine = handedOff
? " A conversa foi aberta para atendimento humano."
Expand All @@ -2306,7 +2321,34 @@ export function contactAuthNoteText(
if (verdict.outcome === "denied") {
const motivo = verdict.endpointReason ?? verdict.reason;
const reason = motivo ? ` Motivo: ${motivo}.` : "";
return `🔒 Contato não autorizado pela verificação externa.${reason} O agente não respondeu automaticamente.${handoffLine}`;
// The note used to say "O agente não respondeu automaticamente" on every refusal, including the
// ones where the deny message HAD just been posted to the customer — a sentence the operator
// reads directly below that very message, which makes the note look broken and hides that the
// customer already knows.
//
// The note earns its space by carrying what is NOT on screen. When the copy went out, the
// operator can see it: saying so adds nothing, so the note stays quiet about it and keeps only
// the reason code, which is the part no one can see. The three cases where NOTHING reached the
// customer are the invisible ones, and each is a different thing for the operator to do: nobody
// configured a copy, the cooldown withheld it, or the send failed and should be chased.
const copyLine = {
sent: "",
none: " Nenhum aviso foi enviado ao contato: não há mensagem de recusa configurada.",
// Says the window was TAKEN, not that a copy landed. A concurrent refusal on the same
// conversation claims the copy window BEFORE it sends, so a claim that fails means "another
// refusal holds it" — which covers both the one that already spoke and the one still in
// flight, and that one may yet fail and give the window back. Whether a copy is on screen is
// the operator's to see; what they cannot see is that THIS message produced none, and why.
suppressed:
" O aviso de recusa não saiu nesta mensagem: a carência entre avisos já estava tomada por outra recusa.",
// Says the RESULT, not a cause. This branch is reached both by a send that failed and by the
// ownership fence standing the copy down (a human took the conversation, or the agent was
// switched off, between the mode read and the refusal): `postPublicMessage` returns the same
// false for both, and naming "delivery failure" here would send the operator chasing a
// problem that does not exist on the second one.
failed: " O aviso de recusa NÃO chegou ao contato.",
}[copy];
return `🔒 Contato não autorizado pela verificação externa.${reason}${copyLine}${handoffLine}`;
}
const cause =
verdict.status !== undefined
Expand Down Expand Up @@ -4072,11 +4114,20 @@ async function maybeConsumeCommandOrGate(params: {
const denyMessage =
verdict.outcome === "denied" ? authCfg.denyMessage : null;
const copyClaim = denyMessage ? claim("copy") : false;
// Tracked so the operator note can say what the CUSTOMER actually got, instead of
// claiming silence on top of a refusal that was just delivered.
let copyOutcome: ContactAuthCopyOutcome = denyMessage
? copyClaim
? "failed"
: "suppressed"
: "none";
Comment on lines +4119 to +4123

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wait for pending copy attempts before reporting suppression

With concurrent refused deliveries for the same conversation, a failed copy claim can mean another send is still pending, not that a notice was already delivered. For example, with handoff disabled, delivery B can post the new cooldown note while delivery A awaits postPublicMessage. If A then fails, B has already consumed the note window, so the only operator note incorrectly describes a withheld repeat rather than an undelivered notice. Serialize the notification sequence per conversation or await the pending copy's outcome before choosing and posting the note.

codex · gpt-6-astra · effort high · confidence 0.96

if (denyMessage && copyClaim) {
// The window is claimed before the send, because two settled deliveries racing must not
// both speak — so a send that does not land has to give it back. Kept, it would silence
// the next refusal for the whole window over a message the customer never received.
if (!(await postPublicMessage(denyMessage))) {
if (await postPublicMessage(denyMessage)) {
copyOutcome = "sent";
} else {
releaseContactAuthNotice(copyClaim);
}
}
Expand All @@ -4090,7 +4141,9 @@ async function maybeConsumeCommandOrGate(params: {
const noteClaim = claim("note");
if (noteClaim) {
if (
!(await postPrivateNote(contactAuthNoteText(verdict, handedOff)))
!(await postPrivateNote(
contactAuthNoteText(verdict, handedOff, copyOutcome),
))
) {
releaseContactAuthNotice(noteClaim);
}
Expand Down
228 changes: 226 additions & 2 deletions tests/modules/contact-auth-gate-e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,41 @@ interface Sent {
// Recording Chatwoot double, injected via deps.makeClient so neither the gate nor the turn ever
// reaches a socket. The factory captures the bot token each client was built with: the deny copy
// must leave as the PERSONA, not as a token-less client that a real Chatwoot would 401.
function stubChatwoot() {
// `fail` injects delivery failures, which is the only way to reach two of the four things the
// operator note can say about the customer's copy: a send that did not land, and the cooldown
// withholding a repeat (reachable when an earlier note failed and freed its own window while the
// copy's stayed spent).
function stubChatwoot(
fail: {
// Only the FIRST send/note fails: the delivery after it has to be able to speak, which is what
// proves the claimed window was handed back.
firstSend?: boolean;
firstNote?: boolean;
// Holds the first public send open until the latch is released, so a second delivery can run to
// completion while the first is still awaiting Chatwoot.
holdFirstSend?: { entered: () => void; wait: Promise<"ok" | "throw"> };
} = {},
) {
const sent: Sent[] = [];
const statusToggles: Array<[number, string]> = [];
const teamAssignments: Array<[number, number]> = [];
let token = "";
let sends = 0;
let notes = 0;
const client = {
sendMessage: async (c: number, content: string) => {
sends += 1;
if (fail.firstSend && sends === 1) {
throw new Error("chatwoot down: send refused");
}
const hold = fail.holdFirstSend;
if (hold) {
fail.holdFirstSend = undefined;
hold.entered();
if ((await hold.wait) === "throw") {
throw new Error("chatwoot down: send refused");
}
}
sent.push({
conversationId: c,
content,
Expand All @@ -101,6 +129,10 @@ function stubChatwoot() {
return {};
},
sendPrivateNote: async (c: number, content: string) => {
notes += 1;
if (fail.firstNote && notes === 1) {
throw new Error("chatwoot down: note refused");
}
sent.push({ conversationId: c, content, private: true, token });
return {};
},
Expand Down Expand Up @@ -567,6 +599,12 @@ describe.skipIf(!dbUp)("contact authorization gate (webhook e2e)", () => {
expect(notes[0]?.content).toContain("não autorizado");
expect(notes[0]?.content).toContain("not_customer");
expect(notes[0]?.content).not.toContain(PHONE);
// The copy went out in this very conversation, one line above. The note used to claim "o agente
// não respondeu automaticamente" here, contradicting the screen; announcing that the contact WAS
// warned would be just as useless, because the operator can see the message. So the note carries
// only what is not on screen — the reason code above — and says nothing about the copy.
expect(notes[0]?.content).not.toContain("não respondeu automaticamente");
expect(notes[0]?.content).not.toContain("aviso");
// The message is consumed: the watermark advanced so no later flush re-answers it.
const conv = await suDb.conversation.findFirstOrThrow({
where: { tenantId, chatwootConversationId: convId },
Expand Down Expand Up @@ -933,7 +971,193 @@ describe.skipIf(!dbUp)("contact authorization gate (webhook e2e)", () => {
expect(cw.statusToggles).toEqual([[convId, "open"]]);
// No team configured: open only, Chatwoot routes.
expect(cw.teamAssignments).toEqual([]);
expect(cw.notesOn(convId)).toHaveLength(1);
// The note is the ONLY place this is visible: there is no message on screen to infer it from,
// so it names both the silence and its cause.
const notes = cw.notesOn(convId);
expect(notes).toHaveLength(1);
expect(notes[0]?.content).toContain("Nenhum aviso foi enviado ao contato");
expect(notes[0]?.content).toContain(
"não há mensagem de recusa configurada",
);
});

test("a deny copy that does not land is named in the note as a delivery failure", async () => {
const convId = 9315;
await seedConversation(convId, inboxFullDbId);
const cw = stubChatwoot({ firstSend: true });
const auth = authDouble(
() => denied("not_customer"),
() => denied("not_customer"),
);
await deliverCustomerMessage({
convId,
chatwootInboxId: INBOX_FULL,
senderId: 815,
phone: PHONE,
fetchImpl: auth.fetchImpl,
makeClient: cw.makeClient,
});
expect(cw.publicOn(convId)).toEqual([]);
// Nothing on screen says the send failed, and the difference matters to the operator: this one
// is a delivery problem to chase, not a decision someone made.
const notes = cw.notesOn(convId);
expect(notes).toHaveLength(1);
expect(notes[0]?.content).toContain("NÃO chegou ao contato");
expect(notes[0]?.content).toContain("not_customer");
expect(notes[0]?.content).not.toContain(PHONE);
expect(notes[0]?.content).not.toContain("carência");
expect(notes[0]?.content).not.toContain("Nenhum aviso");
// And the window came back with it. A send that did not land must not silence the next refusal
// for the whole window over a message the customer never received, so the second delivery — well
// inside the 300s — speaks. This is what `releaseContactAuthNotice(copyClaim)` buys, and nothing
// else in the suite exercises it end to end.
await deliverCustomerMessage({
convId,
chatwootInboxId: INBOX_FULL,
senderId: 815,
phone: PHONE,
fetchImpl: auth.fetchImpl,
makeClient: cw.makeClient,
});
expect(cw.publicOn(convId)).toEqual([
{
conversationId: convId,
content: DENY_COPY,
private: false,
token: BOT_TOKEN,
},
]);
});

// Two refusals racing on ONE conversation. Single-flight is keyed by contact AND request, so two
// different messages are two questions and two deliveries in flight at once. The loser of the copy
// claim writes its note while the winner is still awaiting Chatwoot — and the winner may yet fail
// and hand the window back. So the note that survives must not claim a copy landed: all it can
// say is that the window was taken.
test("a refusal that lost the copy window never claims the other one was delivered", async () => {
const convId = 9318;
await seedConversation(convId, inboxFullDbId);
let entrou = () => {};
const entered = new Promise<void>((r) => {
entrou = r;
});
let liberar: (v: "ok" | "throw") => void = () => {};
const wait = new Promise<"ok" | "throw">((r) => {
liberar = r;
});
const cw = stubChatwoot({ holdFirstSend: { entered: entrou, wait } });
const auth = authDouble(
() => denied(),
() => denied(),
);
const primeira = deliverCustomerMessage({
convId,
chatwootInboxId: INBOX_FULL,
senderId: 819,
phone: PHONE,
fetchImpl: auth.fetchImpl,
makeClient: cw.makeClient,
});
// The first delivery is now parked inside Chatwoot's send, holding the copy window.
await entered;
await deliverCustomerMessage({
convId,
chatwootInboxId: INBOX_FULL,
senderId: 819,
phone: PHONE,
fetchImpl: auth.fetchImpl,
makeClient: cw.makeClient,
});
// ...and only now does it fail, so NOTHING ever reached the customer on either message.
liberar("throw");
await primeira;
expect(cw.publicOn(convId)).toEqual([]);
// The second delivery took the note window, so its note is the only one the operator gets.
const notes = cw.notesOn(convId);
expect(notes).toHaveLength(1);
// It may say the window was taken. It may NOT say a notice was delivered or repeated: the copy
// it lost the race to never landed, and the conversation above it shows that.
expect(notes[0]?.content).toContain("não saiu nesta mensagem");
expect(notes[0]?.content).not.toContain("repetido");
});

// The SAME false from postPublicMessage, for a completely different reason: the copy was stood
// down by the ownership fence because a human took the conversation inside the authorization
// round-trip. Nothing failed to deliver here, so a note that named a delivery failure would send
// the operator chasing one.
test("a copy the fence stood down reads as the copy not arriving, not as a broken send", async () => {
const convId = 9317;
await seedConversation(convId, inboxFullDbId);
const cw = stubChatwoot();
const auth = authDouble(async () => {
await suDb.conversation.updateMany({
where: {
tenantId,
chatwootInstanceId: instanceId,
chatwootConversationId: convId,
},
data: { assigneeType: "User", assigneeId: 44, status: "open" },
});
return denied();
});
await deliverCustomerMessage({
convId,
chatwootInboxId: INBOX_FULL,
senderId: 818,
phone: PHONE,
fetchImpl: auth.fetchImpl,
makeClient: cw.makeClient,
});
// The conversation is the human's: nothing is said to the customer and nothing is toggled.
expect(cw.publicOn(convId)).toEqual([]);
expect(cw.statusToggles).toEqual([]);
// The note still goes out (it has no fence: it is FOR the human who just took over), and it is
// true for them — it reports the result, and claims neither a delivery failure nor a handoff.
const notes = cw.notesOn(convId);
expect(notes).toHaveLength(1);
expect(notes[0]?.content).toContain("NÃO chegou ao contato");
expect(notes[0]?.content).not.toContain("atendimento humano");
expect(notes[0]?.content).not.toContain("carência");
});

test("a repeat withheld by the cooldown is named as the cooldown, not as a failure", async () => {
const convId = 9316;
await seedConversation(convId, inboxFullDbId);
// The first note fails, so it gives ITS window back while the copy's stays spent. That is what
// separates the two windows, and it is the state in which the second refusal has a note to
// write about a copy it did not send.
const cw = stubChatwoot({ firstNote: true });
const auth = authDouble(
() => denied("not_customer"),
() => denied("not_customer"),
);
for (const senderId of [816, 817]) {
await deliverCustomerMessage({
convId,
chatwootInboxId: INBOX_FULL,
senderId,
phone: PHONE,
fetchImpl: auth.fetchImpl,
makeClient: cw.makeClient,
});
}
// One copy for the two refusals: the second was inside the window. Checked by content and token
// too — it has to be the deny copy, sent as the persona, not just "some message".
expect(cw.publicOn(convId)).toEqual([
{
conversationId: convId,
content: DENY_COPY,
private: false,
token: BOT_TOKEN,
},
]);
const notes = cw.notesOn(convId);
expect(notes).toHaveLength(1);
expect(notes[0]?.content).toContain("carência entre avisos");
expect(notes[0]?.content).toContain("não saiu nesta mensagem");
expect(notes[0]?.content).toContain("not_customer");
expect(notes[0]?.content).not.toContain(PHONE);
expect(notes[0]?.content).not.toContain("NÃO chegou ao contato");
});

// A Chatwoot team id belongs to ONE account. The editor cannot warn about an agent MOVED to
Expand Down
Loading