HT-8: mail threading behavioral spec - #5
Merged
Conversation
Helpthread's own threading design, evidenced entirely by the HT-7
black-box fixtures (no FreeScout source). Threading authority is
outbound-side: a signed token minted into every outbound Message-ID,
verified on the way back in.
Proposed token <ht.{conversationId}.{threadId}.{sig}@{domain}> —
properties specified (unguessable, offline-verifiable, self-describing,
rotation-tolerant), encoding left to implementation. Five-rule inbound
algorithm, each rule citing the fixture that proves it: valid token
threads (reply-with-reference), forged token rejected -> new conversation
(forged-reply-token), no token -> new conversation regardless of subject
(reply-subject-only, same-subject-different-customer), valid token beats
unrelated subject (token-authority). Explicit non-goals (no subject
threading, no body-quote heuristics). Edge cases + 5 OPEN QUESTIONs
(keyId rotation, multi-token tie-break, closed/deleted target, forged
rate-limiting). Acceptance-fixture table maps every rule to its file.
All fixture citations verified against the recorded data.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a mail threading specification based on signed tokens embedded in engine-generated ChangesMail threading
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 6
🤖 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 `@specs/mail/threading.md`:
- Around line 33-34: Reword the evidence in rule 3 to state only the observed
behavior: the tampered token was rejected and did not append to the existing
conversation, while a new conversation was created. Remove claims that this
proves cryptographic signature validation or that the system validates
signatures rather than pattern-matching; describe signature verification as the
proposed Helpthread behavior instead.
- Around line 13-15: Update the fenced code block containing the
`<ht.{conversationId}.{threadId}.{sig}@{mailDomain}>` example in the threading
documentation to specify the `text` language identifier, satisfying the MD040
Markdown lint rule.
- Around line 17-24: Define the complete reply-token contract in the threading
specification: select the HMAC hash, canonicalize conversationId and threadId
with an explicit byte encoding and framing, specify signature truncation length
and base encoding, require constant-time signature comparison, and define keyId
inclusion, active-key verification, and retired-key handling. Replace the
current keyId open question with the finalized versioned format and
verification/rotation rules while preserving the properties listed in the
surrounding spec.
- Around line 30-34: Define the RFC 5322 header parsing contract before the
candidate-token procedure, covering header unfolding, CFWS and whitespace
handling, duplicate In-Reply-To/References fields, ordering, and normalization
performed before signature verification. Specify deterministic candidate
selection across parsers and add acceptance cases for each edge input, including
duplicates and folded headers.
- Around line 33-35: Define explicit handling for valid tokens referencing
closed/archived or deleted conversations in §3 and §5: state whether each case
reopens the target or creates a new conversation, and specify whether deleted
targets emit an orphaned-token security event. Add fixtures and expected
outcomes covering both cases, ensuring identical headers produce consistent
behavior across implementations.
- Around line 11-17: Clarify the Message-ID design so every outbound message is
unique: redefine threadId as a per-message identifier, or add a distinct
per-message nonce to the signed canonical payload and resulting format. Update
the surrounding token semantics to state that conversation/thread identity
remains available while each outbound message receives a unique identifier.
🪄 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: dec43492-15c6-4973-b702-c592d6fe2a20
📒 Files selected for processing (1)
specs/mail/threading.md
Comment on lines
+11
to
+17
| 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. |
There was a problem hiding this comment.
🗄️ 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
Lines 11-24: threadId reads as a lineage identifier, and §2(c) says the token identifies the conversation/thread. If that pair is reused for every reply in a thread, outbound messages will repeat the same Message-ID. Define threadId as unique per outbound message or add a per-message nonce.
🧰 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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/mail/threading.md` around lines 11 - 17, Clarify the Message-ID design
so every outbound message is unique: redefine threadId as a per-message
identifier, or add a distinct per-message nonce to the signed canonical payload
and resulting format. Update the surrounding token semantics to state that
conversation/thread identity remains available while each outbound message
receives a unique identifier.
Comment on lines
+13
to
+15
| ``` | ||
| <ht.{conversationId}.{threadId}.{sig}@{mailDomain}> | ||
| ``` |
There was a problem hiding this comment.
📐 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 text so 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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/mail/threading.md` around lines 13 - 15, Update the fenced code block
containing the `<ht.{conversationId}.{threadId}.{sig}@{mailDomain}>` example in
the threading documentation to specify the `text` language identifier,
satisfying the MD040 Markdown lint rule.
Source: Linters/SAST tools
Comment on lines
+17
to
+24
| 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. | ||
|
|
||
| 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? |
There was a problem hiding this comment.
🔒 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 HMAC(secret, canonical(conversationId, threadId)) and notes keyId as an open question, but it still leaves the hash choice, canonical byte encoding, truncation length, encoding format, and signature comparison/retirement rules unspecified. Lock those down now so implementations stay interoperable and the forgery guarantee stays intact.
🤖 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 `@specs/mail/threading.md` around lines 17 - 24, Define the complete
reply-token contract in the threading specification: select the HMAC hash,
canonicalize conversationId and threadId with an explicit byte encoding and
framing, specify signature truncation length and base encoding, require
constant-time signature comparison, and define keyId inclusion, active-key
verification, and retired-key handling. Replace the current keyId open question
with the finalized versioned format and verification/rotation rules while
preserving the properties listed in the surrounding spec.
Comment on lines
+33
to
+35
| 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 (`<FS_reply-36-3cfea8079fafdbb9@...>`, altered from the genuine `<FS_reply-36-8aefc3079fafdbb9@...>`) 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. This also proves the reference system validates the signature rather than pattern-matching the token shape, since the forged value differs by only a few characters yet was rejected. | ||
| 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. |
There was a problem hiding this comment.
🗄️ 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.
Context: ...stomer.json — a second message with the exact same subject as an existing conversation and...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
🤖 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 `@specs/mail/threading.md` around lines 33 - 35, Define explicit handling for
valid tokens referencing closed/archived or deleted conversations in §3 and §5:
state whether each case reopens the target or creates a new conversation, and
specify whether deleted targets emit an orphaned-token security event. Add
fixtures and expected outcomes covering both cases, ensuring identical headers
produce consistent behavior across implementations.
Reworded rule 3's evidence: a forged token being rejected shows the reference system checks token integrity, not merely pattern-matches the shape — but black-box observation can't prove the mechanism is a cryptographic signature (could be an opaque lookup). Helpthread's own design uses HMAC to get the same guarantee; the spec no longer conflates observed behavior with the proposed implementation. Also generalized the now-redacted token example so it doesn't cite a scrubbed value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
This was referenced Jul 11, 2026
Merged
This was referenced Jul 13, 2026
This was referenced Jul 19, 2026
Merged
zaridan
added a commit
that referenced
this pull request
Jul 19, 2026
…raft-approval (HT-70) (#80) * feat(engine): module substrate wave 3 — assistant auth, drafts API, draft-approval orchestration (HT-70) Adds the per-Assistant bearer-token credential class (ht_asst_<id>_<secret>, constant-time verification alongside the service Bearer token), the Assistants admin API, the drafts API (create/list/approve/discard), and the draft-approval orchestration that mints a reply token for the draft's existing thread, derives the envelope exactly as sendReply does, applies HT-32 pixel injection when configured, and hands off to the unchanged delivery path. Wires author-identity forward-carry into replies/notes and emits draft.created/draft.resolved from the wave-1 event outbox inside the appendDraft/resolveDraft transactions. A wire-level equivalence fixture (src/mail/draft-equivalence.test.ts) proves assistant-draft-then-approve produces byte-identical RFC 5322 output to sendReply, modulo the Message-ID token, in both pixel-on and pixel-off configs — the mail-semantics acceptance bar for this change (CHARTER.md invariant #5). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(engine): reopen-on-approve + draft: key-prefix guard (HT-70 review) - resolveDraft's approve branch reopens a closed conversation to active in the same transaction (spec §6's reply-reopen invariant; pending stays pending, spam unreachable via the API's 409). Tests: closed reopens, active untouched, pending stays. - Reply handler rejects caller Idempotency-Keys spelled draft:* (400), closing the cross-namespace collision; store comment now states the actual guarantee. Test for the 400. Gates re-verified on this tree: typecheck 0, lint 0, test 0 (1208/1208). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(engine): close approval TOCTOU + validate approve-with-edits body (HT-70 review) - resolveDraft's approve branch now locks the parent conversation row (FOR UPDATE OF) inside the same transaction as the thread write, and is the AUTHORITATIVE check: 'deleted'/'spam' refuse outright (draft row untouched, returned as a typed sentinel the API maps to its existing 404/409 shapes), 'closed' still reopens. The API's own preflight read is now documented as a fast path only, since a concurrent delete/spam-mark between that read and the write could otherwise let mail be armed from a conversation an Agent already pulled out of the deliverable set. Tests simulate the race with a stale pre-mutation snapshot at both the approveDraft and full-API layers, plus store-level tests for the two new refusals. - approve-with-edits' bodyText override now gets the same 1-5000 length bound parseDraftBody/parseReplyBody already enforce (bodyHtml stays type-only, matching those same two paths). Tests: empty string 400, oversize 400, boundary 5000 accepted. Gates on this tree: typecheck 0, lint 0; targeted suites (conversations, approve-draft, drafts, draft-equivalence, send) 162/162. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(engine): wave-2/3 integration — cross-wave test deps after rebase (HT-70) Rebased onto main (waves 1+2 merged): union-merged the add-add conflicts in router/index/composition (function-ownership kept conversations.ts conflict-free), and each wave's test fixtures gain the OTHER wave's now- required InboxApiDeps member (dummy, never-invoked posture matching index.test.ts's precedent). Comment paths follow the Modules vocabulary. Gates on the integrated tree: typecheck 0, lint 0, test 0 (1339/1339). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(engine): atomic assistant-auth read + contained store failures; biome fixes (HT-70 review) CodeRabbit round 1 on #80 (5 actionables): - getForAuth replaces getTokenHash: status + token_hash from ONE row read, so disable/rotation can never interleave between separate queries and validate stale credentials - assistant auth's pre-try await is contained at the call site — a store failure returns the controlled server_error envelope (with no-store), never an uncontrolled host-runtime 500; new end-to-end test proves it - biome import-order/format violations from the rebase integration patch fixed (the Quality gate failure; local lint had been misread through a piped exit code — re-verified bare) Gates: typecheck 0, lint 0, test 0 (1340/1340). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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 crown-jewel spec — how inbound mail is assigned to a conversation — derived entirely from the HT-7 black-box fixtures and public standards (RFC 5322 §3.6.4), no FreeScout source.
Core: threading authority is outbound-side. The engine mints a signed token into every outbound Message-ID and threads a reply only by finding and verifying that token — never by subject, never by trusting client-echoed headers alone. Proposed token
<ht.{conversationId}.{threadId}.{sig}@{domain}>; properties are the spec, encoding is implementation.Five-rule inbound algorithm, every rule citing the fixture that proves it:
Explicit non-goals (no subject threading, no quoted-body heuristics — the charter's 'boringly faithful' line). 5 OPEN QUESTIONs flagged (keyId rotation, multi-token tie-break, valid-token-to-closed/deleted-conversation, forged-token alerting). Acceptance-fixture table maps each rule to its
fixtures/mail/observed/*.json(lands on main with HT-7).All fixture citations were verified against the recorded data during review.
Jira: https://resonantiq.atlassian.net/browse/HT-8
🤖 Generated with Claude Code
https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
Summary by CodeRabbit