kvcache-ai/AgentENV

Fork 0

mirror of https://github.com/kvcache-ai/AgentENV.git synced 2026-11-05 01:17:36 +00:00

[PR #327] [MERGED] fix(snapshot): retain template workspace during startup recording #348

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/327
Author: @MisaKokoro
Created: 9/29/2026
Status: ✅ Merged
Merged: 9/30/2026
Merged by: @yingdi-shan

Base: main ← Head: fix/posix-startup-pack-workspace-lifetime

📝 Commits (1)

4476d6b fix(snapshot): retain template workspace during startup recording

📊 Changes

2 files changed (+59 additions, -9 deletions)

View changed files

📝 src/snapshot/startup_pack.rs (+2 -2)
📝 src/template/builder.rs (+57 -7)

📄 Description

What

Retain the template build workspace while detached startup-pack recording and
repository attachment are still running.

This change:

wraps TemplateBuildContext.workspace in an Arc<TempDir>;
creates template recordings through
StartupRecording::spawn_with_capture_lease();
gives both the recorder task and detached repository continuation a workspace
lease; and
adds a regression test that delays the trace write until the original
workspace owner has been dropped.

The StartupRecording.keep_alive documentation is also updated to describe
the template workspace lease.

Why

POSIX snapshot publication can finish before startup-pack recording.

Previously, template builds created the recording with a unit keep_alive:

StartupRecording {
    trace: tokio::spawn(record_startup_pack(...)),
    keep_alive: Box::new(()),
}

When TemplateBuilder::execute_and_publish() returned,
TemplateBuildContext.workspace was dropped and its temporary directory was
deleted. A recorder that was still running then failed while creating
memory-startup.trace.tmp:

startup pack: daemon recording failed
reason=write trace tmp file /tmp/snapshot-.../memory-startup.trace.tmp:
No such file or directory (os error 2)

The recording returned None, so the POSIX continuation did not create
memory-startup.pack or attach committed.memory_startup to the catalog
record.

Closes #326

Scope and non-goals

Included:

template-build startup-recording workspace lifetime;
recorder and detached-continuation lifetime handling;
a focused regression test.

Not included:

changes to startup-pack recording or prefetch algorithms;
changes to POSIX catalog or artifact formats;
changes to configuration or defaults;
making snapshot publication wait synchronously for recording;
unrelated template-builder refactoring.

Design and behavior changes

The template workspace is converted into an Arc<TempDir> after template
execution succeeds:

TemplateBuildContext.workspace
             |
             v
       Arc<TempDir>
        /         \
       v           v
recorder task   repository continuation
task lease      keep_alive lease

StartupRecording::spawn_with_capture_lease() clones the lease into the
recorder task and stores another lease in StartupRecording.keep_alive.

This preserves the existing asynchronous publication behavior:

snapshot publication can return without waiting for recording;
the recorder can continue writing into the build workspace;
the POSIX or OSS continuation can process and attach the resulting
manifest; and
the temporary workspace is removed after both leases are released.

If publication fails or its future is cancelled, the recorder task still owns
its lease until its cleanup path finishes.

Compatibility and operations

Public API or generated protocol: N/A; no API or protocol changes.
Configuration or defaults: N/A; existing configuration is unchanged.
Snapshot manifest, artifact layout, or storage format: N/A; existing
memory-startup.pack and committed.memory_startup formats are unchanged.
Upgrade and rollback: Code-only fix. Rolling back restores the previous
lifetime bug but does not require data migration.
Host requirements, permissions, ports, or dependencies: N/A; no changes.

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 \
  startup_recording_retains_template_workspace_until_continuation_finishes
  Result: 1 passed

cargo test -p agentenv --lib capture_lease
  Result: 4 passed

cargo test -p agentenv --lib template::builder::tests
  Result: 14 passed

cargo fmt --all -- --check
  Result: passed

cargo clippy -p agentenv --all-targets -- -D warnings
  Result: passed

git diff --check
  Result: passed

Skipped checks and reasons:

make test-unit: not run; focused unit and lifecycle tests were run instead.
KVM/ublk end-to-end aenv pull: not run as part of the automated validation.
Rust integration tests: not run; this change is covered by focused unit tests.
Services tests: not applicable because services/ is unchanged.
Code generation: not applicable because no generated source or schema changed.
Benchmarks: not applicable because this is a correctness fix.

Risks and reviewer notes

The main operational effect is that a template build's temporary workspace may
remain on disk until startup recording and manifest attachment finish, rather
than being deleted immediately when synchronous publication returns. This is
the intended lifetime and is bounded by the existing recorder and continuation
cleanup paths.

Publication remains non-blocking.

The most important review points are:

src/template/builder.rs: creation and propagation of the workspace lease;
src/template/builder.rs: regression test for delayed trace writes;
src/snapshot/startup_pack.rs: updated keep_alive ownership documentation.

The implementation reuses the existing capture-lease mechanism already used by
ordinary sandbox capture instead of introducing a second lifecycle mechanism.

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.

_{🔄 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/327 **Author:** [@MisaKokoro](https://github.com/MisaKokoro) **Created:** 9/29/2026 **Status:** ✅ Merged **Merged:** 9/30/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/posix-startup-pack-workspace-lifetime` --- ### 📝 Commits (1) - [`4476d6b`](https://github.com/kvcache-ai/AgentENV/commit/4476d6bd847f4a62f3d992bf35bd035669bb1972) fix(snapshot): retain template workspace during startup recording ### 📊 Changes **2 files changed** (+59 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/snapshot/startup_pack.rs` (+2 -2) 📝 `src/template/builder.rs` (+57 -7) </details> ### 📄 Description ## What Retain the template build workspace while detached startup-pack recording and repository attachment are still running. This change: - wraps `TemplateBuildContext.workspace` in an `Arc<TempDir>`; - creates template recordings through `StartupRecording::spawn_with_capture_lease()`; - gives both the recorder task and detached repository continuation a workspace lease; and - adds a regression test that delays the trace write until the original workspace owner has been dropped. The `StartupRecording.keep_alive` documentation is also updated to describe the template workspace lease. ## Why POSIX snapshot publication can finish before startup-pack recording. Previously, template builds created the recording with a unit `keep_alive`: ```rust StartupRecording { trace: tokio::spawn(record_startup_pack(...)), keep_alive: Box::new(()), } ``` When `TemplateBuilder::execute_and_publish()` returned, `TemplateBuildContext.workspace` was dropped and its temporary directory was deleted. A recorder that was still running then failed while creating `memory-startup.trace.tmp`: ```text startup pack: daemon recording failed reason=write trace tmp file /tmp/snapshot-.../memory-startup.trace.tmp: No such file or directory (os error 2) ``` The recording returned `None`, so the POSIX continuation did not create `memory-startup.pack` or attach `committed.memory_startup` to the catalog record. ## Related issue Closes #326 ## Scope and non-goals Included: - template-build startup-recording workspace lifetime; - recorder and detached-continuation lifetime handling; - a focused regression test. Not included: - changes to startup-pack recording or prefetch algorithms; - changes to POSIX catalog or artifact formats; - changes to configuration or defaults; - making snapshot publication wait synchronously for recording; - unrelated template-builder refactoring. ## Design and behavior changes The template workspace is converted into an `Arc<TempDir>` after template execution succeeds: ```text TemplateBuildContext.workspace | v Arc<TempDir> / \ v v recorder task repository continuation task lease keep_alive lease ``` `StartupRecording::spawn_with_capture_lease()` clones the lease into the recorder task and stores another lease in `StartupRecording.keep_alive`. This preserves the existing asynchronous publication behavior: 1. snapshot publication can return without waiting for recording; 2. the recorder can continue writing into the build workspace; 3. the POSIX or OSS continuation can process and attach the resulting manifest; and 4. the temporary workspace is removed after both leases are released. If publication fails or its future is cancelled, the recorder task still owns its lease until its cleanup path finishes. ## Compatibility and operations - Public API or generated protocol: N/A; no API or protocol changes. - Configuration or defaults: N/A; existing configuration is unchanged. - Snapshot manifest, artifact layout, or storage format: N/A; existing `memory-startup.pack` and `committed.memory_startup` formats are unchanged. - Upgrade and rollback: Code-only fix. Rolling back restores the previous lifetime bug but does not require data migration. - Host requirements, permissions, ports, or dependencies: N/A; no changes. ## 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 - [ ] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text cargo test -p agentenv --lib \ startup_recording_retains_template_workspace_until_continuation_finishes Result: 1 passed cargo test -p agentenv --lib capture_lease Result: 4 passed cargo test -p agentenv --lib template::builder::tests Result: 14 passed cargo fmt --all -- --check Result: passed cargo clippy -p agentenv --all-targets -- -D warnings Result: passed git diff --check Result: passed ``` Skipped checks and reasons: - `make test-unit`: not run; focused unit and lifecycle tests were run instead. - KVM/ublk end-to-end `aenv pull`: not run as part of the automated validation. - Rust integration tests: not run; this change is covered by focused unit tests. - Services tests: not applicable because `services/` is unchanged. - Code generation: not applicable because no generated source or schema changed. - Benchmarks: not applicable because this is a correctness fix. ## Risks and reviewer notes The main operational effect is that a template build's temporary workspace may remain on disk until startup recording and manifest attachment finish, rather than being deleted immediately when synchronous publication returns. This is the intended lifetime and is bounded by the existing recorder and continuation cleanup paths. Publication remains non-blocking. The most important review points are: - `src/template/builder.rs`: creation and propagation of the workspace lease; - `src/template/builder.rs`: regression test for delayed trace writes; - `src/snapshot/startup_pack.rs`: updated `keep_alive` ownership documentation. The implementation reuses the existing capture-lease mechanism already used by ordinary sandbox capture instead of introducing a second lifecycle mechanism. ## 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. --- <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:30 +00:00