You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fix ForkTapeStore.reset() so resets performed inside a fork stay isolated from the parent tape until merge-back.
Previously, reset() always called both the in-fork store reset and parent.reset(). That meant tape.reset inside a fork could immediately wipe the parent tape even when merge_back=False, breaking fork isolation and risking data loss.
What Changed
Track the active fork tape and whether it was reset within the current fork context.
In ForkTapeStore.reset():
keep immediate parent reset behavior outside a fork
keep immediate parent reset behavior for non-fork tapes
defer parent reset for the active fork tape
In fetch_all():
hide parent entries once the active fork tape has been reset in the current fork
In fork() finalization:
apply parent.reset(tape) only when merge_back=True and the forked tape was reset
then append forked entries back to the parent
Why
This restores the expected isolation semantics:
merge_back=False: fork changes, including reset, must not affect the parent tape
merge_back=True: reset should replace parent tape contents only when the fork is committed
Tests
Added regression coverage for:
reset inside fork with merge_back=False preserves parent entries
reset inside fork with merge_back=True replaces parent entries
reset inside fork hides parent entries during reads
reset outside fork still resets the parent immediately
That meant tape.reset inside a fork could immediately wipe the parent tape even when merge_back=False, breaking fork isolation and risking data loss.
Can we fix this by not wiping the parent tape when merge_back=False?
Yes, and this PR implements that in a more complete way.
Instead of only skipping the parent wipe when merge_back=False, it keeps reset fork-local during execution and only applies the parent reset when the fork is merged back. That preserves isolation semantics and avoids partial behavior differences between read-time and merge-time state.
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
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
Fix
ForkTapeStore.reset()so resets performed inside a fork stay isolated from the parent tape until merge-back.Previously,
reset()always called both the in-fork store reset andparent.reset(). That meanttape.resetinside a fork could immediately wipe the parent tape even whenmerge_back=False, breaking fork isolation and risking data loss.What Changed
ForkTapeStore.reset():fetch_all():fork()finalization:parent.reset(tape)only whenmerge_back=Trueand the forked tape was resetWhy
This restores the expected isolation semantics:
merge_back=False: fork changes, including reset, must not affect the parent tapemerge_back=True: reset should replace parent tape contents only when the fork is committedTests
Added regression coverage for:
merge_back=Falsepreserves parent entriesmerge_back=Truereplaces parent entriesVerification
uv run ruff check .uv run mypy srcuv run pytest -q