fix(buffer): Maintain Managed for proper bookkeeping
#5546
Conversation
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
Changed the buffer service to maintain Managed<Box<Envelope>> throughout the entire push operation, ensuring that any failure automatically rejects the envelope when the Managed wrapper is dropped. Key changes: - Modified EnvelopeStack trait to accept Managed<Box<Envelope>> instead of Box<Envelope> - Updated SqliteEnvelopeStack::push to accept envelopes only after successful encoding - Updated MemoryEnvelopeStack::push to immediately accept (cannot fail) - Updated CachingEnvelopeStack to cache managed envelopes and accept only on pop - Modified all buffer layers (EnvelopeBuffer, PolymorphicEnvelopeBuffer, top-level push) to pass through Managed wrappers - Added managed_envelope() and managed_envelopes() test utilities - Updated all tests to use managed envelopes This approach enforces that envelopes are only accepted after successful storage, and any error automatically triggers outcome rejection via Drop, eliminating the need for manual error handling and outcome tracking. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Changed MemoryEnvelopeStack to store managed envelopes and only accept them on pop, making it consistent with the caching stack pattern. - MemoryEnvelopeStack now stores Vec<Managed<Box<Envelope>>> instead of Vec<Box<Envelope>> - Envelopes are only accepted when popped, not when pushed - Removed FIXME comment from caching stack - Fixed unused variable warning in test This ensures envelopes are only truly "accepted" when retrieved from the stack, maintaining the invariant that accept is only called on pop or after successful persistence in SQLite. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Extend the Managed envelope pattern to the pop path: - Change EnvelopeStack::pop() to return Option<Managed<Box<Envelope>>> - SqliteEnvelopeStack stores Option<Addr<TrackOutcome>> that gets populated on first push, used to create Managed instances on pop - MemoryEnvelopeStack returns managed envelope directly from storage - CachingEnvelopeStack returns managed envelope from cache or inner - EnvelopeBufferService uses managed envelope from pop directly Also fixes: - Outcome category order in Counted impl to match ManagedEnvelope::reject - Flaky test by using same envelopes for threshold calculation and pushing Co-Authored-By: Claude Opus 4.5 <[email protected]>
jjbayer
force-pushed
the
feat/buffer-managed-envelope-push
branch
from
a4e9d38 to
3cee5b7
Compare
jjbayer
commented
Jan 19, 2026
jjbayer
commented
Jan 19, 2026
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.
Pass
Managedenvelopes into the envelope buffer so that outcomes are reported when they are dropped. When reading envelopes from the buffer, convert them back into managed as well.Fixes RELAY-44.