fix: remove spec/plan artifacts from PR branch
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.7
@@ -1,261 +0,0 @@
|
|||||||
# References Header Threading Implementation Plan
|
|
||||||
|
|
||||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
||||||
|
|
||||||
**Goal:** Add `References` header fallback so threading resolves correctly when `In-Reply-To` is absent.
|
|
||||||
|
|
||||||
**Architecture:** Extract a `resolveThreadID` helper from `processMessage` that accepts a DB lookup function and parsed headers. This makes the threading logic testable without needing IMAP client infrastructure. The helper tries `In-Reply-To` first, then iterates `References` last-to-first on miss.
|
|
||||||
|
|
||||||
**Tech Stack:** Go, `github.com/emersion/go-message/mail`, `gorm.io/gorm`.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 1: Write failing tests for resolveThreadID
|
|
||||||
|
|
||||||
**Files:**
|
|
||||||
- Create: `internal/mail/resolve_thread_test.go`
|
|
||||||
- Modify: `internal/mail/imap.go` (add `resolveThreadID` stub)
|
|
||||||
|
|
||||||
- [ ] **Step 1: Write failing tests**
|
|
||||||
|
|
||||||
Create `internal/mail/resolve_thread_test.go`:
|
|
||||||
|
|
||||||
```go
|
|
||||||
package mail
|
|
||||||
|
|
||||||
import (
|
|
||||||
"errors"
|
|
||||||
"testing"
|
|
||||||
|
|
||||||
"thuanle.me/claw-email-bridge/internal/database"
|
|
||||||
)
|
|
||||||
|
|
||||||
func TestResolveThreadID_NoHeaders_ReturnsMessageID(t *testing.T) {
|
|
||||||
got := resolveThreadID("msg-1", "", "", nilLookup)
|
|
||||||
if got != "msg-1" {
|
|
||||||
t.Fatalf("expected msg-1, got %s", got)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestResolveThreadID_InReplyToMatches_ReturnsParentThreadID(t *testing.T) {
|
|
||||||
lookup := func(messageID string) (*database.Task, error) {
|
|
||||||
if messageID == "<parent>" {
|
|
||||||
return &database.Task{ThreadID: "thread-abc"}, nil
|
|
||||||
}
|
|
||||||
return nil, nil
|
|
||||||
}
|
|
||||||
got := resolveThreadID("msg-2", "<parent>", "", lookup)
|
|
||||||
if got != "thread-abc" {
|
|
||||||
t.Fatalf("expected thread-abc, got %s", got)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestResolveThreadID_ReferencesOnly_LastIDMatches(t *testing.T) {
|
|
||||||
lookup := func(messageID string) (*database.Task, error) {
|
|
||||||
if messageID == "<C>" {
|
|
||||||
return &database.Task{ThreadID: "thread-c"}, nil
|
|
||||||
}
|
|
||||||
return nil, nil
|
|
||||||
}
|
|
||||||
got := resolveThreadID("msg-3", "", "<A> <B> <C>", lookup)
|
|
||||||
if got != "thread-c" {
|
|
||||||
t.Fatalf("expected thread-c, got %s", got)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestResolveThreadID_ReferencesOnly_MiddleIDMatches(t *testing.T) {
|
|
||||||
lookup := func(messageID string) (*database.Task, error) {
|
|
||||||
if messageID == "<B>" {
|
|
||||||
return &database.Task{ThreadID: "thread-b"}, nil
|
|
||||||
}
|
|
||||||
return nil, nil
|
|
||||||
}
|
|
||||||
got := resolveThreadID("msg-4", "", "<A> <B> <C>", lookup)
|
|
||||||
if got != "thread-b" {
|
|
||||||
t.Fatalf("expected thread-b, got %s", got)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestResolveThreadID_ReferencesOnly_NoMatch_ReturnsMessageID(t *testing.T) {
|
|
||||||
got := resolveThreadID("msg-5", "", "<X> <Y>", nilLookup)
|
|
||||||
if got != "msg-5" {
|
|
||||||
t.Fatalf("expected msg-5, got %s", got)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestResolveThreadID_InReplyToMiss_ReferencesMatches(t *testing.T) {
|
|
||||||
lookup := func(messageID string) (*database.Task, error) {
|
|
||||||
if messageID == "<ref>" {
|
|
||||||
return &database.Task{ThreadID: "thread-ref"}, nil
|
|
||||||
}
|
|
||||||
return nil, nil
|
|
||||||
}
|
|
||||||
got := resolveThreadID("msg-6", "<unknown>", "<ref>", lookup)
|
|
||||||
if got != "thread-ref" {
|
|
||||||
t.Fatalf("expected thread-ref, got %s", got)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestResolveThreadID_LookupError_ReturnsMessageID(t *testing.T) {
|
|
||||||
lookup := func(messageID string) (*database.Task, error) {
|
|
||||||
return nil, errors.New("db error")
|
|
||||||
}
|
|
||||||
got := resolveThreadID("msg-7", "<parent>", "", lookup)
|
|
||||||
if got != "msg-7" {
|
|
||||||
t.Fatalf("expected msg-7, got %s", got)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
var nilLookup = func(string) (*database.Task, error) { return nil, nil }
|
|
||||||
```
|
|
||||||
|
|
||||||
- [ ] **Step 2: Run tests to verify they fail**
|
|
||||||
|
|
||||||
Run: `go test ./internal/mail -run ResolveThreadID -v`
|
|
||||||
Expected: FAIL — `resolveThreadID` not defined.
|
|
||||||
|
|
||||||
- [ ] **Step 3: Commit test file**
|
|
||||||
|
|
||||||
```bash
|
|
||||||
git add internal/mail/resolve_thread_test.go
|
|
||||||
git commit -m "test(mail): add resolveThreadID tests (RED)"
|
|
||||||
```
|
|
||||||
|
|
||||||
### Task 2: Implement resolveThreadID
|
|
||||||
|
|
||||||
**Files:**
|
|
||||||
- Modify: `internal/mail/imap.go`
|
|
||||||
|
|
||||||
- [ ] **Step 1: Add resolveThreadID function**
|
|
||||||
|
|
||||||
Add to `internal/mail/imap.go`, before the `processMessage` function:
|
|
||||||
|
|
||||||
```go
|
|
||||||
// resolveThreadID determines the thread_id for a message.
|
|
||||||
// It tries In-Reply-To first, then falls back to References (last-to-first).
|
|
||||||
// Returns the message's own ID if no parent is found.
|
|
||||||
func resolveThreadID(messageID, inReplyTo, references string, lookup func(string) (*database.Task, error)) string {
|
|
||||||
if inReplyTo != "" {
|
|
||||||
if parent, err := lookup(inReplyTo); err == nil && parent != nil {
|
|
||||||
return parent.ThreadID
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, refID := range parseReferences(references) {
|
|
||||||
if parent, err := lookup(refID); err == nil && parent != nil {
|
|
||||||
return parent.ThreadID
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return messageID
|
|
||||||
}
|
|
||||||
|
|
||||||
// parseReferences splits a References header value into individual message IDs,
|
|
||||||
// returned in reverse order (most recent first).
|
|
||||||
func parseReferences(header string) []string {
|
|
||||||
if header == "" {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
raw := strings.Fields(header)
|
|
||||||
for i := range raw {
|
|
||||||
raw[i] = strings.TrimSpace(raw[i])
|
|
||||||
}
|
|
||||||
// Reverse: last (most recent) first.
|
|
||||||
for i, j := 0, len(raw)-1; i < j; i, j = i+1, j-1 {
|
|
||||||
raw[i], raw[j] = raw[j], raw[i]
|
|
||||||
}
|
|
||||||
return raw
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
- [ ] **Step 2: Run tests to verify they pass**
|
|
||||||
|
|
||||||
Run: `go test ./internal/mail -run ResolveThreadID -v`
|
|
||||||
Expected: PASS.
|
|
||||||
|
|
||||||
- [ ] **Step 3: Commit**
|
|
||||||
|
|
||||||
```bash
|
|
||||||
git add internal/mail/imap.go
|
|
||||||
git commit -m "feat(mail): implement resolveThreadID with References fallback"
|
|
||||||
```
|
|
||||||
|
|
||||||
### Task 3: Wire resolveThreadID into processMessage
|
|
||||||
|
|
||||||
**Files:**
|
|
||||||
- Modify: `internal/mail/imap.go`
|
|
||||||
|
|
||||||
- [ ] **Step 1: Replace threading block in processMessage**
|
|
||||||
|
|
||||||
Replace the block at `imap.go:333-369` (the threading section) with:
|
|
||||||
|
|
||||||
```go
|
|
||||||
// 4. Threading: determine thread_id.
|
|
||||||
threadID := messageID
|
|
||||||
inReplyTo := ""
|
|
||||||
references := ""
|
|
||||||
|
|
||||||
// Parse body to get In-Reply-To / References headers and plain text.
|
|
||||||
bodyPlain := ""
|
|
||||||
rawBody := buf.FindBodySection(bodySection)
|
|
||||||
if rawBody != nil {
|
|
||||||
mr, err := gomessage.CreateReader(io.NopCloser(strings.NewReader(string(rawBody))))
|
|
||||||
if err == nil {
|
|
||||||
if irt, err := mr.Header.Text("In-Reply-To"); err == nil && irt != "" {
|
|
||||||
inReplyTo = strings.TrimSpace(irt)
|
|
||||||
}
|
|
||||||
if refs, err := mr.Header.Text("References"); err == nil && refs != "" {
|
|
||||||
references = refs
|
|
||||||
}
|
|
||||||
|
|
||||||
for {
|
|
||||||
p, err := mr.NextPart()
|
|
||||||
if err != nil {
|
|
||||||
break
|
|
||||||
}
|
|
||||||
if _, ok := p.Header.(*gomessage.InlineHeader); ok {
|
|
||||||
b, _ := io.ReadAll(p.Body)
|
|
||||||
if bodyPlain == "" {
|
|
||||||
bodyPlain = string(b)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
threadID = resolveThreadID(messageID, inReplyTo, references, func(mid string) (*database.Task, error) {
|
|
||||||
return database.FindByMessageID(w.db, mid)
|
|
||||||
})
|
|
||||||
```
|
|
||||||
|
|
||||||
- [ ] **Step 2: Run full test suite**
|
|
||||||
|
|
||||||
Run: `go test ./internal/mail -v`
|
|
||||||
Expected: PASS (all ResolveThreadID + existing dial tests).
|
|
||||||
|
|
||||||
- [ ] **Step 3: Run full project tests**
|
|
||||||
|
|
||||||
Run: `go test ./...`
|
|
||||||
Expected: PASS.
|
|
||||||
|
|
||||||
- [ ] **Step 4: Commit**
|
|
||||||
|
|
||||||
```bash
|
|
||||||
git add internal/mail/imap.go
|
|
||||||
git commit -m "feat(mail): wire resolveThreadID into processMessage"
|
|
||||||
```
|
|
||||||
|
|
||||||
### Task 4: Final verification
|
|
||||||
|
|
||||||
- [ ] **Step 1: Run full validation**
|
|
||||||
|
|
||||||
Run: `go build ./... && go vet ./... && go test ./... -v`
|
|
||||||
Expected: all PASS.
|
|
||||||
|
|
||||||
- [ ] **Step 2: Push and create PR**
|
|
||||||
|
|
||||||
```bash
|
|
||||||
git push -u origin <branch>
|
|
||||||
```
|
|
||||||
|
|
||||||
PR body must include `Fixes #4`.
|
|
||||||
@@ -1,44 +0,0 @@
|
|||||||
# Threading: Add References Header Fallback
|
|
||||||
|
|
||||||
## Problem
|
|
||||||
|
|
||||||
Ingress threading only resolves parent via `In-Reply-To`. When a client sets `References` without `In-Reply-To`, the bridge creates a new `thread_id` instead of attaching to the existing conversation, breaking thread continuity and downstream dispatch.
|
|
||||||
|
|
||||||
## Requirements
|
|
||||||
|
|
||||||
From `requirements.md:74`:
|
|
||||||
- If `In-Reply-To` or `References` is present, find parent task in DB and inherit `thread_id`.
|
|
||||||
- Otherwise use `message_id` as new `thread_id`.
|
|
||||||
|
|
||||||
## Design
|
|
||||||
|
|
||||||
### Current behavior (`imap.go:333-369`)
|
|
||||||
|
|
||||||
1. Parse MIME body headers, extract `In-Reply-To`
|
|
||||||
2. If found, lookup parent by message ID → inherit `thread_id`
|
|
||||||
|
|
||||||
### New behavior
|
|
||||||
|
|
||||||
1. Parse MIME body headers, extract both `In-Reply-To` and `References`
|
|
||||||
2. Try `In-Reply-To` first (unchanged)
|
|
||||||
3. If no parent found via `In-Reply-To`, parse `References` header (space-separated `<msg-id>` list)
|
|
||||||
4. Iterate `References` from **last to first** (most recent ancestor first)
|
|
||||||
5. First match in DB wins → inherit `thread_id`
|
|
||||||
|
|
||||||
### Why last-to-first
|
|
||||||
|
|
||||||
`References` lists message IDs chronologically: `<oldest> ... <newest>`. The last entry is the most recent message in the chain, most likely already processed by the bridge.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
- Single file change: `internal/mail/imap.go` (threading block in `processMessage`, ~10 lines)
|
|
||||||
- New tests: `internal/mail/imap_test.go` (References parsing, fallback, multi-ID)
|
|
||||||
- No changes to DB, config, API, or other packages
|
|
||||||
|
|
||||||
### Test cases
|
|
||||||
|
|
||||||
1. `In-Reply-To` matches → existing behavior unchanged
|
|
||||||
2. `In-Reply-To` absent, `References` has match → inherits thread_id from most recent match
|
|
||||||
3. `References` has multiple IDs, only older one in DB → still matches
|
|
||||||
4. Both headers absent → thread_id = message_id (unchanged)
|
|
||||||
5. `References` present but no match in DB → thread_id = message_id (new thread)
|
|
||||||
Reference in New Issue
Block a user