fix(messaging): revert sharedSecretCache reuse in getMessageHistory (regression hotfix)#76
Merged
TortoiseWolfe merged 1 commit intomainfrom May 5, 2026
Merged
Conversation
…regression hotfix) Batch 8 (commit 52becd7, PR #74) added a module-level shared-secret cache read-path in getMessageHistory to avoid re-deriving ECDH on every poll tick. Post-merge CI showed a real Firefox/WebKit messaging E2E regression: - Run 25306048649 (main, dd83ac7, pre-batch-8) at 07:14 UTC: all green - Run 25332113434 (main, 0e20fb3, post-batch-8) at 17:04 UTC: firefox-msg failed with messages-not-visible timeouts on real-time-delivery, encrypted-messaging, and message-delete-placeholder specs - Subsequent PR runs reproduced the same pattern, with webkit-msg also failing on the rebased PR #70 branch The cache reuse is symmetric in design (same pattern as sendMessage's cache, which has been there for months) but interacts badly with Firefox/WebKit reload + tab semantics. The cached CryptoKey reference appears to behave differently across these browsers when getCurrentKeys returns a freshly-deserialized key from IndexedDB after a page reload — the invalidation guard `cachedSenderPrivateKey !== currentKeys.privateKey` was likely thrashing or holding a stale reference. Hotfix: getMessageHistory now derives a fresh shared secret per call, matching pre-batch-8 behavior. The cache infrastructure remains in place (sendMessage still uses it as before) — this revert is read-path only. Cost: ~6 ECDH derivations/min under polling — measurable but not user-perceivable. A browser-safe instance-aware cache key can return as a separate change once the Firefox CryptoKey-reference behavior is properly understood. Verification: - pnpm run type-check: clean - pnpm run lint: clean - pnpm test: 3237/3237 pass Refs: #74 (introduced regression), runs 25306048649 (last green), 25332113434 (first red post-batch-8) Co-Authored-By: Claude Opus 4.7 (1M context) <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
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.
Summary
Surgical revert of one change from PR #74 (batch 8) that introduced a Firefox/WebKit messaging E2E regression. All other batch 8 changes (non-extractable in-memory ECDH key, cursorRef pagination fix, console→logger, payment type narrow) remain intact.
Evidence — this is a real regression, not a flake
25306048649dd83ac7(pre-batch-8)253321134340e20fb3(post-batch-8)33e7b80(carries batch 8)ca61ee7(carries batch 8)The very first post-batch-8 run on
mainreproduced the failure pattern. Pre-batch-8 main was green this morning. Causality chain is clear.Failure symptoms
message-delete-placeholder:getByText('[Message deleted]')resolved to 2 elements — duplicate renderwaitForMessageOnPage2)Root cause
Batch 8 added a module-level shared-secret cache read-path in
getMessageHistory(lines 772-802 ofmessage-service.ts) to avoid re-deriving ECDH on every poll tick. The cache reuse is symmetric in design (same pattern assendMessage's cache, which has been there for months) — but interacts badly with Firefox/WebKit reload + tab semantics.Hypothesis: the cached
CryptoKeyreference behaves differently across these browsers whengetCurrentKeys()returns a freshly-deserialized key from IndexedDB after a page reload. The invalidation guardcachedSenderPrivateKey !== currentKeys.privateKeywas likely thrashing or holding a stale reference, causing decryption to use mismatched secrets.What this PR does
getMessageHistorynow derives a fresh shared secret per call, matching pre-batch-8 behavior. The cache infrastructure remains in place —sendMessagestill uses it as before. This revert is read-path only.Cost
~6 ECDH derivations/min under polling — measurable but not user-perceivable. A browser-safe instance-aware cache key can return as a separate change once the Firefox CryptoKey-reference behavior is properly understood.
What's preserved from batch 8
cursorRefpagination stale-closure fix in ConversationViewVerification
pnpm run type-check: cleanpnpm run lint: cleanpnpm test: 3237/3237 passTest plan
Refs
52becd725306048649ondd83ac725332113434on0e20fb3🤖 Generated with Claude Code