fix(email): RFC 5322 compliance in message ingest (Message-ID whitespace + In-Reply-To multi-id + unknown-zone Date) - #1192
Conversation
`normalize_message_id` stripped surrounding whitespace and angle brackets but left interior whitespace intact. A Message-ID (RFC 5322 §3.6.4) carries no interior whitespace, but header unfolding (RFC 5322 §2.2.3) can leave interior spaces/tabs when a folded header is rejoined, e.g. `<abc@\r\n example.com>` unfolds to `<abc@ example.com>`. The old normalizer returned `abc@ example.com` for the folded form and `abc@example.com` for the clean form, so the same message arriving with a different fold boundary produced two distinct keys — `threading_service` (the sole thread-id owner) and `email_dedupe_service` would then split one message into two threads / fail to de-duplicate it. Collapse all interior whitespace after bracket stripping so folded and unfolded forms of the same Message-ID normalize equal. Valid Message-IDs contain no interior whitespace, so this is a no-op for well-formed input and a correctness fix for folded/malformed input; it also flows through `extract_reference_ids`, keeping References-header matching fold-insensitive. Tests: added `normalize_message_id` coverage (brackets/outer whitespace, empty/ None, the interior-unfolding-whitespace cases) and an `extract_reference_ids` case that de-dupes a reference split over a fold boundary. Verified the pure functions directly and lint-clean under the CI-pinned ruff 0.15.21 (the full pytest suite needs the app's asyncpg/crypto stack, unavailable in this sandbox; CI runs it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
📝 WalkthroughWalkthroughEmail parsing now handles RFC 2047 display names, RFC 5322 unknown timezones, content fallbacks, and byte-input errors. Threading now canonicalizes folded Message-IDs and supports multiple ChangesEmail ingest and threading
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant EmailMessage
participant email_parser
participant threading_service
participant ThreadStore
EmailMessage->>email_parser: parse headers and content
email_parser->>threading_service: normalized Message-ID references
threading_service->>ThreadStore: find existing candidate threads
ThreadStore-->>threading_service: matching thread IDs
threading_service-->>EmailMessage: assigned thread ID
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
`_extract_date` returned `parsedate_to_datetime`'s value directly. RFC 5322 §3.3 defines a "-0000" zone as "time zone unknown", for which `parsedate_to_datetime` returns a NAIVE datetime, whereas every other branch — a real offset, and the `datetime.now(timezone.utc)` fallback for missing/ malformed dates — yields a timezone-AWARE datetime. That inconsistency is a latent crash: sorting or comparing a `-0000` message's date against any other message's date raises `TypeError: can't compare offset-naive and offset-aware datetimes`, and a naive value misbinds the instant when persisted to a `timestamptz` column. `-0000` is common in real mail (list servers / MTAs that decline to assert a meaningful zone). Normalize the unknown zone to UTC so `_extract_date` always returns an aware datetime. Added `test_parse_eml_unknown_timezone_date_is_timezone_aware`; verified the full `tests/test_email_parser.py` (15 passed) and lint-clean under the CI-pinned ruff 0.15.21. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
seonghobae
changed the title
assign_thread_id() parsed In-Reply-To with normalize_message_id(), which
treats the whole header as a single opaque Message-ID: it strips only the
outer <> and collapses interior whitespace. RFC 5322 section 3.6.4 defines
In-Reply-To (like References) as 1*msg-id -- one or more angle-bracketed
ids, each optionally wrapped in CFWS -- so two real-world shapes produced a
garbage candidate that matched no existing thread and silently split the
reply onto its own thread:
"<a@x> <b@x>" -> "a@x><b@x" (multi-parent)
"<parent@x> (sent from ...)" -> "parent@x>(sentfrom...)" (trailing CFWS)
Parse In-Reply-To with extract_reference_ids() (already used for
References, and already fold/CFWS/dedupe-correct), add every extracted
parent id as a thread-lookup candidate, and use the first parent id as the
deterministic root fallback. A single clean id is unaffected --
extract_reference_ids("<parent@x>") == ["parent@x"] -- so all existing
threading behavior is preserved; only the previously-broken multi-id and
CFWS forms change.
Verification (backend/, --noconftest to skip the openai-dependent app import):
- tests/test_threading_service.py: 13 passed (10 existing + 3 new TDD:
multi-id match, multi-id root fallback, CFWS comment).
- tests/test_threading_perf.py: 2 passed (no threading regression).
- ruff check services/threading_service.py tests/test_threading_service.py: clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
seonghobae
changed the title
Follow-up coverage for the email-ingest fixes on this branch. Adds tests
for previously-unexercised paths (measured with coverage --branch on the
two modules, --noconftest to skip the openai-dependent app import):
threading_service.py 89% -> 93%
- generate_email_fingerprint: was entirely untested. Now covers
determinism, SHA-256 hex shape, lower-case + outer-whitespace
collapse, None-component tolerance, and per-field sensitivity.
- assign_thread_id uuid4 root: an email with no in_reply_to / references
/ message_id now asserts a fresh 32-hex uuid root with no DB lookup.
- extract_reference_ids: a bracketed but whitespace-only id ("< >") is
dropped rather than carried as an empty candidate.
- assign_thread_id later-candidate: the lookup loop skips an unmatched
first candidate (unimported immediate parent) and returns a matched
older reference's thread instead of the deterministic root.
email_parser.py 88% -> 90%
- _extract_thread_id: a whitespace-only References/In-Reply-To header
(splits to no token) now asserts fall-through to the next source
(In-Reply-To, then Message-ID) rather than a blank thread id
(branches 155->158 and 161->164).
Test-only; no production code changed. 33 passed; ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
Continues securing coverage on the RFC-5322 ingest path with additive, production-untouched tests for the remaining uncovered lines (measured with coverage --branch, --noconftest to skip the openai-dependent app): - _sanitize_address_display_text: a token with a display name but empty address part keeps the name (49-50); a header yielding no address at all falls back to sanitized raw text (53). - _attachment_part_content: a part whose get_content() cannot decode (unknown charset / bad transfer-encoding) falls back to the raw decoded payload, and to "" when the payload is absent (99-101). - parse_eml_bytes: parses provider bytes (happy path) and wraps an internal parse failure as the sanitized public EmailParseError rather than leaking the exception chain at the ingest boundary (217-222). email_parser.py 90% -> 98% (0 statements missed; the 3 remaining branch partials are defensive isinstance(str) guards for non-str part content). Test-only; no production change. 19 passed; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
Closes the last gaps in the module the In-Reply-To / Message-ID fixes on
this branch touched (measured with coverage --branch, --noconftest):
- extract_reference_ids: a References value with no angle brackets falls
back to a whitespace split (line 65).
- _find_existing_thread_ids: returns {} for no candidates without issuing a
query (86); dedupes a bare id against its already-bracketed form so the
shared lookup key is enqueued once (92->91); skips a stored row with a
null thread_id (106) and a row whose message_id normalizes to nothing
(108->104), so neither pollutes the returned map.
services/threading_service.py 93% -> 100% (0 missed, 0 branch partials);
services/email_parser.py stands at 98% after the prior commit. Test-only;
no production change. 21 passed; ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
Ground the #1192 email-ingest correctness changes (Message-ID interior-whitespace normalization, unknown-zone -0000 date normalization, In-Reply-To 1*msg-id multi-parent + CFWS parsing) in the message-format standard, anchoring each fix to a specific RFC 5322 clause (§2.2.3 header unfolding, §3.6.4 msg-id, §3.2.2 CFWS, §3.3 unknown zone). Also record the precision-first threading design rationale with verified citations (Kooti et al., 2015; Mohiuddin, Joty, & Nguyen, 2018; Zhang et al., 2021), following the existing docs/research/<topic>/ convention. RFC text and PDFs are referenced by canonical URL and bookmarked in the alphaXiv library; they are not committed because the sandbox proxy blocks rfc-editor.org and arxiv.org (disclosed in the pack's Preservation notes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
…coded _sanitize_address_display_text formatted each parsed address with email.utils.formataddr. The From/To/Reply-To headers arrive already header-decoded (policy.default decodes RFC 2047 encoded-words), but formataddr re-encodes any non-ASCII display name back into an =?utf-8?...?= encoded-word, so every non-ASCII sender/recipient/reply-to name (e.g. Korean) was stored and displayed as garbled machine bytes rather than readable text. The subject path was unaffected because it does not round-trip through formataddr. Replace formataddr with _format_display_address, which mirrors formataddr's RFC 5322 quoting/escaping for display-name specials (so ", "-joined multi-address values stay unambiguous and embedded quotes are escaped) but keeps the decoded name literal -- these values are stored for human display, not re-emitted as message headers. Tests: end-to-end parse of RFC 2047-encoded Korean From/To/Reply-To/Subject storing decoded text; unit coverage of the helper's quoting, escaping, empty name, and non-ASCII literal paths. email_parser branch coverage stays at 98% (new helper fully covered); ruff clean; 22 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
…s pack Extend the email-ingest standards-basis pack to cover the RFC 2047 encoded-word decoding fix now on #1192: display names in From/To/Reply-To arrive header-decoded under email.policy.default and must be stored decoded, not re-encoded by email.utils.formataddr (which garbled every non-ASCII sender into =?utf-8?...?=). Adds the RFC 2047 clause to the standards basis, an APA-7 reference (Moore, 1996), and updates the work-item/verification notes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
… -> 100%) Add unit tests for the isinstance(part_content, str) guards in _process_multipart_body (text/plain and text/html parts) and _process_singlepart_body, exercising the case where part.get_content() returns a non-str (e.g. undecodable bytes) so the body-extraction path drops it instead of concatenating bytes. Brings email_parser.py to 100% branch coverage (the org 100% standard), completing the RFC 2047 display-name change's coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/tests/test_email_parser.py`:
- Around line 1-19: Import EmailParseError from services.email_parser in the
test module’s existing import block so
test_parse_eml_bytes_parses_provider_bytes_and_wraps_parse_errors can reference
it in pytest.raises without a NameError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e25ed36-573f-4c81-b5db-54addd189d7a
📒 Files selected for processing (5)
backend/services/email_parser.pybackend/services/threading_service.pybackend/tests/test_email_parser.pybackend/tests/test_threading_service.pydocs/research/email-ingest-threading/README.md
|
PR governance metadata gate update for PR governance metadata gate is ready; all current-head requirements passed. |
Consolidate the test's EmailParseError import into the services.email_parser import block (email_parser re-exports it from services.exceptions), removing the separate services.exceptions import. No behavior change — EmailParseError was already imported and the suite passed; this addresses a review note preferring the module-under-test as the single import source. Ruff clean; 24 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
|
Closing after Loop drain: permanently blocked — branch was updated onto develop for mergeability, which cleared prior APPROVED robot evidence; re-review (CodeRabbit/OpenCode) and/or central gate jobs (metadata-only gate evaluation, coverage-evidence) remained pending/stuck without a re-runnable workflow handle. Not force-merging (merge-gate policy). Re-open a focused PR when robot capacity is available. Related product security fixes that reimplemented cleanly remain on branch goal/carddav-path-traversal-decode (#1206) for relaunch. |
Pull request was closed
|
@coderabbitai review |
✅ Action performedReview finished.
|
The sole requested change was addressed in commit 5db7cac and the associated review thread is resolved; dismissing the stale blocking review after current-head CI revalidation.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head maintenance revalidation completed for RFC 5322 message-id, reply-reference, and unknown-zone date handling. Existing review findings are resolved and the required action suite is green; refresh the central review evidence for this exact head.
|
@coderabbitai review Please re-review the current head |
|
✅ Action performedReview finished.
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current head fadfdba52c039ab70162dd211486a49c119f588b has all repository-local required workflows successful and no unresolved current review thread. Re-triggering central current-head OpenCode review and merge scheduling.
Pull request was converted to draft
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head commercialization loop revalidation: fadfdba52c039ab70162dd211486a49c119f588b remains mergeable, all repository-local required workflows are successful, and the only inline review thread is resolved. Re-triggering the central OpenCode/Noema review and guarded merge scheduler for this exact head.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Branch refreshed onto live develop at merge base a67a2b39b76e186fb469add378924913704121c8. Current head 8792334e9b85e901d72f7115bf2a439db61efbbb is zero commits behind and differs by only the five RFC 5322 ingest/threading files. Revalidating current-head checks and independent review.
|
@coderabbitai review Please perform a complete review of the current head |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 4 minutes. |
|
@coderabbitai review The included-review cooldown has elapsed. Please now perform the complete current-head review requested for |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 1 second. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/services/threading_service.py (1)
134-140: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the ordered-dedup pattern into a helper.
The "iterate, skip if in
seen, else add" pattern appears here and inextract_reference_ids(and, unchanged, in_find_existing_thread_ids). A small_dedupe_ordered(iterable)helper would remove this duplication.♻️ Proposed helper
+def _dedupe_ordered(values: Iterable[str]) -> list[str]: + seen: set[str] = set() + result: list[str] = [] + for value in values: + if value not in seen: + seen.add(value) + result.append(value) + return result🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/threading_service.py` around lines 134 - 140, Extract the repeated ordered-deduplication logic into a shared `_dedupe_ordered(iterable)` helper, preserving first-seen order. Update the candidate construction here, `extract_reference_ids`, and `_find_existing_thread_ids` to use the helper instead of maintaining separate `seen` loops.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/services/threading_service.py`:
- Around line 134-140: Extract the repeated ordered-deduplication logic into a
shared `_dedupe_ordered(iterable)` helper, preserving first-seen order. Update
the candidate construction here, `extract_reference_ids`, and
`_find_existing_thread_ids` to use the helper instead of maintaining separate
`seen` loops.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eebea8c0-d085-45d4-a048-77f5a5fe738e
📒 Files selected for processing (5)
backend/services/email_parser.pybackend/services/threading_service.pybackend/tests/test_email_parser.pybackend/tests/test_threading_service.pydocs/research/email-ingest-threading/README.md
OpenCode Review Overview
--> Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend (4 files)"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend (4 files)"]
R1 --> V1["backend tests"]
Evidence --> S2["Docs: README.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: README.md"]
R2 --> V2["docs review"]
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: APPROVE - changed-file evidence inspected: backend/services/email_parser.py, backend/services/threading_service.py, backend/tests/test_email_parser.py, backend/tests/test_threading_service.py, docs/research/email-ingest-threading/README.md; every hunk is fully implemented and matches the PR intent. Verification posture: Coverage execution evidence reports Result PASS (supported repository test suites passed) and no completed failed GitHub Checks for head 681be66; the repository regression contract is cd backend && python3 -m pytest tests. Linter/static: no lint failures in evidence; backend pyproject contract (python >=3.12) is present. TDD/regression: new tests cover decoded non-ASCII display names (RFC 2047 Korean case), RFC 5322 display-name quoting specials via _format_display_address, folded Message-ID whitespace collapse, In-Reply-To 1msg-id multi-id extraction, and naive -0000 Date -> UTC (commit history 9de4762, bb8880c, 266cd0e, 74d1388, feddde0). Coverage: PASS per Coverage execution evidence; threading_service and email_parser reached 100% coverage per commit history. Docstring coverage: configured repository docstring gates passed or advisory per Coverage execution evidence. DAG: CodeGraph-backed base-to-head changed flow:
flowchart LR
A["parse_eml / parse_eml_bytes"] --> B["_format_display_address (decoded names)"]
A --> C["_extract_date (naive -> UTC)"]
A --> D["EmailData in_reply_to / references / message_id"]
D --> E["assign_thread_id"]
E --> F["normalize_message_id / extract_reference_ids"]
E --> G["_find_existing_thread_ids DB lookup"]
E --> H["thread_id fallback root"]
H --> I["Email.thread_id"]
PoC/execution: no OPENCODE_EXECUTION_RECEIPT tool lines were present; adversarial probes rely on trusted source traces plus the Coverage PASS evidence. DDD/domain: normalize_message_id and assign_thread_id remain the sole owners of thread-id derivation; EmailData/Email consumers (imap_worker, email_import_service, frontend EmailDetail.tsx) keep the same data shapes. CDD/context: RFC 5322 section 2.2.3 (unfolding), 3.3 (unknown zone), 3.6.4 (msg-id; References/In-Reply-To as 1msg-id), 3.4 display-name quoting, and RFC 2047 encoded-words are grounded in code docstrings and the standards-basis docs pack. Similar issues: the earlier CodeRabbit nitpick thread on threading_service.py is resolved per current-head unresolved-thread evidence (none present). Claim/concept check: PR title/body claims verified in hunks - interior-whitespace collapse, In-Reply-To multi-id parsing, unknown-zone Date -> UTC, and decoded display-name storage are all present and tested. Standards search: RFC 5322/RFC 2047 citations are backed by the email-ingest-threading standards-basis README and code docstrings. Compatibility/convention: no renamed or new DB objects, API fields, or routes; new private helpers (_format_display_address, _ADDRESS_SPECIALS_RE, _ADDRESS_QUOTED_ESCAPE_RE) follow repository snake_case conventions; ASCII display-name output stays byte-identical to the replaced formataddr behavior. Breaking-change/backcompat: stored ASCII sender/recipient/reply_to values are unchanged; only previously corrupted encoded-word or whitespace-id values change; thread_id canonical form changes only for interior-whitespace (folded) Message-IDs, which is the intended fix. Implementation completeness: all changed functions are fully implemented with no placeholders or TODO-only branches. Performance: O(n) regex/join work; candidate dedup keeps thread lookup linear. Developer experience: focused unit tests plus the standards doc ease future parser maintenance. User experience: non-ASCII sender/recipient names now display decoded instead of =?utf-8?b?...?= garbling, and unknown-zone dates sort consistently. Visual/DOM: non-web change - the reviewed interaction surface is the email ingest/threading pipeline (CLI/API/log/docs), with no UI code touched. Accessibility/i18n: i18n-positive change verified by the RFC 2047 decoded Korean display-name test (박성호/김천/응답). Supply-chain/license: no dependency or lockfile changes. Packaging: backend pyproject test contract exists; the README addition is docs-only. Security/privacy: no auth/tenant/identifier changes; strip_html_markup and NUL sanitization are retained; display-name quoting prevents stored-string ambiguity; thread roots are SHA-256 hashes or uuid4, so no sequential or guessable identifiers are introduced.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including backend/services/email_parser.py, backend/services/threading_service.py, backend/tests/test_email_parser.py, backend/tests/test_threading_service.py, docs/research/email-ingest-threading/README.md.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects backend/services/email_parser.py to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":"backend/services/threading_service.py","line":34,"hypothesis":"normalize_message_id's new interior-whitespace collapse corrupts a valid Message-ID or merges two distinct ids","attack_or_counterexample":"Folded header value '<abc@\\r\\n example.com>' (unfolds to '<abc@ example.com>') compared against the unfolded '<abc@example.com>' as Message-ID/References inputs","evidence":"Trusted source trace at backend/services/threading_service.py:34 (normalized = \"\".join(stripped.split())) maps '<abc@ example.com>' to 'abc@example.com', exactly equal to the unfolded form's canonical id, so the fold/unfold pair now collides on one id instead of splitting a thread; RFC 5322 section 3.6.4 msg-id permits no interior whitespace, so no valid id can be corrupted by the collapse; the collapse tests (commits 9de47620, feddde03) and Coverage execution evidence Result PASS (supported repository test suites passed) corroborate. source-line-sha256=3cd11b61e46589e0ce69cabbd38e02034e88dad1ce6e4d6c714b46aa2cdab8f5","outcome":"falsified"},{"path":"backend/services/threading_service.py","line":160,"hypothesis":"Parsing In-Reply-To with the multi-id extractor regresses the single-parent case or reorders candidates so an existing thread is missed","attack_or_counterexample":"In-Reply-To: <a@b> combined with References: <a@b> <c@d> (a reply naming several parents)","evidence":"Trusted source trace at backend/services/threading_service.py:160 (for candidate in (*in_reply_to_ids, *references) guarded by the seen set) dedupes to ['a@b','c@d'] with In-Reply-To ids first - identical candidate ordering to the base single-id path - and the fallback still prefers references[0] then in_reply_to_ids[0], so existing threads are matched in the same order while the multi-parent case now converges on an ancestor it previously corrupted; threading tests (commits bb8880c7, db600e39) plus Coverage execution evidence PASS corroborate. source-line-sha256=60c6044925fd105c4cb666a32d0a5e1d8d6907056ff0c95875baa95b2b097cef","outcome":"falsified"},{"path":"backend/services/email_parser.py","line":173,"hypothesis":"Attaching UTC to a parsed Date that is naive misbinds the instant or raises on an already-aware datetime","attack_or_counterexample":"Date: Mon, 27 Apr 2026 10:00:00 -0000 (unknown zone, parsed naive by parsedate_to_datetime) plus an already-aware +0900 date","evidence":"Trusted source trace at backend/services/email_parser.py:173 (parsed_date = parsed_date.replace(tzinfo=datetime.timezone.utc)) executes only under the elif parsed_date.tzinfo is None guard, so -0000 or zone-less dates become aware UTC instants and already-aware datetimes pass through untouched; RFC 5322 section 3.3 interprets -0000 as unknown-zone, and the code comment plus standards-basis README document UTC as the deterministic interpretation; the parser tests (commit 266cd0ec) and Coverage execution evidence Result PASS corroborate. source-line-sha256=1f5102b4668b929ce72919ddaf6b86bfc597ccc6b04e98c4263c6763d647cae3","outcome":"falsified"}],"residual_risk":"Residual risk is bounded: (1) REFERENCE_PATTERN's exact regex definition fell outside the inlined evidence excerpt, so extraction of ids embedded inside CFWS comments is not independently re-verified here; (2) the thread_id canonical form changes for previously stored Message-IDs containing interior whitespace, which can split pre-existing threads created before this deploy for folded-header messages only; (3) _format_display_address no longer raises on newline-containing display names as formataddr did, storing such values for display only - a deliberate tolerance change."}- Result: APPROVE
- Reason: No material defects found: the three RFC 5322 ingest fixes (Message-ID interior-whitespace collapse, In-Reply-To 1*msg-id multi-id parsing, unknown-zone Date normalization, decoded display-name storage) are correctly implemented, RFC-grounded, test-covered, and documented; Coverage execution evidence is PASS, there are no failed GitHub Checks, no unresolved review threads, and mergeability is clean.
- Head SHA:
681be66cb3344ee073b822ec2d71aa43a5552048 - Workflow run: 30841749836
- Workflow attempt: 1
2 participants
Description
Three focused RFC 5322 correctness fixes in the email-ingest/threading path, each with tests.
1.
services/threading_service.py—normalize_message_idcollapses interior whitespacenormalize_message_id(the sole thread-id owner, also used byemail_dedupe_service) stripped surrounding whitespace and brackets but left interior whitespace intact. A Message-ID carries no interior whitespace (RFC 5322 §3.6.4), but header unfolding (§2.2.3) can leave interior spaces/tabs when a folded header is rejoined —<abc@\r\n example.com>→<abc@ example.com>. The old code returnedabc@ example.comfor the folded form andabc@example.comfor the clean form, so the same message with a different fold boundary produced two keys → threading/de-dup split one message into two. Fix: collapse interior whitespace after bracket stripping (no-op for well-formed IDs).2.
services/threading_service.py—assign_thread_idparses In-Reply-To as1*msg-id(multi-id + CFWS)assign_thread_idparsed In-Reply-To withnormalize_message_id, treating the whole header as one opaque Message-ID (strip outer<>, collapse whitespace). But RFC 5322 §3.6.4 defines In-Reply-To — like References — as1*msg-id: one or more angle-bracketed ids, each optionally wrapped in CFWS. Two real-world shapes therefore produced a garbage candidate that matched no existing thread and silently split the reply onto its own thread:Fix: parse In-Reply-To with
extract_reference_ids()(already used for References, and already fold/CFWS/dedupe-correct), add every extracted parent id as a thread-lookup candidate, and use the first parent id as the deterministic root fallback. A single clean id is unaffected —extract_reference_ids("<parent@x>") == ["parent@x"]— so all existing threading behavior is preserved; only the previously-broken multi-id and CFWS forms change. (Subject-based fallback is deliberately not added —test_forwarded_subject_alone_does_not_merge_unrelated_threadpins that Naruon must not merge unrelated messages by subject.)3.
services/email_parser.py—_extract_datenormalizes unknown-zone (-0000) to timezone-awareRFC 5322 §3.3 defines a
-0000zone as "time zone unknown", for whichparsedate_to_datetimereturns a naive datetime — while every other branch (a real offset, or thenow(utc)fallback) returns an aware datetime. That inconsistency is a latent crash: comparing/sorting a-0000message's date against another message's date raisesTypeError: can't compare offset-naive and offset-aware datetimes, and a naive value misbinds the instant in atimestamptzcolumn.-0000is common in real mail. Fix: treat the unknown zone as UTC so_extract_datealways returns an aware datetime.Fixes # (no linked issue — found during a standards-compliance audit of the ingest/threading path)
Type of change
Checklist:
tests/test_threading_service.py13 passed (10 existing + 3 new),tests/test_email_parser.py15 passed,tests/test_threading_perf.py2 passed (no threading regression); all changed files lint-clean under the CI-pinnedruff. (test_threading_pipeline.pyneedsaioimaplib, absent from the review sandbox; unrelated to this change. CI runs the whole suite — see the greencoverage-evidencecheck.)Tests added
tests/test_threading_service.py:test_normalize_message_id_collapses_interior_unfolding_whitespace,test_extract_reference_ids_normalizes_folded_whitespace_and_dedupes,test_multi_id_in_reply_to_threads_on_any_existing_parent,test_multi_id_in_reply_to_fallback_uses_first_parent_as_root,test_in_reply_to_with_cfws_comment_extracts_bare_msg_idtests/test_email_parser.py:test_parse_eml_unknown_timezone_date_is_timezone_awareVerification
🤖 Generated with Claude Code
https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
Summary by CodeRabbit
Bug Fixes
Documentation
Tests