[PR #382] [MERGED] refactor(repository): stop depending on snapshot, image, volume and sandbox #391

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: refactor/repository-leaf

📝 Commits (1)

85358c3 refactor(repository): stop depending on snapshot, image, volume and sandbox

📊 Changes

41 files changed (+1601 additions, -1583 deletions)

View changed files

📝 crates/e2e-tests/tests/snapshot_oss_e2e_test.rs (+5 -4)
📝 docs/src/internals/template-builder-testing.md (+4 -3)
📝 src/api/impls/image_build/tests.rs (+1 -1)
📝 src/api/impls/image_build/worker.rs (+5 -2)
📝 src/api/impls/snapshots.rs (+1 -1)
📝 src/api/impls/template.rs (+2 -3)
📝 src/image/buildkit/history.rs (+2 -2)
📝 src/orchestrator/tests.rs (+2 -2)
📝 src/repository/backends/mod.rs (+2 -180)
📝 src/repository/backends/oss/mod.rs (+0 -106)
📝 src/repository/backends/oss/repository.rs (+27 -60)
📝 src/repository/backends/posixfs/artifacts.rs (+1 -414)
📝 src/repository/backends/posixfs/backend.rs (+8 -633)
📝 src/repository/backends/posixfs/mod.rs (+0 -3)
📝 src/repository/build_cache.rs (+11 -9)
📝 src/repository/errors.rs (+1 -1)
📝 src/repository/interfaces.rs (+7 -111)
📝 src/repository/layout.rs (+15 -7)
📝 src/repository/mod.rs (+38 -1)
➕ src/snapshot/backends/mod.rs (+288 -0)

...and 21 more files

📄 Description

What

Make src/repository a leaf module: it no longer imports snapshot, image, volume or sandbox. After this change it depends only on cfg, digest, types and (for one OSS metrics guard) observability.

Moved	From	To
`SnapshotId`, `SnapshotAlias`	`snapshot/types/value.rs`	`types/snapshot_id.rs` (still re-exported from `crate::snapshot`)
`SnapshotListFilter`	`repository/interfaces.rs`	`snapshot/catalog.rs`
`SnapshotPublication`	`repository/interfaces.rs`	`snapshot/publish.rs`
`SnapshotRuntimeResolver` trait	`repository/interfaces.rs`	`snapshot/resolver.rs`
`PosixFsBackend`, `PosixFsBackendConfig`, `OssBackend`, `build_snapshot_backend`, `build_builder_snapshot_backend`	`repository/backends/{mod.rs,oss/mod.rs,posixfs/backend.rs}`	`snapshot/backends/mod.rs`
Snapshot file names (`vm_state.bin`, `firecracker-manifest.json`, `memory-startup.pack`)	`SNAPSHOT_ARTIFACT_LAYOUT` / `MEMORY_STARTUP_PACK_ARTIFACT`	`SnapshotFile::name()` in `repository/layout.rs`; the snapshot constants now derive from it
`is_valid_volume_component`	`volume/mod.rs`	`repository/layout.rs`, next to the volume document keys it guards (`volume` re-exports it)

The repository side keeps two small constructors that the snapshot wiring uses: PosixFsRepository::new(root) and OssRepository::from_config(..).

Why

The repository's job is to store blobs and documents for the catalogs built on it. But it also imported those catalogs: it built the snapshot runtime resolver and artifact cache, took the image layer store, defined snapshot-specific filter, publication and resolver types, and named its stored files through snapshot constants. As a result repository and snapshot depended on each other, and neither could be read or changed without the other. This is item 3 of the post-#380 architecture review (break the module cycles), limited to the repository edges.

None. This follows the architecture review done after #380.

Scope and non-goals

Code moves and import updates only. No logic changes.
Tests that exercised snapshot publish, store and resolve through a repository move with that code. All 19 keep their names and bodies; only their imports and the test_repository helper change:
- posixfs/backend.rs and backends/mod.rs tests → snapshot/backends/tests.rs
- posixfs/artifacts.rs tests → snapshot/store/posix_tests.rs
- the OSS volume-backing test → snapshot/resolver.rs
The build-cache test stays in repository and now opens PosixFsRepository directly.
Not included:
- repository → observability: one MetricGuard in the OSS client.
- the other cycles from the review (image → template/snapshot/sandbox, volume → snapshot, observability → orchestrator).
- repository/backends/common::write_dense_overlaybd_layer_to_file, which only snapshot uses but has no outward dependency.

Design and behavior changes

No behavior change. A new unit test, repository::tests::repository_does_not_depend_on_its_users, scans src/repository/**/*.rs and fails if any file references crate::{snapshot,image,volume,sandbox}::. I checked that it fails when such an import is added back.

Compatibility and operations

Public API or generated protocol: Rust import paths of the agentenv lib change. Use agentenv::snapshot::backends::{PosixFsBackend, PosixFsBackendConfig, OssBackend} instead of agentenv::repository::backends::…, and agentenv::snapshot::{SnapshotListFilter, SnapshotRuntimeResolver, SnapshotPublication} instead of agentenv::repository::…. The workspace callers (tests/common, crates/e2e-tests) are updated. HTTP API: N/A.
Configuration or defaults: N/A.
Snapshot manifest, artifact layout, or storage format: unchanged. The three stored file names are now defined once in repository/layout.rs, with the same values. The existing blob_paths_match_existing_repositories test pins them.
Upgrade and rollback: N/A.
Host requirements, permissions, ports, or dependencies: N/A.

Validation

make fmt
make clippy
make test-unit (the agentenv lib suite was run; see below)
Relevant Rust integration tests (compiled by clippy --all-targets, not run)
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:

make fmt                              # ok
make clippy                           # ok (workspace, all targets/features, -D warnings)
cargo test -p agentenv --lib          # 984 passed, 0 failed, 4 ignored
                                      # baseline on origin/main: 983 passed, 4 ignored (+1 new guard test)

Skipped checks and reasons:

tests/ integration and crates/e2e-tests were compiled (clippy --all-targets) but not run. They need KVM, ublk or an object-storage service, and their only change is an import path.
The capability-runner (--ignored) tests and non-agentenv crates in make test-unit were not run: this PR does not touch them.
services/ is unchanged.

Risks and reviewer notes

Most of the diff is moved code. With git diff -M, types/snapshot_id.rs shows as a rename. The new snapshot/backends/mod.rs is repository/backends/mod.rs + oss/mod.rs + the bundle half of posixfs/backend.rs, unchanged except that the repository is built with PosixFsRepository::new(root) / OssRepository::from_config.
snapshot/image_export/service.rs used to assemble PosixFsRepository from its two internal stores. It now calls PosixFsRepository::new(root), and those stores are private to posixfs.
docs/src/internals/template-builder-testing.md is updated for the moved resolver trait and tests.

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. (No new behavior; existing tests moved with the code. One new guard test.)
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/382 **Author:** [@yingdi-shan](https://github.com/yingdi-shan) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/14/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `refactor/repository-leaf` --- ### 📝 Commits (1) - [`85358c3`](https://github.com/kvcache-ai/AgentENV/commit/85358c3f4acfde4ad7a4167c6827529cab4e894b) refactor(repository): stop depending on snapshot, image, volume and sandbox ### 📊 Changes **41 files changed** (+1601 additions, -1583 deletions) <details> <summary>View changed files</summary> 📝 `crates/e2e-tests/tests/snapshot_oss_e2e_test.rs` (+5 -4) 📝 `docs/src/internals/template-builder-testing.md` (+4 -3) 📝 `src/api/impls/image_build/tests.rs` (+1 -1) 📝 `src/api/impls/image_build/worker.rs` (+5 -2) 📝 `src/api/impls/snapshots.rs` (+1 -1) 📝 `src/api/impls/template.rs` (+2 -3) 📝 `src/image/buildkit/history.rs` (+2 -2) 📝 `src/orchestrator/tests.rs` (+2 -2) 📝 `src/repository/backends/mod.rs` (+2 -180) 📝 `src/repository/backends/oss/mod.rs` (+0 -106) 📝 `src/repository/backends/oss/repository.rs` (+27 -60) 📝 `src/repository/backends/posixfs/artifacts.rs` (+1 -414) 📝 `src/repository/backends/posixfs/backend.rs` (+8 -633) 📝 `src/repository/backends/posixfs/mod.rs` (+0 -3) 📝 `src/repository/build_cache.rs` (+11 -9) 📝 `src/repository/errors.rs` (+1 -1) 📝 `src/repository/interfaces.rs` (+7 -111) 📝 `src/repository/layout.rs` (+15 -7) 📝 `src/repository/mod.rs` (+38 -1) ➕ `src/snapshot/backends/mod.rs` (+288 -0) _...and 21 more files_ </details> ### 📄 Description ## What Make `src/repository` a leaf module: it no longer imports `snapshot`, `image`, `volume` or `sandbox`. After this change it depends only on `cfg`, `digest`, `types` and (for one OSS metrics guard) `observability`. | Moved | From | To | | --- | --- | --- | | `SnapshotId`, `SnapshotAlias` | `snapshot/types/value.rs` | `types/snapshot_id.rs` (still re-exported from `crate::snapshot`) | | `SnapshotListFilter` | `repository/interfaces.rs` | `snapshot/catalog.rs` | | `SnapshotPublication` | `repository/interfaces.rs` | `snapshot/publish.rs` | | `SnapshotRuntimeResolver` trait | `repository/interfaces.rs` | `snapshot/resolver.rs` | | `PosixFsBackend`, `PosixFsBackendConfig`, `OssBackend`, `build_snapshot_backend`, `build_builder_snapshot_backend` | `repository/backends/{mod.rs,oss/mod.rs,posixfs/backend.rs}` | `snapshot/backends/mod.rs` | | Snapshot file names (`vm_state.bin`, `firecracker-manifest.json`, `memory-startup.pack`) | `SNAPSHOT_ARTIFACT_LAYOUT` / `MEMORY_STARTUP_PACK_ARTIFACT` | `SnapshotFile::name()` in `repository/layout.rs`; the snapshot constants now derive from it | | `is_valid_volume_component` | `volume/mod.rs` | `repository/layout.rs`, next to the volume document keys it guards (`volume` re-exports it) | The repository side keeps two small constructors that the snapshot wiring uses: `PosixFsRepository::new(root)` and `OssRepository::from_config(..)`. ## Why The repository's job is to store blobs and documents for the catalogs built on it. But it also imported those catalogs: it built the snapshot runtime resolver and artifact cache, took the image layer store, defined snapshot-specific filter, publication and resolver types, and named its stored files through snapshot constants. As a result `repository` and `snapshot` depended on each other, and neither could be read or changed without the other. This is item 3 of the post-#380 architecture review (break the module cycles), limited to the `repository` edges. ## Related issue None. This follows the architecture review done after #380. ## Scope and non-goals - Code moves and import updates only. No logic changes. - Tests that exercised snapshot publish, store and resolve *through* a repository move with that code. All 19 keep their names and bodies; only their imports and the `test_repository` helper change: - `posixfs/backend.rs` and `backends/mod.rs` tests → `snapshot/backends/tests.rs` - `posixfs/artifacts.rs` tests → `snapshot/store/posix_tests.rs` - the OSS volume-backing test → `snapshot/resolver.rs` - The build-cache test stays in `repository` and now opens `PosixFsRepository` directly. - Not included: - `repository → observability`: one `MetricGuard` in the OSS client. - the other cycles from the review (`image → template/snapshot/sandbox`, `volume → snapshot`, `observability → orchestrator`). - `repository/backends/common::write_dense_overlaybd_layer_to_file`, which only snapshot uses but has no outward dependency. ## Design and behavior changes No behavior change. A new unit test, `repository::tests::repository_does_not_depend_on_its_users`, scans `src/repository/**/*.rs` and fails if any file references `crate::{snapshot,image,volume,sandbox}::`. I checked that it fails when such an import is added back. ## Compatibility and operations - Public API or generated protocol: Rust import paths of the `agentenv` lib change. Use `agentenv::snapshot::backends::{PosixFsBackend, PosixFsBackendConfig, OssBackend}` instead of `agentenv::repository::backends::…`, and `agentenv::snapshot::{SnapshotListFilter, SnapshotRuntimeResolver, SnapshotPublication}` instead of `agentenv::repository::…`. The workspace callers (`tests/common`, `crates/e2e-tests`) are updated. HTTP API: N/A. - Configuration or defaults: N/A. - Snapshot manifest, artifact layout, or storage format: unchanged. The three stored file names are now defined once in `repository/layout.rs`, with the same values. The existing `blob_paths_match_existing_repositories` test pins them. - Upgrade and rollback: N/A. - Host requirements, permissions, ports, or dependencies: N/A. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` (the agentenv lib suite was run; see below) - [ ] Relevant Rust integration tests (compiled by clippy `--all-targets`, not run) - [ ] `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 make fmt # ok make clippy # ok (workspace, all targets/features, -D warnings) cargo test -p agentenv --lib # 984 passed, 0 failed, 4 ignored # baseline on origin/main: 983 passed, 4 ignored (+1 new guard test) ``` Skipped checks and reasons: - `tests/` integration and `crates/e2e-tests` were compiled (clippy `--all-targets`) but not run. They need KVM, ublk or an object-storage service, and their only change is an import path. - The capability-runner (`--ignored`) tests and non-agentenv crates in `make test-unit` were not run: this PR does not touch them. - `services/` is unchanged. ## Risks and reviewer notes - Most of the diff is moved code. With `git diff -M`, `types/snapshot_id.rs` shows as a rename. The new `snapshot/backends/mod.rs` is `repository/backends/mod.rs` + `oss/mod.rs` + the bundle half of `posixfs/backend.rs`, unchanged except that the repository is built with `PosixFsRepository::new(root)` / `OssRepository::from_config`. - `snapshot/image_export/service.rs` used to assemble `PosixFsRepository` from its two internal stores. It now calls `PosixFsRepository::new(root)`, and those stores are private to `posixfs`. - `docs/src/internals/template-builder-testing.md` is updated for the moved resolver trait and tests. ## 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. (No new behavior; existing tests moved with the code. One new guard test.) - [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:22:54 +00:00