From b99997b51f449dc63e1c9e595cf8aaa94cf59df5 Mon Sep 17 00:00:00 2001 From: joao-crm Date: Tue, 8 Sep 2026 00:10:21 +0000 Subject: [PATCH] fix: match an inbound Message-ID with and without its RFC 5322 angle brackets in GetTaskByMessageID, since the outbound stamp always stores the bracketed form while an In-Reply-To header arrives either way depending on the replying client, so every reply whose brackets had been stripped missed its task, replied_at was never stamped, stop_on_reply stayed inert and a contact who had already answered kept receiving the rest of the sequence, and an empty header matched one of the 151 tasks carrying an empty message_id and linked the reply to an arbitrary campaign --- internal/repository/pg_task.go | 12 ++- .../repository/task_message_id_live_test.go | 101 ++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 internal/repository/task_message_id_live_test.go diff --git a/internal/repository/pg_task.go b/internal/repository/pg_task.go index 2db2816b..5f49b27e 100644 --- a/internal/repository/pg_task.go +++ b/internal/repository/pg_task.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "strings" "time" "github.com/google/uuid" @@ -293,19 +294,24 @@ func (r *taskRepository) GetTask(ctx context.Context, taskID uuid.UUID) (*Task, return task, err } -// GetTaskByMessageID retrieves the latest task by RFC Message-ID. +// GetTaskByMessageID retrieves the latest task by RFC Message-ID. Probes both +// bracket forms: the stamp stores "", an inbound In-Reply-To may not. func (r *taskRepository) GetTaskByMessageID(ctx context.Context, messageID string) (*Task, error) { + bare := strings.Trim(strings.TrimSpace(messageID), "<>") + if bare == "" { + return nil, nil + } query := ` SELECT id, task_type, email_account_id, status, message_id, scheduled_at, completed_at, cloud_task_name, created_at, updated_at FROM tasks - WHERE message_id = $1 + WHERE message_id = $1 OR message_id = $2 ORDER BY created_at DESC LIMIT 1 ` task := &Task{} - err := r.db.QueryRow(ctx, query, messageID).Scan( + err := r.db.QueryRow(ctx, query, bare, "<"+bare+">").Scan( &task.ID, &task.TaskType, &task.EmailAccountID, diff --git a/internal/repository/task_message_id_live_test.go b/internal/repository/task_message_id_live_test.go new file mode 100644 index 00000000..5d862653 --- /dev/null +++ b/internal/repository/task_message_id_live_test.go @@ -0,0 +1,101 @@ +package repository + +import ( + "context" + "testing" + + "github.com/google/uuid" +) + +// The stamp stores "" but an inbound In-Reply-To arrives either way, +// and cleanMessageID strips the brackets before the lookup. Under the old exact +// compare the bare-form case found nothing, so replied_at was never stamped and +// stop_on_reply kept mailing a contact who had already answered. +// +// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \ +// go test ./internal/repository/ -run LiveTaskByMessageID -v +func TestLiveTaskByMessageIDMatchesBracketedAndBareForms(t *testing.T) { + _, pool := liveContactDB(t) + ctx := context.Background() + + org := uuid.New() + user := uuid.New() + account := uuid.New() + task := uuid.New() + bare := "regression-" + uuid.NewString() + "@example.test" + stored := "<" + bare + ">" + + exec := func(sql string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("setup %q: %v", sql, err) + } + } + + // The user exists first: organizations.owner_user_id points at it. + exec(`INSERT INTO users (id, first_name, last_name, email) + VALUES ($1, 'MsgID', 'Regression', $2)`, + user, "msgid-"+uuid.NewString()+"@example.test") + exec(`INSERT INTO organizations (id, name, owner_user_id) + VALUES ($1, 'msgid regression', $2)`, org, user) + exec(`INSERT INTO email_accounts + (id, user_id, organization_id, email, name, signature_plain, signature_html, provider) + VALUES ($1, $2, $3, $4, 'MsgID Regression', '', '', 'smtp_imap')`, + account, user, org, "box-"+uuid.NewString()+"@example.test") + exec(`INSERT INTO tasks (id, task_type, email_account_id, status, message_id) + VALUES ($1, 'campaign', $2, 'completed', $3)`, task, account, stored) + + t.Cleanup(func() { + // Unwound in dependency order: tasks cascade from the mailbox, the + // mailbox and the org do not cascade from each other, and the org + // owns the user by foreign key, so the user goes last. + for _, stmt := range []struct { + sql string + arg any + }{ + {`DELETE FROM email_accounts WHERE id = $1`, account}, + {`DELETE FROM organizations WHERE id = $1`, org}, + {`DELETE FROM users WHERE id = $1`, user}, + } { + if _, err := pool.Exec(context.Background(), stmt.sql, stmt.arg); err != nil { + t.Errorf("cleanup %q: %v", stmt.sql, err) + } + } + }) + + repo := NewTaskRepository(pool) + + for _, tc := range []struct { + name string + lookup string + }{ + {"bare form, as cleanMessageID hands it over", bare}, + {"bracketed form, as the header carried it", stored}, + {"bracketed with surrounding space", " " + stored + " "}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := repo.GetTaskByMessageID(ctx, tc.lookup) + if err != nil { + t.Fatalf("lookup %q: %v", tc.lookup, err) + } + if got == nil { + t.Fatalf("lookup %q found no task; the reply would never link to its campaign", tc.lookup) + } + if got.ID != task { + t.Fatalf("lookup %q returned task %s, want %s", tc.lookup, got.ID, task) + } + }) + } + + t.Run("empty message id is not a wildcard", func(t *testing.T) { + for _, empty := range []string{"", " ", "<>"} { + got, err := repo.GetTaskByMessageID(ctx, empty) + if err != nil { + t.Fatalf("lookup %q: %v", empty, err) + } + if got != nil { + t.Fatalf("lookup %q returned task %s; an absent header must match nothing", empty, got.ID) + } + } + }) +}