Say on the refusal note what the customer actually got, instead of claiming silence over the copy just sent - #585
Conversation
…aiming silence over the copy just sent The operator note said "O agente não respondeu automaticamente" on EVERY contact-auth refusal, including the ones that had just delivered the deny message to the customer. The operator reads that note in the same conversation, one line below that very message: the sentence contradicts the screen, and it hides from whoever picks the conversation up that the customer already knows. The fix is not to announce the opposite. "O contato foi avisado" is just as useless, for the same reason: the message is right there. The note earns its space by carrying what is NOT on screen. So when the copy went out, the note stays quiet about it and keeps only the reason code, which is the part nobody can see. The three cases where NOTHING reached the customer are the invisible ones, and each is a different thing to do: - nobody configured a deny message; - the notice cooldown withheld the repeat (without this, a second refusal inside the window looks like a broken send); - the send failed, which is a delivery problem to chase. The call site already knew which of the four it was: it decides whether there is a `denyMessage`, whether the cooldown let it through, and whether the send landed. That knowledge just never reached the note. `no_identity` and `error` keep their wording: they are silent to the customer by design, there is no copy to describe, and the old sentence was true there. The new argument defaults to `none`, so a caller that sends no copy stays correct. Coverage: 7 unit tests on the note text plus 4 end-to-end paths through the gate (copy delivered, no copy configured, send failed, repeat withheld by the cooldown). A 10-mutant battery over both the wording and the call-site mapping leaves no survivors; the two that first survived — inverting `failed`/`suppressed`, and pinning the outcome to `none` — are what the two new e2e paths exist to kill. One test pins the `sent` note as the SHORTEST of the four, so nobody "improves" it back into a redundant sentence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fazer-ai-bot
left a comment
There was a problem hiding this comment.
🔍 Codex Review
The change accurately tracks refusal-message outcomes and incorporates them into operator notes without changing delivery or cooldown behavior. All seven new unit tests passed; database-backed integration tests were not run.
No actionable findings in this pass.
Reviewed commit: 108a69caf9 · gpt-6-astra · effort high
… a failed send from a fence The holdout scenarios for this issue named a case the first pass got wrong. `postPublicMessage` returns `false` for two different things: the send threw, and the ownership fence stood the copy down because a human took the conversation inside the authorization round-trip (or the agent was switched off in that window). The call site sees one boolean. Wording that says "o envio NÃO foi concluído" reads as a delivery failure, and on the takeover it sends the operator chasing a problem that does not exist — on the very conversation they just picked up, since the note has no fence of its own and is written FOR them. So that line now reports the result and no cause: "O aviso de recusa NÃO chegou ao contato." The other two invisible states keep naming their cause, because there the runtime knows it: no `denyMessage` configured, and the cooldown withholding a repeat. Also from the holdout: a discrimination test that does not depend on the words chosen — with the same reason code and the same handoff, the four states must produce four different notes, all carrying the reason code and none carrying the contact's phone. Byte-identical notes were the actual defect, and this pins it independently of any wording. Plus the e2e for the fence case, and the `denied` bullet in docs/contact-auth.md, which described the note as one fixed sentence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fazer-ai-bot
left a comment
There was a problem hiding this comment.
🔍 Codex Review
The sequential paths are covered and the eight new unit tests pass, but concurrent deliveries can produce an incorrect cooldown explanation that suppresses the actual failure note.
Reviewed commit: 2b69e8f116 · gpt-6-astra · effort high
| let copyOutcome: ContactAuthCopyOutcome = denyMessage | ||
| ? copyClaim | ||
| ? "failed" | ||
| : "suppressed" | ||
| : "none"; |
There was a problem hiding this comment.
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
… a racing refusal may still fail Review found a case the cooldown wording asserted and could not know. Single-flight for the authorization call is keyed by contact AND request, so two messages from the same person are two questions and two deliveries in flight at once on one conversation. The copy window is claimed BEFORE the send, so the delivery that loses the claim proceeds immediately, takes the note window, and writes its note while the winner is still awaiting Chatwoot. If the winner's send then fails it hands the copy window back — and the only note the operator got says a notice was already delivered, on a conversation where nothing ever reached the customer. The note cannot know: the loser sees a failed claim, and nothing more. So it now reports what is true in both branches — this message produced no notice because another refusal holds the window — and never that a copy was repeated or delivered. Whether one is on screen is the operator's to see; what they cannot see is that THIS message produced none, and why. The e2e for it parks the first delivery inside Chatwoot's send (a latch in the double), runs the second to completion against the held window, and only then fails the first. It fails against the old wording, as does the unit test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fazer-ai-bot
left a comment
There was a problem hiding this comment.
🔍 Codex Review
The change accurately carries the refusal-copy outcome into operator notes while preserving existing cooldown, handoff, and ownership behavior. All eight focused unit tests passed; database-backed integration tests were not run.
No actionable findings in this pass.
Reviewed commit: e6d213a168 · gpt-6-astra · effort high
…ing exercised end to end The scenario run found three gaps in the tests this PR adds, all of them things I had checked by hand and left unpinned. The one that matters: `releaseContactAuthNotice(copyClaim)` hands the copy window back when the send does not land, and no end-to-end test made a second delivery afterwards. Without it, one message the customer never received would silence the next refusal for the whole 300s window — the exact failure the release exists to prevent, and the suite would not have noticed it going away. The stub now fails only the FIRST send, so the delivery after it can speak, and the test asserts the deny copy goes out as the persona. Removing the release makes it fail. The other two: the cooldown test counted the copies without checking what they were, so it now asserts content and token like the others; and the new scenarios asked for a refusal with no reason code, which left "the reason survives in all four states, and the phone never leaks" asserted end to end only in the delivered-copy case. They now carry `not_customer` and assert both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fazer-ai-bot
left a comment
There was a problem hiding this comment.
🔍 Codex Review
The change correctly tracks refusal-message outcomes and uses them in operator notes without altering delivery, cooldown, or handoff behavior. All eight new unit tests passed; database-backed integration tests were not run.
No actionable findings in this pass.
Reviewed commit: 80ac1d9e23 · gpt-6-astra · effort high
|
ship gate · tier=T2 · dod=PASS · head=80ac1d9 |
Fixes #584
What was wrong
The contact-auth gate posts a private note to the operator on every refusal. That note said "O agente não respondeu automaticamente" on every refusal, including the ones that had just posted the configured deny message to the customer one line above it.
The operator reads that note in the same conversation, directly below that message. So on the most common refusal the note contradicted what is on the screen, and — worse — it hid the thing whoever picks up the conversation needs: whether the customer already knows they were refused.
What the note should carry
Not the opposite sentence.
"O contato foi avisado"is just as useless, for the same reason: the message is visible. A private note earns its space by carrying what is not on screen.So the note now says nothing about a copy that was delivered (it keeps the endpoint's reason code, which is the part nobody can see), and speaks up in the three cases where nothing reached the customer — each of which is a different action for the operator:
denyMessageconfiguredNenhum aviso foi enviado ao contato: não há mensagem de recusa configurada.O aviso de recusa não saiu nesta mensagem: a carência entre avisos já estava tomada por outra recusa.O aviso de recusa NÃO chegou ao contato.Without the third line, a second refusal inside the window reads as a broken send; without the last one, a copy that never arrived reads as one that did.
The third line says the window was taken, not that a copy landed, and that is not pedantry: single-flight for the authorization call is keyed by contact and request, so two messages from the same person are two deliveries in flight on one conversation. The copy window is claimed before the send, so the delivery that loses the claim runs on, takes the note window, and writes its note while the winner is still awaiting Chatwoot. If the winner's send then fails, it hands the window back — and the only note the operator got would have claimed a delivery that never happened.
That last line reports the result and names no cause on purpose.
postPublicMessagereturns the samefalsefor two different things: the send threw, and the ownership fence stood the copy down because a human took the conversation inside the authorization round-trip (or the agent was switched off in that window). The runtime cannot tell them apart, and the note has no fence of its own — it is written for the person who just took the conversation over. Saying "the send failed" there would send them chasing a problem that does not exist.The call site already knew which of the four applied — it decides whether there is a
denyMessage, whether the cooldown let it through, and whether the send landed. The knowledge simply never reached the note.no_identityanderrorare unchanged: they are silent to the customer by design, there is no copy to describe, and the old sentence was true there. The new parameter defaults tonone, so any caller that sends no copy stays correct.Validation scope
Proved by test
src/modules/chatwoot/webhook.tsreverted to the base (f1194d4d) and the new tests in place, 6 of 20 fail — the five note-text cases plus the e2edeniedassertion. With the fix, 22/22 pass (bun test tests/modules/contact-auth-note-copy.test.ts tests/modules/contact-auth-gate-e2e.test.ts).contactAuthNoteText, one of which pins thesentnote as the shortest of the four, so nobody restores a redundant sentence.failed/suppressed, and pinning the outcome tonone— which is exactly why the two new e2e paths were added; both die now.bun biome checkclean on the three touched files,bunx tsc --noEmitclean.Proved live
Against a real Chatwoot (a local fork on
127.0.0.1:10997, account 5, API inbox 11) with the gate driven throughprocessChatwootDeliveryand a realChatwootClient, so the note is rendered by Chatwoot itself and read back over its API. Both halves, side by side:webhook.tsreverted to the base: the deny copy (msg 197) and, right below it, the note claiming the agent stayed silent (msg 198);denyMessage: nothing to the customer, and the note (msg 203) says so and why;Not proved here
tests/graph/tools-http-body-bound.test.ts(a memory-pressure test), which fails identically on a clean checkout of the base on this machine.suppressedstate (an earlier note that failed frees its own window while the copy's stays spent) is exercised in the test suite, not in the live run.🤖 Generated with Claude Code