[PR #367] [MERGED] refactor(repository): store build logs and cache state as shared documents #379

Closed
opened 2026-10-29 14:22:48 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: refactor/repository-documents


📝 Commits (1)

  • 5bafb6c refactor(repository): store build logs and cache state as shared documents

📊 Changes

13 files changed (+337 additions, -296 deletions)

View changed files

📝 src/api/impls/image_build/cache.rs (+1 -0)
📝 src/api/impls/image_build/tests.rs (+1 -1)
📝 src/api/impls/template.rs (+2 -1)
📝 src/snapshot/repository/backends/oss/layout.rs (+0 -4)
📝 src/snapshot/repository/backends/oss/repository.rs (+47 -87)
📝 src/snapshot/repository/backends/posixfs/backend.rs (+14 -36)
📝 src/snapshot/repository/backends/posixfs/catalog.rs (+35 -124)
📝 src/snapshot/repository/backends/posixfs/layout.rs (+0 -6)
📝 src/snapshot/repository/build_cache.rs (+118 -3)
📝 src/snapshot/repository/interfaces.rs (+20 -31)
➕ src/snapshot/repository/layout.rs (+53 -0)
📝 src/snapshot/repository/mod.rs (+5 -2)
📝 src/template/logs.rs (+41 -1)

📄 Description

What

Replace the repository's build-log and build-cache trait methods with three generic document operations: get_document, put_document, and a compare-and-swap update_document. Each backend implements them once. A new DocumentKey enum in src/snapshot/repository/layout.rs is the single place that names these paths.

Why

Each small record the repository stores has needed its own trait method, plus layout, wrapper, and implementation code in both backends. That cost is about 6 files per record type. In #321, about a quarter of the handwritten files existed only to store one JSON file per build, and #356 repeats the pattern for webhook config. After this change, a new record type is one DocumentKey variant plus the feature's own code.

N/A. This came out of a review of why recent feature PRs (#321, #338, #341, #347, #356) each touched 20–40 files.

Scope and non-goals

  • Moves only the build logs and the build-cache state.
  • Snapshot records, aliases, and volumes are untouched here; volumes follow in a later PR.
  • Typed access moves next to its types as the BuildLogStore (template/logs.rs) and BuildCacheStore (repository/build_cache.rs) extension traits, so call sites only gain an import.

Design and behavior changes

  • update_document runs the caller's update with an ETag compare-and-swap on OSS and under a file lock on POSIX. The update may run more than once and returns the document it was applied to.
  • DocumentKey keeps the existing paths: snapshots/build-logs/{id}.json and template-build/cache-head.json.
  • The POSIX build-cache lock stays at volumes/aliases/aenv-buildkit-cache-head.lock, so mixed-version nodes on a shared filesystem still exclude each other.

Compatibility and operations

  • Public API or generated protocol: none.
  • Configuration or defaults: none.
  • Snapshot manifest, artifact layout, or storage format: same paths. POSIX now writes these two documents as compact JSON instead of pretty-printed; both read back the same.
  • Upgrade and rollback: compatible both ways, since paths and lock files are unchanged.
  • Host requirements, permissions, ports, or dependencies: none.

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                                   # 963 passed, 4 ignored
cargo test -p agentenv --lib -- snapshot::repository template::logs image_build
                                                               # 110 passed (109 before)

New tests: one pins every document path and the POSIX lock path. The POSIX build-cache concurrency test (16 repository instances replacing the head at once) now runs through the public BuildCacheStore methods.

Skipped checks and reasons:

  • make test-unit capability tests (--ignored) were not run; the change does not touch capability-dependent code.
  • The OSS compare-and-swap path has no unit test, before or after this change. A later PR in this series adds a MinIO test for it, which also found a bug on main in the OSS create-if-absent call.

Risks and reviewer notes

  • Most important files: src/snapshot/repository/layout.rs and interfaces.rs, then the two backends' update_document.
  • First of three stacked PRs: documents (this one), then moving the repository to src/repository/, then the volume catalog.

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/367 **Author:** [@yingdi-shan](https://github.com/yingdi-shan) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `refactor/repository-documents` --- ### 📝 Commits (1) - [`5bafb6c`](https://github.com/kvcache-ai/AgentENV/commit/5bafb6c90c22afb79bb97a1a3e5c4b8de33be526) refactor(repository): store build logs and cache state as shared documents ### 📊 Changes **13 files changed** (+337 additions, -296 deletions) <details> <summary>View changed files</summary> 📝 `src/api/impls/image_build/cache.rs` (+1 -0) 📝 `src/api/impls/image_build/tests.rs` (+1 -1) 📝 `src/api/impls/template.rs` (+2 -1) 📝 `src/snapshot/repository/backends/oss/layout.rs` (+0 -4) 📝 `src/snapshot/repository/backends/oss/repository.rs` (+47 -87) 📝 `src/snapshot/repository/backends/posixfs/backend.rs` (+14 -36) 📝 `src/snapshot/repository/backends/posixfs/catalog.rs` (+35 -124) 📝 `src/snapshot/repository/backends/posixfs/layout.rs` (+0 -6) 📝 `src/snapshot/repository/build_cache.rs` (+118 -3) 📝 `src/snapshot/repository/interfaces.rs` (+20 -31) ➕ `src/snapshot/repository/layout.rs` (+53 -0) 📝 `src/snapshot/repository/mod.rs` (+5 -2) 📝 `src/template/logs.rs` (+41 -1) </details> ### 📄 Description ## What Replace the repository's build-log and build-cache trait methods with three generic document operations: `get_document`, `put_document`, and a compare-and-swap `update_document`. Each backend implements them once. A new `DocumentKey` enum in `src/snapshot/repository/layout.rs` is the single place that names these paths. ## Why Each small record the repository stores has needed its own trait method, plus layout, wrapper, and implementation code in both backends. That cost is about 6 files per record type. In #321, about a quarter of the handwritten files existed only to store one JSON file per build, and #356 repeats the pattern for webhook config. After this change, a new record type is one `DocumentKey` variant plus the feature's own code. ## Related issue N/A. This came out of a review of why recent feature PRs (#321, #338, #341, #347, #356) each touched 20–40 files. ## Scope and non-goals - Moves only the build logs and the build-cache state. - Snapshot records, aliases, and volumes are untouched here; volumes follow in a later PR. - Typed access moves next to its types as the `BuildLogStore` (`template/logs.rs`) and `BuildCacheStore` (`repository/build_cache.rs`) extension traits, so call sites only gain an import. ## Design and behavior changes - `update_document` runs the caller's update with an ETag compare-and-swap on OSS and under a file lock on POSIX. The update may run more than once and returns the document it was applied to. - `DocumentKey` keeps the existing paths: `snapshots/build-logs/{id}.json` and `template-build/cache-head.json`. - The POSIX build-cache lock stays at `volumes/aliases/aenv-buildkit-cache-head.lock`, so mixed-version nodes on a shared filesystem still exclude each other. ## Compatibility and operations - Public API or generated protocol: none. - Configuration or defaults: none. - Snapshot manifest, artifact layout, or storage format: same paths. POSIX now writes these two documents as compact JSON instead of pretty-printed; both read back the same. - Upgrade and rollback: compatible both ways, since paths and lock files are unchanged. - Host requirements, permissions, ports, or dependencies: none. ## Validation - [x] `make fmt` - [x] `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: ```text cargo test -p agentenv --lib # 963 passed, 4 ignored cargo test -p agentenv --lib -- snapshot::repository template::logs image_build # 110 passed (109 before) ``` New tests: one pins every document path and the POSIX lock path. The POSIX build-cache concurrency test (16 repository instances replacing the head at once) now runs through the public `BuildCacheStore` methods. Skipped checks and reasons: - `make test-unit` capability tests (`--ignored`) were not run; the change does not touch capability-dependent code. - The OSS compare-and-swap path has no unit test, before or after this change. A later PR in this series adds a MinIO test for it, which also found a bug on `main` in the OSS create-if-absent call. ## Risks and reviewer notes - Most important files: `src/snapshot/repository/layout.rs` and `interfaces.rs`, then the two backends' `update_document`. - First of three stacked PRs: documents (this one), then moving the repository to `src/repository/`, then the volume catalog. ## 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:22:48 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
kvcache-ai/AgentENV#379
No description provided.