HT-13: inbound threading decision (5-rule algorithm) - #9
Merged
Conversation
Wires the parser (HT-11) + reply tokens (HT-12) into the inbound
threading decision, specs/mail/threading.md §3. Pure decideThreading(
ParsedEmail, Keyring) -> append {conversationId, threadId} | new. Scans
In-Reply-To then References most-recent-first; first valid token wins;
shaped-but-invalid tokens are forged (counted, ignored); no valid token
-> new conversation, never by subject. Adds isReplyTokenShaped() to
reply-token.ts to tell forged-ours from not-ours.
Codex (independent GPT-lineage) adversarial review caught a HIGH bug the
Claude reviewers missed: In-Reply-To was compared whole, but RFC 5322
allows CFWS/comments and multiple ids in it — so 'In-Reply-To: (note)
<valid-token@d>' would mis-thread a real reply to a NEW conversation.
Fixed: both In-Reply-To and References are tokenized (angle-bracketed
ids extracted) before scanning. +5 tests for CFWS/multi-id/whitespace.
thread.ts 100% coverage; typecheck/lint/tests green. Second time Codex's
independent lineage caught a real threading bug — the standing rule earns it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesReply-token threading
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ParsedEmail
participant decideThreading
participant extractMessageIds
participant verifyReplyMessageId
participant Keyring
ParsedEmail->>decideThreading: In-Reply-To and References
decideThreading->>extractMessageIds: Extract ordered message IDs
extractMessageIds-->>decideThreading: Candidate IDs
decideThreading->>verifyReplyMessageId: Verify candidates
verifyReplyMessageId->>Keyring: Resolve signing key
Keyring-->>verifyReplyMessageId: Key material
verifyReplyMessageId-->>decideThreading: Valid token or failure
decideThreading-->>ParsedEmail: Append or new decision
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/mail/thread.test.ts`:
- Around line 54-56: Update the comment associated with the reply-with-reference
fixture to refer to the human support staff as an “Agent” rather than an
“agent,” using the canonical role terminology consistently.
In `@src/mail/thread.ts`:
- Around line 111-114: The extractMessageIds function incorrectly treats
angle-bracketed text in comments or quoted content as message IDs. Replace its
regex scan with RFC 5322-aware tokenization that skips comments, quoted strings,
and quoted pairs, returning only actual msg-id productions; add a regression
fixture for an In-Reply-To value such as (<token>) and preserve existing
valid-ID behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f3faeef-cfa7-4cb0-9959-5e9e7a9ce85b
📒 Files selected for processing (4)
src/mail/reply-token.test.tssrc/mail/reply-token.tssrc/mail/thread.test.tssrc/mail/thread.ts
Comment on lines
+54
to
+56
| // Evidence: fixtures/mail/observed/reply-with-reference.json — a customer | ||
| // reply whose In-Reply-To points at the agent reply's Message-ID appended | ||
| // to the same conversation (threadsCount 3→4, appendedToSameConversation). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the canonical Agent role name.
“agent reply” refers to human support staff; use Agent consistently.
Proposed wording
- // reply whose In-Reply-To points at the agent reply's Message-ID appended
+ // reply whose In-Reply-To points at an Agent reply's Message-ID appendedAs per coding guidelines, human support staff are Agents, and AI actors are Assistants; never conflate them in schema, code, documentation, or prose.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Evidence: fixtures/mail/observed/reply-with-reference.json — a customer | |
| // reply whose In-Reply-To points at the agent reply's Message-ID appended | |
| // to the same conversation (threadsCount 3→4, appendedToSameConversation). | |
| // Evidence: fixtures/mail/observed/reply-with-reference.json — a customer | |
| // reply whose In-Reply-To points at the Agent reply's Message-ID appended | |
| // to the same conversation (threadsCount 3→4, appendedToSameConversation). |
🤖 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 `@src/mail/thread.test.ts` around lines 54 - 56, Update the comment associated
with the reply-with-reference fixture to refer to the human support staff as an
“Agent” rather than an “agent,” using the canonical role terminology
consistently.
Source: Coding guidelines
CodeRabbit flagged that the /<[^>]+>/g bracket-scan (added for the Codex In-Reply-To fix, and pre-existing in parse.ts's References handling) also grabs a <...> sitting INSIDE a comment or quoted string — which RFC 5322 says to ignore. Worst case: 'In-Reply-To: (<token-for-B>) <token-for-A>' would thread to the WRONG conversation. Fix: a shared src/mail/message-id.ts extractMessageIds() that walks the header skipping comments (nested, with \ quoted-pairs) and quoted strings, returning only top-level msg-ids. Wired into BOTH thread.ts (In-Reply-To + References) and parse.ts (References — same latent bug, now fixed). 12 extractor tests (comments, nested, quoted, escapes, unterminated) + a thread.ts regression for the token-in-comment wrong-conversation case. This honors the charter's 'boringly faithful on mail semantics, RFCs as primary source'. 113 tests total; typecheck/lint green; message-id.ts ~100%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
Codex re-review of message-id.ts: a > inside the msg-id's own quoted
id-left ("...") or domain-literal ([...]) was terminating the id early
(indexOf('>')), so <"a>b"@x> and <l@[a>b]> mis-parsed. Doesn't affect
OUR tokens (no quoted parts) but contradicted the module's RFC-aware
contract. The <...> scan now tracks quote/bracket substructure + \
quoted-pairs and only accepts a > outside them. +3 tests.
typecheck/lint green; full suite passes; message-id.ts ~100%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
This was referenced Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The piece that unites the engine: an inbound email is parsed (HT-11), its threading headers checked for a valid reply token (HT-12), and
decideThreadingreturns append to a conversation or start a new one — per specs/mail/threading.md §3.Pure, no I/O. The five rules, each tested against the HT-7 fixture that motivated it:
Codex (independent GPT lineage) caught a HIGH bug the Claude reviewers missed:
In-Reply-Towas matched whole, but RFC 5322 allows comments/multiple ids in it — so a decorated header would orphan a real reply into a new conversation. Fixed by tokenizing both headers; +5 CFWS/multi-id tests. thread.ts at 100% coverage, 100 tests total.Third core threading piece — parse + tokens + decision now route an inbound reply to its conversation end-to-end.
Jira: https://resonantiq.atlassian.net/browse/HT-13
🤖 Generated with Claude Code
Summary by CodeRabbit
In-Reply-ToandReferences, including handling comments/quoted strings and multiple ids.Message-IDmatches the expected reply-token shape.