fix(storage): don't assume V4+ ancestor gaps are uncompressed

[arkamar]

On every header version, LoadHeader filled a zero BuildData for any build the mapping referenced but Builds did not cover. That is fair on V3, which has neither a Builds section nor compression, but on V4+ the gap means the entry was lost, not that the build is uncompressed.

createDiff reads the zero entry as CompressionNone and composes the suffix-less buildID/memfile. For a compressed ancestor that object does not exist, and because a zero entry also latches UncompressedFullFrameTable the source is never re-resolved, so every fault into that ancestor fails for the lifetime of the diff.

Restricting the backfill on V4+ to the header's own build keeps the one case that needs it — runV3 never writes a self entry and only runs with compression off — while leaving ancestor gaps absent, so createDiff resolves them from the build's own header instead. The trade is one header GET per V3-era ancestor on first fault, giving back #2999.

[cursor]

PR Summary

Medium Risk
Touches snapshot resume and diff resolution for V4+ memfile headers; wrong behavior previously bricked resumes, and the fix trades extra header fetches on first fault for ambiguous ancestor gaps.

Overview
LoadHeader no longer fabricates zero BuildData entries for missing ancestor builds on V4+ headers; only the header’s own build still gets the uncompressed sentinel, while V3 keeps backfilling every mapped build. That stops createDiff from treating lost compressed ancestor metadata as uncompressed and permanently faulting on the wrong memfile path, and instead lets the no-entry path refresh from the ancestor’s real header. Tests cover V3 backfill, pause/serialize round-trips for absent ancestor gaps, and an integration read that goes through LoadHeader.

^{Reviewed by Cursor Bugbot for commit 3c447aa. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

❌ 6 Tests Failed:

Tests completed	Failed	Passed	Skipped
3648	6	3642	9

View the full list of 8 ❄️ flaky test(s)

github.com/e2b-dev/infra/tests/integration/internal/tests/api/templates::TestTemplateBuildDistroFamilies

Flake rate in main: 23.44% (Passed 49 times, Failed 15 times)

Stack Traces | 0s run time

=== RUN   TestTemplateBuildDistroFamilies
=== PAUSE TestTemplateBuildDistroFamilies
=== CONT  TestTemplateBuildDistroFamilies
--- FAIL: TestTemplateBuildDistroFamilies (0.00s)

github.com/e2b-dev/infra/tests/integration/internal/tests/api/templates::TestTemplateBuildDistroFamilies/Arch

Flake rate in main: 23.81% (Passed 48 times, Failed 15 times)

Stack Traces | 301s run time

=== RUN   TestTemplateBuildDistroFamilies/Arch
=== PAUSE TestTemplateBuildDistroFamilies/Arch
=== CONT  TestTemplateBuildDistroFamilies/Arch
    build_template_test.go:159: test-distro-arch: [info] Building template cs8iukrs1oup5e4jf7i2/a8695f47-0943-4bd1-b54c-9cbc3adbeffc
    build_template_test.go:159: test-distro-arch: [info] [base] FROM archlinux:base [92783431813c1525ce11fdf675e5f05309f1ad27383c7d264d4206c279a34fbb]
    build_template_test.go:159: test-distro-arch: [info] Base Docker image size: 133 MB
    build_template_test.go:159: test-distro-arch: [info] Creating file system and pulling Docker image
    build_template_test.go:159: test-distro-arch: [info] Uncompressing layer sha256:b29eedb0cba68b40ef1c21ad33ace0b0b65e43c800ce9ae3fb885ed388c8ff1f 133 MB
    build_template_test.go:159: test-distro-arch: [info] Uncompressing layer sha256:79c8b8c2b0d09dc3a8ac0a624ff34d637e7d300e2aa444e5daf77dea4d38cdd4 8.7 kB
    build_template_test.go:159: test-distro-arch: [info] Uncompressing layer sha256:bedd843c9cbbcb248831d66212fd634a2cc6a255ede1601d56bb6fc8dba9c391 14 MB
    build_template_test.go:159: test-distro-arch: [info] Uncompressing layer sha256:e216621d8a0a7a4486d6a228759b29bc378eff95e6ea9469854b8624b267dd13 137 B
    build_template_test.go:159: test-distro-arch: [info] Layers extracted
    build_template_test.go:159: test-distro-arch: [info] Root filesystem structure: bin, boot, dev, etc, home, lib, lib64, mnt, opt, proc, root, run, sbin, srv, sys, tmp, usr, var
    build_template_test.go:159: test-distro-arch: [info] Provisioning sandbox template
    build_template_test.go:159: test-distro-arch: [info] Provisioning was successful, cleaning up
    build_template_test.go:159: test-distro-arch: [info] Sandbox template provisioned
    build_template_test.go:159: test-distro-arch: [info] [base] DEFAULT USER user [f3d3bf06d501aa6f9d41fb8eb035aec7972af51ad446b8faa9123e46679a495b]
    distro_build_test.go:66: 
        	Error Trace:	.../api/templates/build_template_test.go:124
        	            				.../api/templates/distro_build_test.go:66
        	Error:      	Received unexpected error:
        	            	Get "http://localhost:.../builds/a8695f47-0943-4bd1-b54c-9cbc3adbeffc/status?logsOffset=14&level=info": context deadline exceeded
        	Test:       	TestTemplateBuildDistroFamilies/Arch
--- FAIL: TestTemplateBuildDistroFamilies/Arch (300.62s)

github.com/e2b-dev/infra/tests/integration/internal/tests/api/templates::TestUpdateTemplateWithInvalidAPIKey

Flake rate in main: 11.19% (Passed 119 times, Failed 15 times)

Stack Traces | 154s run time

=== RUN   TestUpdateTemplateWithInvalidAPIKey
=== PAUSE TestUpdateTemplateWithInvalidAPIKey
=== CONT  TestUpdateTemplateWithInvalidAPIKey
    template_update_test.go:107: Build failed: {<nil> An internal error occurred. Please try again or contact support with the build ID. <nil>}
--- FAIL: TestUpdateTemplateWithInvalidAPIKey (154.23s)

github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir

Flake rate in main: 26.13% (Passed 212 times, Failed 75 times)

Stack Traces | 1.25s run time

=== RUN   TestListDir
=== PAUSE TestListDir
=== CONT  TestListDir
--- FAIL: TestListDir (1.25s)
Executing command cat in sandbox ihclz7ijqmfz0qkjoxj3l (user: root)

github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_0_lists_only_root_directory

Flake rate in main: 30.43% (Passed 48 times, Failed 21 times)

Stack Traces | 0.02s run time

=== RUN   TestListDir/depth_0_lists_only_root_directory
=== PAUSE TestListDir/depth_0_lists_only_root_directory
=== CONT  TestListDir/depth_0_lists_only_root_directory
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_0_lists_only_root_directory
--- FAIL: TestListDir/depth_0_lists_only_root_directory (0.02s)

github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_1_lists_root_directory

Flake rate in main: 23.30% (Passed 158 times, Failed 48 times)

Stack Traces | 0.01s run time

=== RUN   TestListDir/depth_1_lists_root_directory
=== PAUSE TestListDir/depth_1_lists_root_directory
=== CONT  TestListDir/depth_1_lists_root_directory
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_1_lists_root_directory
--- FAIL: TestListDir/depth_1_lists_root_directory (0.01s)

github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)

Flake rate in main: 50.00% (Passed 1 times, Failed 1 times)

Stack Traces | 0.01s run time

=== RUN   TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
=== PAUSE TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
=== CONT  TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory)
--- FAIL: TestListDir/depth_2_lists_first_level_of_subdirectories_(in_this_case_the_root_directory) (0.01s)

github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestListDir/depth_3_lists_all_directories_and_files

Flake rate in main: 23.14% (Passed 93 times, Failed 28 times)

Stack Traces | 0.01s run time

=== RUN   TestListDir/depth_3_lists_all_directories_and_files
=== PAUSE TestListDir/depth_3_lists_all_directories_and_files
=== CONT  TestListDir/depth_3_lists_all_directories_and_files
    filesystem_test.go:96: 
        	Error Trace:	.../tests/envd/filesystem_test.go:96
        	Error:      	Received unexpected error:
        	            	unavailable: 502 Bad Gateway
        	Test:       	TestListDir/depth_3_lists_all_directories_and_files
--- FAIL: TestListDir/depth_3_lists_all_directories_and_files (0.01s)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

[claude]

LGTM — the backfill fix is narrowly scoped and the new tests pin the exact before/after behavior on both V3 and V4+ headers; the inline nit is a stale comment elsewhere, not a functional issue.

Extended reasoning...

Overview

This PR changes backfillMissingV3UncompressedBuilds in packages/shared/pkg/storage/header/serialization.go so that on V4+ headers, only the header's own build gets a zero-BuildData sentinel inserted for mapping-referenced builds missing from Builds; ancestor gaps are now left absent instead of being fabricated as 'uncompressed'. This routes createDiff (in packages/orchestrator/pkg/sandbox/build/storage_diff.go, not touched by this PR) through its existing refreshHeader fallback for those ancestors, resolving the true compression path instead of permanently faulting on a zstd-compressed ancestor treated as uncompressed. Tests in both serialization_test.go and storage_diff_test.go are updated/added to pin the new behavior, including a regression test that round-trips through LoadHeader (the existing tests in that file all hand-built headers and skipped the backfill entirely, which is why the original bug went unnoticed).

Security risks

None. This is internal storage-header bookkeeping with no user input, auth, or crypto surface.

Level of scrutiny

This touches the snapshot/resume critical path (header backfill feeds directly into diff/UFFD fault resolution), so it warrants more than a rubber stamp. However, the change itself is small and surgical (one added conditional in one function), the reasoning in the PR description is precise about why V3 vs V4+ differ (V3 has no Builds section or compression at all; V4+ writers always record an entry for every build they know about, so a gap is lost information, not an implicit 'uncompressed'), and I traced the consuming code in storage_diff.go's createDiff to confirm the default (no-entry) branch already handles this correctly via probe-then-refresh — this is exactly the path the new TestStorageDiff_AbsentEntryOnV4ResolvesFromOwnHeader test exercises. I also checked the V5 backfill test's inverted assertions against the V3-only sibling test and confirmed both are internally consistent with the new v4plus gate.

Other factors

Test coverage is strong: TestFillMissingBuildsAsSentinels (V5, updated), the new TestFillMissingBuildsAsSentinels_V3, and the new TestStorageDiff_AbsentEntryOnV4ResolvesFromOwnHeader together pin both the old-format sentinel behavior and the new-format absent-gap behavior. The one inline finding is a documentation nit (a comment in an untouched file, storage_diff.go, whose invariant is invalidated by this change) rather than a functional bug, so it doesn't block approval.

[ValentaTomas]

Reviewed this end to end against a live reproduction, and traced the read path rather than just the diff. The approach is right and I verified the full chain works: absent entry → createDiff default branch → OpenSeekable at the basic name is lazy so it does not 404 → not peer-routed → refreshHeader → openFromLoadedHeader latches the authoritative full frame table → StorageDiff.resolve returns that table first, so the chunker reads the compressed object at correct offsets. resolve's nil-callerFT branch is a second backstop (reloadProactive) if a diff ever exists without a latched table.

Two things worth noting, pushed as a follow-up commit rather than left as comments:

1. createDiff's hasEntry=false comment still asserted that storage-loaded headers always carry an entry, so only peer-served headers reach that branch. This PR is exactly what invalidates that — a V4+ ancestor gap now lands there by design. That is the kind of stale invariant someone later "simplifies" straight back into the 404, so I rewrote it to name both sources.

2. The fix depends on GetBuildFrameData returning nil (unknown) for an absent entry rather than UncompressedFrameTable — nil is what routes to the ancestor's own header instead of the suffix-less name. Nothing pinned that: a later change returning the uncompressed sentinel for absent entries would restore the permanent 404 with every existing assertion still green. Now asserted, mirroring the V3 test that asserts the opposite.

On safety, I ran a negative control: reverting just the production guard makes both TestFillMissingBuildsAsSentinels and TestStorageDiff_AbsentEntryOnV4ResolvesFromOwnHeader fail, so the coverage genuinely guards the behaviour instead of merely describing it. Full pkg/storage/header and pkg/sandbox/build suites pass (the latter needs a Linux container for the build constraints).

Two smaller observations, no change requested. Mapping.Builds() excludes uuid.Nil (holes use a separate index sentinel), so hole mappings never reach the backfill and cannot lose a sentinel — worth knowing since the fault path relies on nil checks upstream. And the self-entry carve-out is load-bearing beyond runV3: SelfBuildData documents that a missing self entry is only possible on peer-served headers, which restricting the skip to bid != Metadata.BuildId preserves.

Independently, I can confirm this is not a storage-compaction side effect — the referenced objects all exist in their compressed form, none were written or touched by compaction tooling, and the failures predate the current campaign. Not approving or merging, leaving that to code owners.

[ValentaTomas]

Second audit pass, focused on every consumer an absent entry can now reach:

• dedup.go (parentFetchKey, countExternalBlocks): nil FT degrades to windowBytes-bucketed fetch keys — planning granularity only, no correctness impact. copy-build/inspect-build: nil-safe or presence-checked.
• The !hasEntry probe can't eagerly 404: OpenSeekable is lazy on all providers (GCS/AWS/fs), and on the non-peer path the probe upstream is replaced by openFromLoadedHeader before any read.
• runV3 ancestor reached through a gap: its own header's backfilled zero self entry → SelfBuildData → FullFromTable(nil) = UncompressedFullFrameTable, so the diff still latches authoritative-uncompressed and size 0 is re-statted. No refresh loop.
• metadataFormatVersion masks feature bits, so a future V6 defaults to the restrictive (gap-preserving) backfill.

One leg was unpinned: gap persistence across a pause generation. newDiffHeader copies only entries the parent has and writeV4BuildsSection writes only entries that exist — if either ever fabricated an entry per mapping-referenced build, the gap would persist as a zero entry ("authoritatively uncompressed") in the child header, where this fix can no longer help, and the 404 would return for grandchild resumes with every existing test green. Added TestToDiffHeader_AncestorGapRoundTripsAbsent (pause → serialize → reload), negative-controlled against both fabrication points.