-
Notifications
You must be signed in to change notification settings - Fork 0
HT-8: mail threading behavioral spec #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # Mail Threading Spec | ||
|
|
||
| Status: draft. Governs how inbound email is assigned to a conversation/thread lineage. Sacred invariant #3 (charter §6): threading correctness outranks feature velocity. | ||
|
|
||
| ## 1. Purpose & principle | ||
|
|
||
| Threading authority lives on the outbound side (charter §2). `In-Reply-To` and `References` (RFC 5322 §3.6.4) are written by every mail client on earth, inconsistently — they are not something the engine controls and cannot be trusted as evidence on their own. The one header the engine fully controls is the Message-ID it mints on its own outbound mail. So the engine embeds a signed token in every outbound Message-ID, and threading a reply back in means finding and verifying that token — never interpreting subject text, never blindly trusting whatever a client echoed back. If no token verifies, the message is a new conversation, full stop. This is "boringly faithful" in the charter §2 sense: reproducing the proven shape of a mechanism ("signed reply tokens in outbound Message-IDs... a pattern as old as mailing-list software," charter §2) without reverse-engineering or trusting inbound header content we didn't produce. | ||
|
|
||
| ## 2. The reply token | ||
|
|
||
| Every outbound message (agent reply, auto-response, and any future first-party auto-reply) embeds a signed token in its `Message-ID`. Proposed format: | ||
|
|
||
| ``` | ||
| <ht.{conversationId}.{threadId}.{sig}@{mailDomain}> | ||
| ``` | ||
|
|
||
| where `sig = HMAC(secret, canonical(conversationId, threadId))`, truncated and hex/base32-encoded. This is Helpthread's own design, not derived from any observed system's internals — we only observed a black-box Message-ID *shape*, never a secret or algorithm. | ||
|
Comment on lines
+11
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== specs/mail/threading.md (1-120) ==\n'
cat -n specs/mail/threading.md | sed -n '1,120p'
printf '\n== Search threadId / Message-ID references ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'threadId|Message-ID|Message-ID uniqueness|Message-ID shape|signed token' .Repository: Helpthread/helpthread Length of output: 15410 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== Search for thread identity definitions ==\n'
rg -n --hidden --glob '!**/.git/**' -e '\bthreadId\b|\bthread\b.*identity|\blineage\b|\bMessage-ID\b' specs CHARTER.md fixtures .
printf '\n== Likely related spec files ==\n'
fd -a -e md specsRepository: Helpthread/helpthread Length of output: 8910 Make outbound Message-ID uniqueness explicit 🧰 Tools🪛 markdownlint-cli2 (0.22.1)[warning] 13-13: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🤖 Prompt for AI Agents |
||
|
|
||
| The properties that ARE the spec, independent of encoding: | ||
|
|
||
| - **(a) Unguessable without the secret** — not forgeable by an attacker who has seen valid tokens (cf. §3 rule 3). | ||
| - **(b) Verifiable offline** — no DB round-trip to detect tampering; pure computation against the signing secret(s). | ||
| - **(c) Carries the conversation+thread identity** — a verified token deterministically identifies its conversation/thread; no lookup table of issued tokens required. | ||
| - **(d) Rotation-tolerant** — the signing secret must be rotatable without invalidating outstanding tokens, implying a `keyId` alongside the signature. **OPEN QUESTION:** does `keyId` ship in v1, or wait for the first rotation? | ||
|
Comment on lines
+17
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== threading.md (around lines 1-80) =="
nl -ba specs/mail/threading.md | sed -n '1,120p'
echo
echo "== Search for crypto/key-rotation mentions in specs/mail =="
rg -n --no-heading -S "HMAC|canonical|keyId|rotation|truncate|base32|hex|Message-ID|forg" specs/mail || trueRepository: Helpthread/helpthread Length of output: 238 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== threading.md (first 140 lines, numbered with awk) =="
awk 'NR>=1 && NR<=140 { printf "%4d %s\n", NR, $0 }' specs/mail/threading.md
echo
echo "== Nearby mail specs/files =="
fd -a . specs/mail || true
echo
echo "== Search for crypto/key-rotation mentions in specs/mail =="
rg -n --no-heading -S "HMAC|canonical|keyId|rotation|truncate|base32|hex|Message-ID|forg" specs/mail || trueRepository: Helpthread/helpthread Length of output: 16739 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Search repo for cryptographic threading/token requirements =="
rg -n --no-heading -S "keyId|HMAC|canonical\\(|truncat|hex/base32|constant-time|compare|Message-ID|thread token|reply token|rotation" . || trueRepository: Helpthread/helpthread Length of output: 4087 Specify the reply-token contract. The spec names 🤖 Prompt for AI Agents |
||
|
|
||
| **Contrast with the observed reference format.** The fixtures show a reference helpdesk emitting Message-IDs shaped like `<FS_reply-{threadId}-{token}@{domain}>` — e.g. `<FS_reply-36-{token}@helpdesk.example.test>` (reply-with-reference.json, `agentReplyEmail.messageId`; the token value in the committed fixtures is a redacted placeholder — the real capability token is never published). Notably `{threadId}` there is a *thread* id (36), not the conversation id (15) — conversation is resolved via the thread's parent, not encoded directly. This is cited only as evidence the "signed token in the outbound Message-ID" pattern works in production (charter §2); Helpthread's `sig` derivation, secret, and truncation are unrelated to whatever that system does internally, which was never observed. | ||
|
|
||
| ## 3. Inbound threading decision — the algorithm | ||
|
|
||
| Ordered, testable procedure applied to every inbound message: | ||
|
|
||
| 1. **Extract candidate tokens from `In-Reply-To`, then each `References` entry, most-recent-first.** `In-Reply-To` names the specific message being replied to; `References` accumulates over a conversation's life, so scanning newest-first reaches what the customer is most immediately replying to before older entries. Evidence: reply-with-reference.json's `agentReplyEmail.references` field shows the list growing across replies (`<...@example.test> <FS_reply-31-9cbef04c5307d744@helpdesk.example.test>`) alongside a distinct `In-Reply-To` — proving both headers carry independent candidates and References accumulates over time. | ||
| 2. **For each candidate matching our Message-ID pattern, verify the signature; the first VALID token wins → append to that conversation/thread lineage.** Evidence: reply-with-reference.json — a reply's `In-Reply-To`/`References` pointed at the helpdesk's prior reply Message-ID and the conversation's thread count grew 3→4 with `appendedToSameConversation: true`. | ||
| 3. **If a candidate matches our pattern but FAILS signature verification, it is forged/corrupted: do not thread on it.** Treat it as absent and continue scanning remaining candidates; only fall through to rule 4 once all candidates are exhausted. Record a security-relevant event (forged-token observed; see §5). Evidence: forged-reply-token.json — a tampered token (altered from a genuine one captured in the base run) did not append to the real conversation (`appendedToRealConversation: false`; conversation 15's thread count held at 4 before and after); a new conversation (id 20) was created instead. The forged value differed from the genuine one by only a few characters yet was rejected, which shows the reference system checks token *integrity* rather than merely pattern-matching the `FS_reply-…` shape. (Black-box observation can't tell us *how* — HMAC signature, opaque store lookup, or otherwise — and this spec doesn't claim to know; Helpthread's own design, §2, uses an HMAC signature to get the same rejection guarantee.) | ||
| 4. **If no valid token is found in any header, this is a NEW conversation — regardless of subject.** Subject is NEVER used to thread. Evidence: reply-subject-only.json — a `Re:`-prefixed reply with matching subject and no reference headers produced a *separate* conversation (`outcome: "split"`, new id 17 distinct from original 16). Evidence: same-subject-different-customer.json — a second message with the exact same subject as an existing conversation and no reference headers produced its own conversation (`outcome: "own-conversation"`, id 19, distinct from 14); its correction note shows this held even though the sender turned out to be the *same* customer (Gmail flattens plus-addressed From to the canonical address) — neither subject nor sender identity merged them. | ||
|
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files 'specs/mail/threading.md' 'specs/mail/*' | sed -n '1,120p'
printf '\n== threading.md outline ==\n'
ast-grep outline specs/mail/threading.md --view expanded || true
printf '\n== threading.md targeted lines ==\n'
nl -ba specs/mail/threading.md | sed -n '1,140p'
printf '\n== search for related terms ==\n'
rg -n -i 'orphan|closed|deleted|reopen|unresolvable|invalid token|forged|signature|thread|conversation|Message-ID|References|In-Reply-To' specs/mailRepository: Helpthread/helpthread Length of output: 342 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== threading.md 1-120 ==\n'
sed -n '1,120p' specs/mail/threading.md | cat -n
printf '\n== targeted search ==\n'
rg -n -i 'orphan|closed|deleted|reopen|unresolvable|invalid token|forged|signature|append|thread|conversation|Message-ID|References|In-Reply-To' specs/mailRepository: Helpthread/helpthread Length of output: 22297 Define the closed/deleted-token path. §3 says the first valid token wins, but §5 still leaves closed/archived and deleted conversations open. Spell out whether those tokens reopen the target or start a new conversation, and whether deleted targets emit an orphaned-token event; add fixtures for both cases so identical headers can't diverge across implementations. 🧰 Tools🪛 LanguageTool[style] ~35-~35: ‘exact same’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_EXACT_SAME) 🤖 Prompt for AI Agents |
||
| 5. **A valid token threads even when the subject is completely unrelated.** The signed token is the sole threading authority; subject carries zero weight once a token verifies. Evidence: token-authority.json — a reply carrying the genuine token from reply-with-reference.json but subject `"Completely unrelated subject [...]"` still appended to conversation 15 (`appendedToRealConversation: true`, thread count 4→5, `newConversation: null`). | ||
|
|
||
| ## 4. What is deliberately NOT done | ||
|
|
||
| - **No subject-based threading**, under any condition, including exact matches and `Re:` prefixes (§3 rule 4; reply-subject-only.json, same-subject-different-customer.json). | ||
| - **No fuzzy/heuristic matching of quoted bodies.** Quoted-text detection is a known source of false-positive threading; out of scope entirely, not a fallback when token matching fails. | ||
| - **No trusting `References`/`In-Reply-To` values that don't contain one of our verified tokens.** A header that merely *looks* like a threading reference but matches no signed token carries no authority — inert, not a weaker signal. | ||
|
|
||
| Rationale: charter §2's "boringly faithful on mail semantics" principle draws the line at *reproducing proven behavior*, not *reverse-engineering or second-guessing inbound headers* clients write inconsistently. Heuristic threading is exactly the kind of "improvement" the charter's origin story (well-intentioned mail-handling changes that silently destroyed content) warns against — trading a deterministic, testable rule for a guess. | ||
|
|
||
| ## 5. Edge cases & open questions | ||
|
|
||
| - **Multiple valid tokens across `References` pointing at DIFFERENT conversations.** Not observed. Per rule 1's most-recent-first scan, the first valid token wins by construction. **OPEN QUESTION:** confirm most-recent-wins is intended — plausible since it reflects what the customer is immediately replying to, but needs its own acceptance fixture before it's load-bearing. | ||
| - **A valid token to a CLOSED/archived conversation.** Not observed. **OPEN QUESTION:** reopen, or start a new conversation referencing it? Reopen matches the charter's Help Scout-like ease-of-use bar but is undecided. | ||
| - **A valid token to a deleted conversation.** Not observed. Token verifies but its target is gone — must not crash or silently drop mail (invariant #1). **OPEN QUESTION:** likely "create a new conversation, log the orphaned-token event," undecided. | ||
| - **Forged-token rate-limiting/alerting.** forged-reply-token.json proves detection works; it says nothing about response. A single forgery is unremarkable; a burst against one conversation or sender is a security signal. **OPEN QUESTION:** threshold/alerting mechanism unspecified — security follow-up, not blocking v1 correctness. | ||
| - **`keyId` rotation.** See §2(d). | ||
| - **Auto-Submitted mail creates conversations.** auto-submitted.json: a message with `Auto-Submitted: auto-replied` was ingested normally, creating conversation 18 — not suppressed. In scope here only insofar as such mail runs through the algorithm above; whether Helpthread should suppress or specially route it (to avoid reply loops when its own auto-response gets auto-answered) is cross-referenced to a future auto-responder spec. | ||
| - **HTML `<script>` tag stored verbatim.** html-body.json: an inbound body containing `<script>alert(1)</script>` was stored and returned raw, unsanitized. Threading doesn't own sanitization, but this is a security flag: storage must not assume the reader sanitizes, and a dedicated sanitization spec is needed before HTML bodies render anywhere untrusted. | ||
|
|
||
| ## 6. Acceptance fixtures | ||
|
|
||
| The mail engine implementation (a later ticket) must pass an acceptance suite derived from these observed fixtures, carried forward from the HT-7 branch to `fixtures/mail/observed/` on `main`: | ||
|
|
||
| | Rule | Fixture | | ||
| |---|---| | ||
| | §3 rule 2 — valid token threads into same conversation | `fixtures/mail/observed/reply-with-reference.json` | | ||
| | §3 rule 3 — forged token does not thread; new conversation created | `fixtures/mail/observed/forged-reply-token.json` | | ||
| | §3 rule 4 — no token, subject-only reply → new conversation | `fixtures/mail/observed/reply-subject-only.json` | | ||
| | §3 rule 4 — no token, identical subject (even same sender) → new conversation | `fixtures/mail/observed/same-subject-different-customer.json` | | ||
| | §3 rule 5 — valid token wins over unrelated subject | `fixtures/mail/observed/token-authority.json` | | ||
| | Baseline — fresh message, no headers → new conversation | `fixtures/mail/observed/new-conversation.json` | | ||
| | Cross-ref — Auto-Submitted mail still creates a conversation | `fixtures/mail/observed/auto-submitted.json` | | ||
| | Cross-ref — HTML/script body stored verbatim (sanitization spec, not threading) | `fixtures/mail/observed/html-body.json` | | ||
|
|
||
| Each row is a minimum bar, not a ceiling: the suite should assert both the observed outcome (conversation, thread-count delta) and the specific rule from §3 that produced it, so a change that reaches the right conversation via the wrong rule (e.g. accidentally matching on subject) still fails. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced block.
The Markdown lint warning at Line 13 is valid; use a language such as
textso the documentation passes MD040.🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Source: Linters/SAST tools