hir-124: dedupe replyTriage + createReply on duplicate webhook delivery#16
Open
jaredzwick wants to merge 1 commit intopypesdev:hir-120/warm-reply-triagefrom
Open
Conversation
Gate the warm-reply triage path on the same alreadyReplied check the event-write already had, so a redelivered Gmail Pub/Sub (or repeated IMAP poll) hit doesn't double-write the reply row, double-bill the LLM triage call, or cancel a still-scheduled silent follow-up twice. Adds a unique index on reply (contact_id, event_id) as a belt-and-braces DB-layer invariant — Postgres treats NULL as distinct so legacy rows with a null event_id won't collide. Test additions: a two-delivery test that asserts triageReply, createReply, createEmailEvent, createReplyFollowup, and incrementCampaignStat each fire exactly once across the two calls; and rewrites the existing "alreadyReplied" test to match the new duplicate_delivery short-circuit (the old test was implicitly conflating duplicate webhooks with genuine new replies, which is the bug HIR-124 is fixing).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
recordInboundReplyonalreadyRepliedso a redelivered Gmail Pub/Sub (or repeated IMAP poll) hit doesn't double-write thereplyrow, double-bill the LLM triage call, double-cancel a still-scheduled silent follow-up, or duplicate cards in/dashboard/replies. Returnsreason: "duplicate_delivery"so callers can log the dedup explicitly.reply (contact_id, event_id)(drizzle migration0009_damp_mulholland_black.sql) as belt-and-braces in case theeventExistsForTrackingcheck ever races. Postgres treats NULL as distinct, so legacy rows with a nullevent_idwon't collide.triageReply,createReply,createEmailEvent,createReplyFollowup, andincrementCampaignStateach fire exactly once across the two calls. Rewrote the existing "alreadyReplied" test to match the new short-circuit semantics — the old test was implicitly conflating duplicate webhooks with genuine new replies, which is the bug HIR-124 is fixing.Why this branches off PR #14
This is the follow-up CTO review surfaced on HIR-122 / PR #14. Branching off
hir-120/warm-reply-triageso it can land in the same week as v1 of the warm-reply UI without sitting behind a main merge.Behavior note worth a CTO eye
With this change, a genuine second reply from the same prospect against the same outbound (same
tracking_id) will also short-circuit — v1 of the warm-reply UI doesn't model multi-turn replies. That's the correct trade for v1 (the founder will almost always notice the new reply via Gmail UI), but if we want multi-turn we'll need a different dedup key (e.g. message-id, not tracking-id).Test plan
pnpm vitest run tests/int/replyFollowup.int.spec.ts— 14/14 passpnpm tsc --noEmit -p tsconfig.json— cleanpnpm vitest run— 110/110 pass (the one failed file,api.int.spec.ts, is a pre-existing Payload secret-key env issue on the base branch, not introduced here — verified by re-running it from base)pnpm db:migrateagainst staging to apply0009_damp_mulholland_black.sql