Repository navigation
fix(context-manager): never stash an offload preview under its original's key - #5623
Closed
strandly-the-agent wants to merge 2 commits into
Closed
strandly-the-agent wants to merge 2 commits into
strandly-the-agent wants to merge 2 commits into
Conversation
…al's key After a session restore onto a host whose stash no longer holds the originals, the one-time backfill stored every restored block, including already-truncated tool-result previews, under the original's reference. retrieve_context then served the preview as the original. Backfill now skips blocks that are offload markers, and retrieve_context refuses to serve a stashed entry that is itself a marker, which also covers stores already polluted by earlier releases. Fixes strands-agents#5620
…its own key Review follow-up: bound the ref-token regex (no backtracking across '[') and require the marker to reference the key it would be stored under, so an original that merely cites another stash entry is still stored and served.
Member
|
closing out in favor of #5626 which resolves in py and ts |
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.
Requested by: @ekumar96 — Slack DM 2026-10-28: after a session restore onto a host whose stash no longer holds the originals,
retrieve_contextreturns the truncated preview as if it were the original.Human Overview
Description
After a session restore,
ContextManager._backfill_stashrunsstore_message(..., keep_existing=True)over every restored message.keep_existingonly protects keys the stash already holds, so when the stash on the new host is empty (new host, expired store, failed write) an already-truncated tool-result preview is written under the original's key<toolUseId>_0. From then onretrieve_contextserves the 9-line preview as the "original": a pattern search reports no matches and a full retrieval returns[Truncated: …], with nothing telling the model the content is gone. #4699 fixed the sibling case where the stash still holds the original; this is the complement.Two changes. On the backfill path,
Stash.store_messagenow skips a block that is itself the offload marker for the key it would be stored under: every strategy's replacement ends in the[ref: …]/[refs: …]token that only_format_stash_refsproduces, so a block whose trailing token names its own key stands in for an original and must not be stored in its place. The liveMessageAddedEventpath is unchanged, and an original that merely cites another entry is still stored. Separately,retrieve_contextrefuses to serve a stashed entry that is the marker for the requested reference ("original content … is no longer available; only its offloaded preview remains"), which also covers durable stores already polluted by 1.62.0. TypeScript has no backfill step and is unaffected.Related Issues
Fixes #5620
Documentation PR
N/A
Type of Change
Bug fix
Testing
SDK-only repro (two
ContextManagers, the second seeded with the first one's messages and an empty stash) returns the preview onmainandError: reference not foundwith this change — script and both outputs are attached to the issue card. New unit tests: backfill onto an empty stash leaves the original's key empty while still stashing plain blocks;_is_offload_markeragainst every strategy's output and against originals that cite another ref; retrieval guard rejects a stashed preview but serves an original that cites another ref; bracket-heavy 200 KB input stays bounded. Both behaviour tests fail without the fix.hatch run prepare(equivalent, see details)Additional Details
Gate output and review ledger
The 10 failures (
bidi/test_deprecated_alias.py,bidi/agent/test_agent.py,test_agent.py::test_baggage_attributes_stamped_on_agent_span) and 23 collection errors are identical onmainin the same venv — missing optional deps (a2a,fastapi, …), unrelated to this change.Independent review (fresh-context reviewer) raised two 🟡 findings, both fixed in the second commit:
\[refs?: [^\]]*_\d+\]$backtracked quadratically on bracket-heavy text (600 KB → 60 s). Now[^\[\]]*, bounded; test added.[ref: …]token would have been skipped on backfill / rejected on retrieval. The check now requires the token to name the block's own key (refs_for) or the requestedreference, so only true self-referencing markers are affected; tests added for both paths.Known limit: a stash already polluted by 1.62.0 keeps the preview under the original's key (backfill never overwrites); the retrieval guard makes that honest rather than recovering the original, which is gone.
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.