From 4090a44524464d0b16d4aacec2442a3135da020a Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 04:51:38 +0700 Subject: [PATCH 1/8] docs: add References header threading design spec Co-Authored-By: Claude Opus 4.7 --- .../2026-04-28-references-threading-design.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-28-references-threading-design.md diff --git a/docs/superpowers/specs/2026-04-28-references-threading-design.md b/docs/superpowers/specs/2026-04-28-references-threading-design.md new file mode 100644 index 0000000..139bebf --- /dev/null +++ b/docs/superpowers/specs/2026-04-28-references-threading-design.md @@ -0,0 +1,44 @@ +# 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 `` 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: ` ... `. 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) -- 2.54.0 From 270b13834addf81dc210c23624d4785928a418a0 Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 04:57:00 +0700 Subject: [PATCH 2/8] docs: add References threading implementation plan Co-Authored-By: Claude Opus 4.7 --- .../plans/2026-04-28-references-threading.md | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-28-references-threading.md diff --git a/docs/superpowers/plans/2026-04-28-references-threading.md b/docs/superpowers/plans/2026-04-28-references-threading.md new file mode 100644 index 0000000..d3823d6 --- /dev/null +++ b/docs/superpowers/plans/2026-04-28-references-threading.md @@ -0,0 +1,261 @@ +# 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 == "" { + return &database.Task{ThreadID: "thread-abc"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-2", "", "", 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 == "" { + return &database.Task{ThreadID: "thread-c"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-3", "", " ", 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 == "" { + return &database.Task{ThreadID: "thread-b"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-4", "", " ", 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", "", " ", 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 == "" { + return &database.Task{ThreadID: "thread-ref"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-6", "", "", 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", "", "", 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 +``` + +PR body must include `Fixes #4`. -- 2.54.0 From aebae1cea50b73695810aee030b71ff6af555f83 Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 05:01:17 +0700 Subject: [PATCH 3/8] test(mail): add resolveThreadID tests (RED) Co-Authored-By: Claude Opus 4.7 --- internal/mail/resolve_thread_test.go | 86 ++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 internal/mail/resolve_thread_test.go diff --git a/internal/mail/resolve_thread_test.go b/internal/mail/resolve_thread_test.go new file mode 100644 index 0000000..9e52251 --- /dev/null +++ b/internal/mail/resolve_thread_test.go @@ -0,0 +1,86 @@ +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 == "" { + return &database.Task{ThreadID: "thread-abc"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-2", "", "", 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 == "" { + return &database.Task{ThreadID: "thread-c"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-3", "", " ", 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 == "" { + return &database.Task{ThreadID: "thread-b"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-4", "", " ", 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", "", " ", 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 == "" { + return &database.Task{ThreadID: "thread-ref"}, nil + } + return nil, nil + } + got := resolveThreadID("msg-6", "", "", 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", "", "", lookup) + if got != "msg-7" { + t.Fatalf("expected msg-7, got %s", got) + } +} + +var nilLookup = func(string) (*database.Task, error) { return nil, nil } \ No newline at end of file -- 2.54.0 From c63895f4da2dbb3818ffb506678d63ec31268438 Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 05:02:11 +0700 Subject: [PATCH 4/8] feat(mail): implement resolveThreadID with References fallback Co-Authored-By: Claude Opus 4.7 --- internal/mail/imap.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/internal/mail/imap.go b/internal/mail/imap.go index 3536af5..db2acca 100644 --- a/internal/mail/imap.go +++ b/internal/mail/imap.go @@ -283,6 +283,42 @@ func (w *IMAPWatcher) fetchUnseen(c *imapclient.Client) error { return nil } +// 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 +} + // processMessage handles a single fetched email message. func (w *IMAPWatcher) processMessage(c *imapclient.Client, buf *imapclient.FetchMessageBuffer, bodySection *imap.FetchItemBodySection) { env := buf.Envelope -- 2.54.0 From 15e697ddf377c4bc30278a254a9c4280fc70cab8 Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 05:04:45 +0700 Subject: [PATCH 5/8] fix(mail): remove redundant TrimSpace in parseReferences strings.Fields already strips whitespace from each token. Co-Authored-By: Claude Opus 4.7 --- internal/mail/imap.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/mail/imap.go b/internal/mail/imap.go index db2acca..0cf84ed 100644 --- a/internal/mail/imap.go +++ b/internal/mail/imap.go @@ -309,9 +309,6 @@ func parseReferences(header string) []string { 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] -- 2.54.0 From c55a27d710e5d282c4f506d8cd62102634381bc9 Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 05:07:40 +0700 Subject: [PATCH 6/8] feat(mail): wire resolveThreadID into processMessage - Add references variable to extract References header - Replace manual inReplyTo handling with resolveThreadID call - Maintain existing body parsing functionality - Includes proper lambda function for database lookups Co-Authored-By: Claude Opus 4.7 --- .../plans/2026-04-27-callback-base-url.md | 172 ++++++++++++++ .../plans/2026-04-27-imap-proxy-support.md | 224 ++++++++++++++++++ internal/mail/imap.go | 14 +- 3 files changed, 403 insertions(+), 7 deletions(-) create mode 100644 docs/superpowers/plans/2026-04-27-callback-base-url.md create mode 100644 docs/superpowers/plans/2026-04-27-imap-proxy-support.md diff --git a/docs/superpowers/plans/2026-04-27-callback-base-url.md b/docs/superpowers/plans/2026-04-27-callback-base-url.md new file mode 100644 index 0000000..537a4af --- /dev/null +++ b/docs/superpowers/plans/2026-04-27-callback-base-url.md @@ -0,0 +1,172 @@ +# Callback Base URL 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:** Separate server bind address from callback URL generation and default callback base to localhost when unset. + +**Architecture:** Keep `LISTEN_ADDR` only for HTTP server binding. Add `CALLBACK_BASE_URL` in config, default to `http://localhost` when empty, and build dispatch callback URL from this base plus `/callback`. + +**Tech Stack:** Go, stdlib `net/url` + `strings`, existing project packages. + +--- + +### Task 1: Add callback base URL config with localhost fallback + +**Files:** +- Modify: `internal/config/config.go` +- Test: `internal/config/config_test.go` (create) + +- [ ] **Step 1: Write the failing tests** + +```go +func TestLoad_DefaultCallbackBaseURL_WhenUnset(t *testing.T) { + t.Setenv("CALLBACK_BASE_URL", "") + setRequiredEnv(t) + + cfg, err := Load() + require.NoError(t, err) + require.Equal(t, "http://localhost", cfg.CallbackBaseURL) +} + +func TestLoad_UsesCallbackBaseURL_WhenSet(t *testing.T) { + t.Setenv("CALLBACK_BASE_URL", "https://bridge.example.com") + setRequiredEnv(t) + + cfg, err := Load() + require.NoError(t, err) + require.Equal(t, "https://bridge.example.com", cfg.CallbackBaseURL) +} +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/config -run CallbackBaseURL -v` +Expected: FAIL because `Config` has no `CallbackBaseURL` field yet. + +- [ ] **Step 3: Write minimal implementation** + +```go +type Config struct { + // ...existing fields... + CallbackBaseURL string +} + +cfg := &Config{ + // ...existing env reads... + CallbackBaseURL: os.Getenv("CALLBACK_BASE_URL"), +} + +if cfg.CallbackBaseURL == "" { + cfg.CallbackBaseURL = "http://localhost" +} +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `go test ./internal/config -run CallbackBaseURL -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add internal/config/config.go internal/config/config_test.go +git commit -m "feat(config): add callback base URL with localhost default" +``` + +### Task 2: Build callback URL from callback base URL + +**Files:** +- Modify: `internal/ai_client/client.go` +- Test: `internal/ai_client/client_test.go` (create) + +- [ ] **Step 1: Write the failing tests** + +```go +func TestBuildCallbackURL_DefaultLocalhostBase(t *testing.T) { + got := buildCallbackURL("http://localhost") + require.Equal(t, "http://localhost/callback", got) +} + +func TestBuildCallbackURL_TrimsTrailingSlash(t *testing.T) { + got := buildCallbackURL("https://bridge.example.com/") + require.Equal(t, "https://bridge.example.com/callback", got) +} +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/ai_client -run BuildCallbackURL -v` +Expected: FAIL because helper function does not exist. + +- [ ] **Step 3: Write minimal implementation** + +```go +func buildCallbackURL(base string) string { + base = strings.TrimRight(base, "/") + return base + "/callback" +} + +// in Dispatch: +callbackURL := buildCallbackURL(d.cfg.CallbackBaseURL) +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `go test ./internal/ai_client -run BuildCallbackURL -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add internal/ai_client/client.go internal/ai_client/client_test.go +git commit -m "fix(ai-client): use callback base URL for callback endpoint" +``` + +### Task 3: Update environment docs/examples + +**Files:** +- Modify: `.env.example` +- Modify: `README.md` + +- [ ] **Step 1: Add config key to `.env.example`** + +```dotenv +LISTEN_ADDR=:8080 +CALLBACK_BASE_URL=http://localhost +``` + +- [ ] **Step 2: Document purpose in README** + +```md +- `LISTEN_ADDR`: local bind address for HTTP server (e.g. `:8080`) +- `CALLBACK_BASE_URL`: public base URL used to build callback endpoint; defaults to `http://localhost` when unset +``` + +- [ ] **Step 3: Run full test suite** + +Run: `go test ./...` +Expected: PASS. + +- [ ] **Step 4: Commit** + +```bash +git add .env.example README.md +git commit -m "docs: document callback base URL configuration" +``` + +### Task 4: Prepare PR + +**Files:** +- No code changes expected + +- [ ] **Step 1: Push branch** + +Run: `git push -u origin ` +Expected: branch published. + +- [ ] **Step 2: Open PR with summary and test evidence** + +Include: +- Problem: callback URL incorrectly built from listen addr +- Fix: separate `CALLBACK_BASE_URL` with localhost fallback +- Tests: `go test ./internal/config -run CallbackBaseURL -v`, `go test ./internal/ai_client -run BuildCallbackURL -v`, `go test ./...` diff --git a/docs/superpowers/plans/2026-04-27-imap-proxy-support.md b/docs/superpowers/plans/2026-04-27-imap-proxy-support.md new file mode 100644 index 0000000..30879bf --- /dev/null +++ b/docs/superpowers/plans/2026-04-27-imap-proxy-support.md @@ -0,0 +1,224 @@ +# IMAP Proxy Support 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:** Make IMAP connections route through `IMAP_PROXY_URL` when configured, while keeping current direct dial behavior when unset. + +**Architecture:** Introduce a small dial abstraction in IMAP watcher so connection setup is testable and can branch between direct TLS dial and SOCKS5 proxy dial. Keep authentication/fetch logic unchanged; only connection creation path changes. Validate proxy URL format early and fail clearly when invalid. + +**Tech Stack:** Go, `github.com/emersion/go-imap/v2/imapclient`, `golang.org/x/net/proxy`, `crypto/tls`. + +--- + +### Task 1: Add failing tests for dial-path behavior + +**Files:** +- Modify: `internal/mail/imap_test.go` (create) +- Modify: `internal/mail/imap.go` (for injectable dial function used by tests) + +- [ ] **Step 1: Write failing test for direct dial when proxy unset** + +```go +func TestConnectAndWatch_UsesDirectDial_WhenProxyUnset(t *testing.T) { + watcher := &IMAPWatcher{cfg: &config.Config{IMAPHost: "imap.example.com", IMAPPort: "993", IMAPUser: "u", IMAPPass: "p"}} + + var directCalled bool + watcher.dialIMAP = func(addr string, _ *imapclient.Options) (*imapclient.Client, error) { + directCalled = true + return nil, errors.New("stop") + } + + _ = watcher.connectAndWatch(context.Background()) + + if !directCalled { + t.Fatal("expected direct dial path") + } +} +``` + +- [ ] **Step 2: Write failing test for proxy dial when proxy set** + +```go +func TestConnectAndWatch_UsesProxyDial_WhenProxySet(t *testing.T) { + watcher := &IMAPWatcher{cfg: &config.Config{IMAPHost: "imap.example.com", IMAPPort: "993", IMAPUser: "u", IMAPPass: "p", IMAPProxyURL: "socks5://127.0.0.1:1080"}} + + var proxyCalled bool + watcher.dialIMAPViaProxy = func(addr, proxyURL string, _ *imapclient.Options) (*imapclient.Client, error) { + proxyCalled = true + return nil, errors.New("stop") + } + + _ = watcher.connectAndWatch(context.Background()) + + if !proxyCalled { + t.Fatal("expected proxy dial path") + } +} +``` + +- [ ] **Step 3: Run tests to verify RED** + +Run: `go test ./internal/mail -run Uses -v` +Expected: FAIL because `dialIMAP` / `dialIMAPViaProxy` hooks do not exist yet. + +- [ ] **Step 4: Commit test scaffold after it compiles with new hooks (still failing behavior checks if needed)** + +```bash +git add internal/mail/imap_test.go internal/mail/imap.go +git commit -m "test(mail): add dial path tests for IMAP proxy" +``` + +### Task 2: Implement proxy-aware dialing with minimal changes + +**Files:** +- Modify: `internal/mail/imap.go` + +- [ ] **Step 1: Add dial hooks to watcher for testability** + +```go +type IMAPWatcher struct { + cfg *config.Config + db *gorm.DB + client *imapclient.Client + + dialIMAP func(addr string, options *imapclient.Options) (*imapclient.Client, error) + dialIMAPViaProxy func(addr, proxyURL string, options *imapclient.Options) (*imapclient.Client, error) + + OnReceived func(task *database.Task) +} +``` + +- [ ] **Step 2: Initialize default dial hooks in constructor** + +```go +func NewIMAPWatcher(cfg *config.Config, db *gorm.DB) *IMAPWatcher { + return &IMAPWatcher{ + cfg: cfg, + db: db, + dialIMAP: imapclient.DialTLS, + dialIMAPViaProxy: dialTLSViaSOCKS5, + } +} +``` + +- [ ] **Step 3: Branch connection setup by `IMAPProxyURL`** + +```go +if strings.TrimSpace(w.cfg.IMAPProxyURL) == "" { + c, err = w.dialIMAP(addr, options) +} else { + c, err = w.dialIMAPViaProxy(addr, w.cfg.IMAPProxyURL, options) +} +if err != nil { + return fmt.Errorf("dial IMAP: %w", err) +} +``` + +- [ ] **Step 4: Implement SOCKS5 TLS dial helper** + +```go +func dialTLSViaSOCKS5(addr, proxyURL string, options *imapclient.Options) (*imapclient.Client, error) { + u, err := url.Parse(proxyURL) + if err != nil { + return nil, fmt.Errorf("parse proxy url: %w", err) + } + if u.Scheme != "socks5" { + return nil, fmt.Errorf("unsupported proxy scheme: %s", u.Scheme) + } + + var auth *proxy.Auth + if u.User != nil { + pw, _ := u.User.Password() + auth = &proxy.Auth{User: u.User.Username(), Password: pw} + } + + d, err := proxy.SOCKS5("tcp", u.Host, auth, proxy.Direct) + if err != nil { + return nil, fmt.Errorf("create socks5 dialer: %w", err) + } + + conn, err := d.Dial("tcp", addr) + if err != nil { + return nil, fmt.Errorf("proxy dial: %w", err) + } + + host, _, _ := net.SplitHostPort(addr) + tlsConn := tls.Client(conn, &tls.Config{ServerName: host}) + if err := tlsConn.Handshake(); err != nil { + _ = tlsConn.Close() + return nil, fmt.Errorf("tls handshake: %w", err) + } + + return imapclient.New(tlsConn, options), nil +} +``` + +- [ ] **Step 5: Run focused tests to verify GREEN** + +Run: `go test ./internal/mail -run Uses -v` +Expected: PASS. + +- [ ] **Step 6: Commit implementation** + +```bash +git add internal/mail/imap.go internal/mail/imap_test.go +git commit -m "feat(mail): support IMAP SOCKS5 proxy dialing" +``` + +### Task 3: Add URL validation coverage and docs consistency + +**Files:** +- Modify: `internal/mail/imap_test.go` +- Modify: `README.md` (only if wording needs tighter scheme requirement) + +- [ ] **Step 1: Write test for invalid proxy scheme** + +```go +func TestDialTLSViaSOCKS5_RejectsNonSocks5Scheme(t *testing.T) { + _, err := dialTLSViaSOCKS5("imap.example.com:993", "http://proxy:8080", nil) + if err == nil || !strings.Contains(err.Error(), "unsupported proxy scheme") { + t.Fatalf("expected unsupported scheme error, got %v", err) + } +} +``` + +- [ ] **Step 2: Run targeted test** + +Run: `go test ./internal/mail -run RejectsNonSocks5Scheme -v` +Expected: PASS. + +- [ ] **Step 3: Ensure README explicitly says socks5 scheme (already present, keep if unchanged)** + +```md +IMAP_PROXY_URL: socks5://user:pass@host:port +``` + +- [ ] **Step 4: Commit doc/test alignment** + +```bash +git add internal/mail/imap_test.go README.md +git commit -m "test/docs: validate IMAP proxy URL scheme" +``` + +### Task 4: Final verification + +**Files:** +- No new files expected + +- [ ] **Step 1: Run package tests for mail** + +Run: `go test ./internal/mail -v` +Expected: PASS. + +- [ ] **Step 2: Run full suite** + +Run: `go test ./...` +Expected: PASS. + +- [ ] **Step 3: Push and update issue/PR with evidence** + +```bash +git push -u origin +``` + +Include commands and pass results in issue/PR comment. diff --git a/internal/mail/imap.go b/internal/mail/imap.go index 0cf84ed..6fe6f32 100644 --- a/internal/mail/imap.go +++ b/internal/mail/imap.go @@ -366,6 +366,7 @@ func (w *IMAPWatcher) processMessage(c *imapclient.Client, buf *imapclient.Fetch // 4. Threading: determine thread_id. threadID := messageID inReplyTo := "" + references := "" // Parse body to get In-Reply-To / References headers and plain text. bodyPlain := "" @@ -373,10 +374,12 @@ func (w *IMAPWatcher) processMessage(c *imapclient.Client, buf *imapclient.Fetch if rawBody != nil { mr, err := gomessage.CreateReader(io.NopCloser(strings.NewReader(string(rawBody)))) if err == nil { - // Extract In-Reply-To header. 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 + } // Read body parts for plain text. for { @@ -394,12 +397,9 @@ func (w *IMAPWatcher) processMessage(c *imapclient.Client, buf *imapclient.Fetch } } - if inReplyTo != "" { - parent, err := database.FindByMessageID(w.db, inReplyTo) - if err == nil && parent != nil { - threadID = parent.ThreadID - } - } + threadID = resolveThreadID(messageID, inReplyTo, references, func(mid string) (*database.Task, error) { + return database.FindByMessageID(w.db, mid) + }) // 5. Save task with status RECEIVED. taskUUID := uuid.New().String() -- 2.54.0 From 4145f7a50b308754624dc89e683ba8b1c2b37d40 Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 05:18:18 +0700 Subject: [PATCH 7/8] fix: remove out-of-scope plan files and gofmt test file Address review feedback on PR #13: - Remove docs/superpowers/plans/ files not related to issue #4 - gofmt internal/mail/resolve_thread_test.go Co-Authored-By: Claude Opus 4.7 --- .../plans/2026-04-27-callback-base-url.md | 172 -------------- .../plans/2026-04-27-imap-proxy-support.md | 224 ------------------ internal/mail/resolve_thread_test.go | 2 +- 3 files changed, 1 insertion(+), 397 deletions(-) delete mode 100644 docs/superpowers/plans/2026-04-27-callback-base-url.md delete mode 100644 docs/superpowers/plans/2026-04-27-imap-proxy-support.md diff --git a/docs/superpowers/plans/2026-04-27-callback-base-url.md b/docs/superpowers/plans/2026-04-27-callback-base-url.md deleted file mode 100644 index 537a4af..0000000 --- a/docs/superpowers/plans/2026-04-27-callback-base-url.md +++ /dev/null @@ -1,172 +0,0 @@ -# Callback Base URL 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:** Separate server bind address from callback URL generation and default callback base to localhost when unset. - -**Architecture:** Keep `LISTEN_ADDR` only for HTTP server binding. Add `CALLBACK_BASE_URL` in config, default to `http://localhost` when empty, and build dispatch callback URL from this base plus `/callback`. - -**Tech Stack:** Go, stdlib `net/url` + `strings`, existing project packages. - ---- - -### Task 1: Add callback base URL config with localhost fallback - -**Files:** -- Modify: `internal/config/config.go` -- Test: `internal/config/config_test.go` (create) - -- [ ] **Step 1: Write the failing tests** - -```go -func TestLoad_DefaultCallbackBaseURL_WhenUnset(t *testing.T) { - t.Setenv("CALLBACK_BASE_URL", "") - setRequiredEnv(t) - - cfg, err := Load() - require.NoError(t, err) - require.Equal(t, "http://localhost", cfg.CallbackBaseURL) -} - -func TestLoad_UsesCallbackBaseURL_WhenSet(t *testing.T) { - t.Setenv("CALLBACK_BASE_URL", "https://bridge.example.com") - setRequiredEnv(t) - - cfg, err := Load() - require.NoError(t, err) - require.Equal(t, "https://bridge.example.com", cfg.CallbackBaseURL) -} -``` - -- [ ] **Step 2: Run tests to verify failure** - -Run: `go test ./internal/config -run CallbackBaseURL -v` -Expected: FAIL because `Config` has no `CallbackBaseURL` field yet. - -- [ ] **Step 3: Write minimal implementation** - -```go -type Config struct { - // ...existing fields... - CallbackBaseURL string -} - -cfg := &Config{ - // ...existing env reads... - CallbackBaseURL: os.Getenv("CALLBACK_BASE_URL"), -} - -if cfg.CallbackBaseURL == "" { - cfg.CallbackBaseURL = "http://localhost" -} -``` - -- [ ] **Step 4: Run tests to verify pass** - -Run: `go test ./internal/config -run CallbackBaseURL -v` -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add internal/config/config.go internal/config/config_test.go -git commit -m "feat(config): add callback base URL with localhost default" -``` - -### Task 2: Build callback URL from callback base URL - -**Files:** -- Modify: `internal/ai_client/client.go` -- Test: `internal/ai_client/client_test.go` (create) - -- [ ] **Step 1: Write the failing tests** - -```go -func TestBuildCallbackURL_DefaultLocalhostBase(t *testing.T) { - got := buildCallbackURL("http://localhost") - require.Equal(t, "http://localhost/callback", got) -} - -func TestBuildCallbackURL_TrimsTrailingSlash(t *testing.T) { - got := buildCallbackURL("https://bridge.example.com/") - require.Equal(t, "https://bridge.example.com/callback", got) -} -``` - -- [ ] **Step 2: Run tests to verify failure** - -Run: `go test ./internal/ai_client -run BuildCallbackURL -v` -Expected: FAIL because helper function does not exist. - -- [ ] **Step 3: Write minimal implementation** - -```go -func buildCallbackURL(base string) string { - base = strings.TrimRight(base, "/") - return base + "/callback" -} - -// in Dispatch: -callbackURL := buildCallbackURL(d.cfg.CallbackBaseURL) -``` - -- [ ] **Step 4: Run tests to verify pass** - -Run: `go test ./internal/ai_client -run BuildCallbackURL -v` -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add internal/ai_client/client.go internal/ai_client/client_test.go -git commit -m "fix(ai-client): use callback base URL for callback endpoint" -``` - -### Task 3: Update environment docs/examples - -**Files:** -- Modify: `.env.example` -- Modify: `README.md` - -- [ ] **Step 1: Add config key to `.env.example`** - -```dotenv -LISTEN_ADDR=:8080 -CALLBACK_BASE_URL=http://localhost -``` - -- [ ] **Step 2: Document purpose in README** - -```md -- `LISTEN_ADDR`: local bind address for HTTP server (e.g. `:8080`) -- `CALLBACK_BASE_URL`: public base URL used to build callback endpoint; defaults to `http://localhost` when unset -``` - -- [ ] **Step 3: Run full test suite** - -Run: `go test ./...` -Expected: PASS. - -- [ ] **Step 4: Commit** - -```bash -git add .env.example README.md -git commit -m "docs: document callback base URL configuration" -``` - -### Task 4: Prepare PR - -**Files:** -- No code changes expected - -- [ ] **Step 1: Push branch** - -Run: `git push -u origin ` -Expected: branch published. - -- [ ] **Step 2: Open PR with summary and test evidence** - -Include: -- Problem: callback URL incorrectly built from listen addr -- Fix: separate `CALLBACK_BASE_URL` with localhost fallback -- Tests: `go test ./internal/config -run CallbackBaseURL -v`, `go test ./internal/ai_client -run BuildCallbackURL -v`, `go test ./...` diff --git a/docs/superpowers/plans/2026-04-27-imap-proxy-support.md b/docs/superpowers/plans/2026-04-27-imap-proxy-support.md deleted file mode 100644 index 30879bf..0000000 --- a/docs/superpowers/plans/2026-04-27-imap-proxy-support.md +++ /dev/null @@ -1,224 +0,0 @@ -# IMAP Proxy Support 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:** Make IMAP connections route through `IMAP_PROXY_URL` when configured, while keeping current direct dial behavior when unset. - -**Architecture:** Introduce a small dial abstraction in IMAP watcher so connection setup is testable and can branch between direct TLS dial and SOCKS5 proxy dial. Keep authentication/fetch logic unchanged; only connection creation path changes. Validate proxy URL format early and fail clearly when invalid. - -**Tech Stack:** Go, `github.com/emersion/go-imap/v2/imapclient`, `golang.org/x/net/proxy`, `crypto/tls`. - ---- - -### Task 1: Add failing tests for dial-path behavior - -**Files:** -- Modify: `internal/mail/imap_test.go` (create) -- Modify: `internal/mail/imap.go` (for injectable dial function used by tests) - -- [ ] **Step 1: Write failing test for direct dial when proxy unset** - -```go -func TestConnectAndWatch_UsesDirectDial_WhenProxyUnset(t *testing.T) { - watcher := &IMAPWatcher{cfg: &config.Config{IMAPHost: "imap.example.com", IMAPPort: "993", IMAPUser: "u", IMAPPass: "p"}} - - var directCalled bool - watcher.dialIMAP = func(addr string, _ *imapclient.Options) (*imapclient.Client, error) { - directCalled = true - return nil, errors.New("stop") - } - - _ = watcher.connectAndWatch(context.Background()) - - if !directCalled { - t.Fatal("expected direct dial path") - } -} -``` - -- [ ] **Step 2: Write failing test for proxy dial when proxy set** - -```go -func TestConnectAndWatch_UsesProxyDial_WhenProxySet(t *testing.T) { - watcher := &IMAPWatcher{cfg: &config.Config{IMAPHost: "imap.example.com", IMAPPort: "993", IMAPUser: "u", IMAPPass: "p", IMAPProxyURL: "socks5://127.0.0.1:1080"}} - - var proxyCalled bool - watcher.dialIMAPViaProxy = func(addr, proxyURL string, _ *imapclient.Options) (*imapclient.Client, error) { - proxyCalled = true - return nil, errors.New("stop") - } - - _ = watcher.connectAndWatch(context.Background()) - - if !proxyCalled { - t.Fatal("expected proxy dial path") - } -} -``` - -- [ ] **Step 3: Run tests to verify RED** - -Run: `go test ./internal/mail -run Uses -v` -Expected: FAIL because `dialIMAP` / `dialIMAPViaProxy` hooks do not exist yet. - -- [ ] **Step 4: Commit test scaffold after it compiles with new hooks (still failing behavior checks if needed)** - -```bash -git add internal/mail/imap_test.go internal/mail/imap.go -git commit -m "test(mail): add dial path tests for IMAP proxy" -``` - -### Task 2: Implement proxy-aware dialing with minimal changes - -**Files:** -- Modify: `internal/mail/imap.go` - -- [ ] **Step 1: Add dial hooks to watcher for testability** - -```go -type IMAPWatcher struct { - cfg *config.Config - db *gorm.DB - client *imapclient.Client - - dialIMAP func(addr string, options *imapclient.Options) (*imapclient.Client, error) - dialIMAPViaProxy func(addr, proxyURL string, options *imapclient.Options) (*imapclient.Client, error) - - OnReceived func(task *database.Task) -} -``` - -- [ ] **Step 2: Initialize default dial hooks in constructor** - -```go -func NewIMAPWatcher(cfg *config.Config, db *gorm.DB) *IMAPWatcher { - return &IMAPWatcher{ - cfg: cfg, - db: db, - dialIMAP: imapclient.DialTLS, - dialIMAPViaProxy: dialTLSViaSOCKS5, - } -} -``` - -- [ ] **Step 3: Branch connection setup by `IMAPProxyURL`** - -```go -if strings.TrimSpace(w.cfg.IMAPProxyURL) == "" { - c, err = w.dialIMAP(addr, options) -} else { - c, err = w.dialIMAPViaProxy(addr, w.cfg.IMAPProxyURL, options) -} -if err != nil { - return fmt.Errorf("dial IMAP: %w", err) -} -``` - -- [ ] **Step 4: Implement SOCKS5 TLS dial helper** - -```go -func dialTLSViaSOCKS5(addr, proxyURL string, options *imapclient.Options) (*imapclient.Client, error) { - u, err := url.Parse(proxyURL) - if err != nil { - return nil, fmt.Errorf("parse proxy url: %w", err) - } - if u.Scheme != "socks5" { - return nil, fmt.Errorf("unsupported proxy scheme: %s", u.Scheme) - } - - var auth *proxy.Auth - if u.User != nil { - pw, _ := u.User.Password() - auth = &proxy.Auth{User: u.User.Username(), Password: pw} - } - - d, err := proxy.SOCKS5("tcp", u.Host, auth, proxy.Direct) - if err != nil { - return nil, fmt.Errorf("create socks5 dialer: %w", err) - } - - conn, err := d.Dial("tcp", addr) - if err != nil { - return nil, fmt.Errorf("proxy dial: %w", err) - } - - host, _, _ := net.SplitHostPort(addr) - tlsConn := tls.Client(conn, &tls.Config{ServerName: host}) - if err := tlsConn.Handshake(); err != nil { - _ = tlsConn.Close() - return nil, fmt.Errorf("tls handshake: %w", err) - } - - return imapclient.New(tlsConn, options), nil -} -``` - -- [ ] **Step 5: Run focused tests to verify GREEN** - -Run: `go test ./internal/mail -run Uses -v` -Expected: PASS. - -- [ ] **Step 6: Commit implementation** - -```bash -git add internal/mail/imap.go internal/mail/imap_test.go -git commit -m "feat(mail): support IMAP SOCKS5 proxy dialing" -``` - -### Task 3: Add URL validation coverage and docs consistency - -**Files:** -- Modify: `internal/mail/imap_test.go` -- Modify: `README.md` (only if wording needs tighter scheme requirement) - -- [ ] **Step 1: Write test for invalid proxy scheme** - -```go -func TestDialTLSViaSOCKS5_RejectsNonSocks5Scheme(t *testing.T) { - _, err := dialTLSViaSOCKS5("imap.example.com:993", "http://proxy:8080", nil) - if err == nil || !strings.Contains(err.Error(), "unsupported proxy scheme") { - t.Fatalf("expected unsupported scheme error, got %v", err) - } -} -``` - -- [ ] **Step 2: Run targeted test** - -Run: `go test ./internal/mail -run RejectsNonSocks5Scheme -v` -Expected: PASS. - -- [ ] **Step 3: Ensure README explicitly says socks5 scheme (already present, keep if unchanged)** - -```md -IMAP_PROXY_URL: socks5://user:pass@host:port -``` - -- [ ] **Step 4: Commit doc/test alignment** - -```bash -git add internal/mail/imap_test.go README.md -git commit -m "test/docs: validate IMAP proxy URL scheme" -``` - -### Task 4: Final verification - -**Files:** -- No new files expected - -- [ ] **Step 1: Run package tests for mail** - -Run: `go test ./internal/mail -v` -Expected: PASS. - -- [ ] **Step 2: Run full suite** - -Run: `go test ./...` -Expected: PASS. - -- [ ] **Step 3: Push and update issue/PR with evidence** - -```bash -git push -u origin -``` - -Include commands and pass results in issue/PR comment. diff --git a/internal/mail/resolve_thread_test.go b/internal/mail/resolve_thread_test.go index 9e52251..054c489 100644 --- a/internal/mail/resolve_thread_test.go +++ b/internal/mail/resolve_thread_test.go @@ -83,4 +83,4 @@ func TestResolveThreadID_LookupError_ReturnsMessageID(t *testing.T) { } } -var nilLookup = func(string) (*database.Task, error) { return nil, nil } \ No newline at end of file +var nilLookup = func(string) (*database.Task, error) { return nil, nil } -- 2.54.0 From 1eb9147ba17c1308e0a7122b6e60a23001217206 Mon Sep 17 00:00:00 2001 From: thuanle Date: Tue, 28 Apr 2026 05:35:03 +0700 Subject: [PATCH 8/8] fix: remove spec/plan artifacts from PR branch Co-Authored-By: Claude Opus 4.7 --- .../plans/2026-04-28-references-threading.md | 261 ------------------ .../2026-04-28-references-threading-design.md | 44 --- 2 files changed, 305 deletions(-) delete mode 100644 docs/superpowers/plans/2026-04-28-references-threading.md delete mode 100644 docs/superpowers/specs/2026-04-28-references-threading-design.md diff --git a/docs/superpowers/plans/2026-04-28-references-threading.md b/docs/superpowers/plans/2026-04-28-references-threading.md deleted file mode 100644 index d3823d6..0000000 --- a/docs/superpowers/plans/2026-04-28-references-threading.md +++ /dev/null @@ -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 == "" { - return &database.Task{ThreadID: "thread-abc"}, nil - } - return nil, nil - } - got := resolveThreadID("msg-2", "", "", 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 == "" { - return &database.Task{ThreadID: "thread-c"}, nil - } - return nil, nil - } - got := resolveThreadID("msg-3", "", " ", 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 == "" { - return &database.Task{ThreadID: "thread-b"}, nil - } - return nil, nil - } - got := resolveThreadID("msg-4", "", " ", 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", "", " ", 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 == "" { - return &database.Task{ThreadID: "thread-ref"}, nil - } - return nil, nil - } - got := resolveThreadID("msg-6", "", "", 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", "", "", 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 -``` - -PR body must include `Fixes #4`. diff --git a/docs/superpowers/specs/2026-04-28-references-threading-design.md b/docs/superpowers/specs/2026-04-28-references-threading-design.md deleted file mode 100644 index 139bebf..0000000 --- a/docs/superpowers/specs/2026-04-28-references-threading-design.md +++ /dev/null @@ -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 `` 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: ` ... `. 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) -- 2.54.0