[PR #433] [MERGED] fix(snapshot): stop interrupted deletes from shadowing aliases #434

New issue

Closed

opened 2026-10-29 14:23:22 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:23:22 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/433
Author: @yingdi-shan
Created: 10/24/2026
Status: ✅ Merged
Merged: 10/26/2026
Merged by: @yingdi-shan

Base: main ← Head: fix/template-delete-tombstones

📝 Commits (1)

d1fe9fa fix(snapshot): stop interrupted deletes from shadowing aliases

📊 Changes

4 files changed (+180 additions, -17 deletions)

View changed files

📝 docs/src/concepts/templates/managing.md (+3 -0)
📝 src/api/impls/image_build/tests.rs (+45 -0)
📝 src/api/impls/template.rs (+12 -1)
📝 src/snapshot/catalog.rs (+120 -16)

📄 Description

What

Stops template deletes that stopped partway (deleting: true tombstones) from holding on to their alias:

Alias released when the delete starts (src/snapshot/catalog.rs): delete() now releases the alias right after marking the record deleting, before it removes files, the build log and the record. This is the order used before 087f599. A delete that stops partway no longer keeps the name, and it is retried by ID.
Alias claims (src/snapshot/catalog.rs): claim_alias and ensure_alias_available now treat an alias whose owner record is deleting as stale, the same way they already treat a missing owner. This heals tombstones left by earlier versions, whose aliases still point at them.
DELETE /templates/{id|name} (src/api/impls/template.rs): when lookup finds no live template, the handler now also runs the catalog delete, which finishes a partial delete. Before, this path only deleted image-build records, so a retry returned 204 and left the tombstone in place.

Why

In #432, 10 of 16 production templates became unusable: start-by-name returned 400 template <name> not found, and re-registering the name failed with an alias conflict. 087f599 moved the alias release to the end of the delete so that a retry by alias could find the record. The side effect was that any delete interrupted during the slow blob removal left the alias pointing at a record that every lookup hides.

Closes #432

Scope and non-goals

Start-by-name for a template whose delete has started still returns "not found". That is correct, because the template is being deleted. The fix is that the name can now be reused right away.
A delete that fails partway is retried by ID. Its alias is already released, so retrying by name no longer finds it, except for tombstones left by earlier versions whose alias still points at them.
No startup sweep. Once the alias is freed, a tombstone only leaves files behind, which a retry by ID removes. Tombstones stay out of GET /templates.
DELETE /snapshots/{id} is unchanged.

Design and behavior changes

Order of delete(): mark deleting → release alias → delete files → build log → record. release_alias only removes the alias while it still points at this ID, so a retry is safe, and so is a new snapshot claiming the name in the meantime.
If the process stops between marking the record and releasing the alias, the claim rule above still lets the name be registered again.

Compatibility and operations

Public API or generated protocol: no schema change. DELETE /templates/{id|name} now finishes partial deletes it used to skip.
Configuration or defaults: N/A
Snapshot manifest, artifact layout, or storage format: N/A. Records and aliases keep the same format.
Upgrade and rollback: existing tombstones no longer block their names after upgrading. Their files stay until someone runs DELETE /templates/{id} on them. Rolling back is safe.
Host requirements, permissions, ports, or dependencies: N/A

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

cargo test -p agentenv --lib -- snapshot::catalog template_deletion
  13 passed; 0 failed
cargo test -p agentenv --lib -- snapshot:: repository:: api::impls::image_build module_cycles
  197 passed; 0 failed
make fmt       -> ok
make clippy    -> ok, no warnings
make test-unit -> all suites ok (lib: 1020 passed, 5 ignored)

New tests:

snapshot::catalog::tests::deletes_that_fail_partway_release_the_alias: makes removing the snapshot directory fail, then checks that the alias is gone, the name can be registered again, and a retry by ID finishes the delete. It fails when the alias is released last.
snapshot::catalog::tests::aliases_of_snapshots_being_deleted_can_be_claimed_again: covers the create and commit claims over a tombstone, and finishing the old delete without touching the new alias.
api::impls::image_build::tests::template_deletion_by_id_finishes_an_interrupted_delete: covers the API retry path.

Skipped checks and reasons: no integration or E2E suites were run. The change is limited to catalog and API logic, which the unit tests above cover with the POSIX backend. No services/ or schema changes.

Risks and reviewer notes

Main files: src/snapshot/catalog.rs (delete order and alias claims) and src/api/impls/template.rs (delete_hidden_template).
Releasing the alias first undoes the retry-by-alias behavior that 087f599 added on purpose. Retry by ID still works because find_record returns records that are being deleted.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/433 **Author:** [@yingdi-shan](https://github.com/yingdi-shan) **Created:** 10/24/2026 **Status:** ✅ Merged **Merged:** 10/26/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/template-delete-tombstones` --- ### 📝 Commits (1) - [`d1fe9fa`](https://github.com/kvcache-ai/AgentENV/commit/d1fe9fa483a7710c2eca2eea03b50382ade771b9) fix(snapshot): stop interrupted deletes from shadowing aliases ### 📊 Changes **4 files changed** (+180 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `docs/src/concepts/templates/managing.md` (+3 -0) 📝 `src/api/impls/image_build/tests.rs` (+45 -0) 📝 `src/api/impls/template.rs` (+12 -1) 📝 `src/snapshot/catalog.rs` (+120 -16) </details> ### 📄 Description ## What Stops template deletes that stopped partway (`deleting: true` tombstones) from holding on to their alias: - **Alias released when the delete starts** (`src/snapshot/catalog.rs`): `delete()` now releases the alias right after marking the record `deleting`, before it removes files, the build log and the record. This is the order used before 087f599. A delete that stops partway no longer keeps the name, and it is retried by ID. - **Alias claims** (`src/snapshot/catalog.rs`): `claim_alias` and `ensure_alias_available` now treat an alias whose owner record is `deleting` as stale, the same way they already treat a missing owner. This heals tombstones left by earlier versions, whose aliases still point at them. - **DELETE /templates/{id|name}** (`src/api/impls/template.rs`): when lookup finds no live template, the handler now also runs the catalog delete, which finishes a partial delete. Before, this path only deleted image-build records, so a retry returned `204` and left the tombstone in place. ## Why In #432, 10 of 16 production templates became unusable: start-by-name returned `400 template <name> not found`, and re-registering the name failed with an alias conflict. 087f599 moved the alias release to the end of the delete so that a retry by alias could find the record. The side effect was that any delete interrupted during the slow blob removal left the alias pointing at a record that every lookup hides. ## Related issue Closes #432 ## Scope and non-goals - Start-by-name for a template whose delete has started still returns "not found". That is correct, because the template is being deleted. The fix is that the name can now be reused right away. - A delete that fails partway is retried by ID. Its alias is already released, so retrying by name no longer finds it, except for tombstones left by earlier versions whose alias still points at them. - No startup sweep. Once the alias is freed, a tombstone only leaves files behind, which a retry by ID removes. Tombstones stay out of `GET /templates`. - `DELETE /snapshots/{id}` is unchanged. ## Design and behavior changes - Order of `delete()`: mark `deleting` → release alias → delete files → build log → record. `release_alias` only removes the alias while it still points at this ID, so a retry is safe, and so is a new snapshot claiming the name in the meantime. - If the process stops between marking the record and releasing the alias, the claim rule above still lets the name be registered again. ## Compatibility and operations - Public API or generated protocol: no schema change. `DELETE /templates/{id|name}` now finishes partial deletes it used to skip. - Configuration or defaults: N/A - Snapshot manifest, artifact layout, or storage format: N/A. Records and aliases keep the same format. - Upgrade and rollback: existing tombstones no longer block their names after upgrading. Their files stay until someone runs `DELETE /templates/{id}` on them. Rolling back is safe. - Host requirements, permissions, ports, or dependencies: N/A ## Validation - [x] `make fmt` - [x] `make clippy` - [x] `make test-unit` - [ ] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [x] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text cargo test -p agentenv --lib -- snapshot::catalog template_deletion 13 passed; 0 failed cargo test -p agentenv --lib -- snapshot:: repository:: api::impls::image_build module_cycles 197 passed; 0 failed make fmt -> ok make clippy -> ok, no warnings make test-unit -> all suites ok (lib: 1020 passed, 5 ignored) ``` New tests: - `snapshot::catalog::tests::deletes_that_fail_partway_release_the_alias`: makes removing the snapshot directory fail, then checks that the alias is gone, the name can be registered again, and a retry by ID finishes the delete. It fails when the alias is released last. - `snapshot::catalog::tests::aliases_of_snapshots_being_deleted_can_be_claimed_again`: covers the create and commit claims over a tombstone, and finishing the old delete without touching the new alias. - `api::impls::image_build::tests::template_deletion_by_id_finishes_an_interrupted_delete`: covers the API retry path. Skipped checks and reasons: no integration or E2E suites were run. The change is limited to catalog and API logic, which the unit tests above cover with the POSIX backend. No `services/` or schema changes. ## Risks and reviewer notes - Main files: `src/snapshot/catalog.rs` (`delete` order and alias claims) and `src/api/impls/template.rs` (`delete_hidden_template`). - Releasing the alias first undoes the retry-by-alias behavior that 087f599 added on purpose. Retry by ID still works because `find_record` returns records that are being deleted. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:23:22 +00:00