[Parked] feat(snapshot): E2B alias rebuild semantics on template publish - #70
[Parked] feat(snapshot): E2B alias rebuild semantics on template publish#70JoyboyBrian wants to merge 2 commits into
Conversation
|
🔍 OpenCodeReview found 4 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // `import_built_artifacts` copies `vm_state.bin` first, so removing it | ||
| // fails the publish before any catalog state is committed. | ||
| fs::remove_file(&broken_artifacts.vm_state.path).expect("remove seeded vm state"); |
There was a problem hiding this comment.
[test · medium]
This failure occurs before commit_publish, so the test does not exercise the newly added rollback ordering that must preserve the prior alias after the commit marker/new record has been written but alias rebinding fails. A regression in rollback_failed_publish (for example, leaving the committed marker/record or damaging the previous binding) would still pass this test. Add fault injection around the catalog alias write, or otherwise force a late commit failure, and assert the old binding/record are restored and the new directory is removed.
There was a problem hiding this comment.
Good point that this test does not inject a failure at the final alias write. Its purpose is to verify the repository-level contract that a failed rebuild leaves the previous alias usable and cleans up the new snapshot directory, rather than to exercise every internal rollback branch.
A deterministic failure between the committed-record write and the atomic alias rename would require a catalog fault-injection hook solely for this test. I’d prefer not to add that machinery in this focused change. The ordering is explicit: the atomic alias rename is the final fallible catalog operation, so the previous binding remains untouched if it fails.
|
@yingdi-shan I reviewed the four automated comments against the supported snapshot publication lifecycle and replied to each thread individually. Three threads are resolved: they assume unsupported concurrent/same-ID publication or treat |
|
We're considering a refactor of the alias system over the next few months, which may make it difficult to merge this PR. Could we proceed with COPY/ADD execution support first instead? |
|
Hi @yingdi-shan thanks for your comment! Yes, that makes sense. The COPY/ADD layers are separable from the alias-rebuild change. I verified that #71–#74 can be replayed without #70. I’ll rebase that stack onto the current main branch, update the documentation so rebuilding an existing alias remains unsupported for now, and rerun the Linux validation and fresh-template COPY/ADD E2E tests. I’ll park #70 pending the planned alias refactor. |
JoyboyBrian
changed the title
2 participants
Status: parked
This PR was moved to draft on August 6, 2026 (PT), following reviewer guidance that the alias system is expected to be refactored. It is intentionally not part of the active COPY/ADD stack and is not required by #71–#74.
The branch and discussion are being preserved rather than closed so the behavior and review history can be revisited after the alias refactor has a stable direction.
What this explored
E2B-style rebuild semantics for snapshot aliases:
Why it is parked
The proposed behavior modifies alias binding, publication ordering, rollback, and previous-record cleanup. Landing that immediately before a broader alias-system refactor would create avoidable merge and migration work.
Existing-alias rebuild therefore remains unsupported for now. Callers should use a new alias or delete the existing template before rebuilding.
Active COPY/ADD stack
e2b-build/03-build-file-storee2b-build/04-upload-apie2b-build/05-copy-plane2b-build/06-copy-execThose branches were replayed directly onto current main at
00ba6cb; none contains this PR.Reference
Related: #28, which retains the original full-stack concept, design discussion, and earlier E2E evidence.