[the-gigi-pplx]

Summary

Template-manager now gives each template build one owner at a time. A pull, an import, a registration, background rootfs compression, DeleteTemplateBuild and startup recovery's per-directory check all take the same per-digest claim before they write or remove the build's directory or catalog row, and DeleteTemplate claims the whole template. None of them runs alongside another on the same build. A delete that finds a build busy refuses with a retryable ABORTED instead of removing files under a live writer.

This is part 1 of a six-PR stack, described under Stack below. Like every part before the migration, it keeps template-manager's current build key, (name, content_hash); the execution environment joins that key in part 6.

Terms

• Digest: one template build, identified by (name, content_hash). Its artifacts live in the build directory {template root}/{name}/{content_hash} and its catalog row in template_builds.
• Pull in-flight map: VolumePuller::in_flight, which coalesces concurrent GetTemplate pulls of one digest onto a single materialization task.
• Claim: a DigestClaim, an entry in that map held by something other than a pull, for one digest or for every digest of a template. It's released when dropped. A pull that arrives while a claim is held waits for it to go and then runs as the owner.

The problem

Only pulls coordinated with each other. Every other writer of a build directory or row ran unsynchronized with pulls and with each other:

1. Registration racing a delete. register_template checks whether the build directory exists, renames its staging directory into place, then upserts the row. DeleteTemplateBuild removes the artifacts, then the rows. A delete that lands between the rename and the upsert leaves a ready row over a directory that's gone, and the registration reports success. A delete that lands between the existence check and the metadata refresh deletes the build the registration is about to report as registered.
2. Import racing a delete or a pull. ImportTemplate held a key in importing_templates, which keeps one import of a digest from overlapping another, but neither deletes nor pulls look at that set.
3. Template deletes and compression. DeleteTemplate removes every build directory under the template and then all its rows, with no coordination at all. After a registration or import, a background task compresses the rootfs, rewrites manifest.json and upserts the row. A DeleteTemplateBuild that runs while it's working leaves it to write a row for a build that's gone.
4. Startup recovery racing live requests. spawn_startup_recovery runs recovery after the socket binds, so requests are already flowing. A pull writes manifest.json into the final build directory before the rootfs. Recovery's per-directory pass can read that manifest, find the rootfs missing, and remove the directory as invalid under the pull.

Each of these loses a build or leaves a row that names artifacts that aren't there. Sandboxes then fail to create from a build the catalog says is ready.

How this fixes it

• Make the pull in-flight map the digest's one ownership primitive. VolumePuller gains try_claim_digest (claim, or None while held), claim_digest (claim, waiting for the current holder), and the DigestClaim guard. A claim is an ordinary in-flight entry, so a pull that starts while one is held joins it, sees it end without a result, and retries as the owner. That path already existed for a pull whose owner was cancelled.
• Registration claims after hashing. register_template takes claim_digest once it knows the content hash and holds it through the existence check, the publish rename and the row upsert, on both the fresh and the existing-digest paths (ExistingDigestRegistration carries the claim). It lets go before the base-snapshot reconcile, which can take minutes.
• Imports claim while they write. import_template takes claim_digest after its import-set key and holds it through do_import, releasing it before the snapshot reconcile. It waits rather than refusing: node-api's PullTemplate maps any import error to INTERNAL, so a retryable refusal would reach its caller as a hard failure.
• DeleteTemplateBuild claims first and refuses when busy. claim_digest_for_delete uses try_claim_digest and returns ABORTED ("a pull, import or registration … is in flight; retry the delete") while anything holds the digest. The claim lasts until the delete returns. Pulls, imports and registrations of the digest wait for a running delete.
• DeleteTemplate claims the whole template. try_claim_template adds a template-wide entry (a PullKey with no content hash). It's refused while any digest of the template is held, and while it's held every pull and claim of the template's digests waits, including digests that weren't on disk when the delete started. The delete refuses with ABORTED when the template is busy.
• Compression owns the digest while it runs. The compression task takes claim_digest before it reads anything, so it waits for the registration or import that spawned it to let go. A delete that runs in between leaves no rootfs, and the task fails without writing a manifest or row.
• A cancelled caller can't release a claim early. RegisterTemplate, ImportTemplate, DeleteTemplate and DeleteTemplateBuild run as their own tasks, as DeleteIdleTemplate already does. Their claims are locals of the handler, and the disk and DB work they guard runs in spawn_blocking, which outlives a dropped handler. Without this, a caller that cancels or times out would release the claim while that work kept going.
• Startup recovery skips a held digest. The per-directory pass takes try_claim_digest for each build and skips one someone else holds. A complete directory a holder leaves without a row is checked at the next startup, or recovered sooner by a pull when the build's release is published. The pass now reads the service's template root rather than the global constant. In production they're the same path, and the change lets the test drive recovery against a temporary root.

The import set stays, for import-versus-import exclusion. Eviction keeps coordinating through the eviction gate and the row's deleting state, and the pull path itself is unchanged. The DeleteTemplate and DeleteTemplateBuild RPC comments in manager.proto document the new refusals.

Stack

The stack lets one node cache two builds of the same rootfs that are bound to different execution environments (twins), for example a template's default-environment build and its twin bound to a newer runtime. Each twin needs its own build directory, catalog row, base snapshots and tag pins. Today template-manager keys all of those by (name, content_hash), so twins collide. Part 6 adds a cache partition, derived from the build's execution environment, to that key, with a migration. Parts 1 to 4 harden the single-partition code first, so the migration lands on code whose ownership and recovery rules are already settled. Each part builds, passes its tests and is safe to deploy on its own, and each PR targets the one below it.

Part	PR	What it does
1	This PR	One owner per template build at a time: the per-digest claim.
2	#1572	Startup recovery keeps a build it can't verify right now instead of deleting it, and a pull refuses a build directory that has no catalog row.
3	#1573	The template-cache GC tick verifies build directories that have no row, and DeleteTemplateBuild can clear one.
4	Not open yet	Sandbox-manager takes rootfs delta bases only from builds template-manager's catalog has verified.
5	Not open yet	Plumbing for cache partitions outside template-manager storage, inert until part 6.
6	#1075	The migration, and template-manager keyed by (name, content_hash, cache partition).

Release identity outside the node isn't part of the stack. The control plane's release catalog, its lookups, its object-store layout and the node inventory stay keyed by (name, content_hash), so a twin of a published release can't be registered or published until a separate control-plane change lets them tell twins apart.

Validation

New tests, each mutation-checked: I broke the mechanism it covers, saw the test fail, and restored the code.

• register_race::a_registration_racing_a_delete_never_succeeds_over_or_loses_a_build drives both orders. With the delete holding the digest, the registration publishes nothing until the delete lets go, then lands its build and row. With the registration parked after its publish rename and before its row, the delete refuses with ABORTED without waiting, and the build and row survive. It fails when the registration's claim is removed, and when the delete's is. The registration-first half carries the proof: the delete-first half waits a fixed 300 ms, which a slow hash can outlast before the registration reaches its claim.

• import_rpc::import_waits_for_a_delete_that_holds_its_digest and import_rpc::an_import_in_flight_holds_its_digest_against_a_delete cover both orders for imports over the real Unix-socket gRPC server. Both fail when the import's claim is removed.

• startup_recovery::tests::delete_template_build_waits_for_a_pull_of_the_digest: while a pull holds the digest, DeleteTemplateBuild returns ABORTED and the directory and the row the pull writes survive. Once the pull lets go, the delete removes both. It fails when the delete's claim is removed.

• startup_recovery::tests::startup_recovery_skips_a_build_someone_else_claims: recovery leaves a held, rowless build alone and registers it on the next pass once the claim goes. It fails when recovery's claim is removed.

• register_race::a_cancelled_registration_keeps_its_claim_until_its_row_is_written: a registration whose caller is aborted between its publish and its row keeps the claim, so a delete is refused, then writes the row and lets go. It fails when the registration RPC runs in the caller's task again.

• volume::tests::a_pull_waits_for_a_digest_claim_then_runs: a pull that arrives while a claim is held waits, then runs as the owner. Making a claim not enter the map fails all six tests above.

• volume::tests::a_template_claim_holds_every_digest_of_the_template: a template claim is refused while one of its digests is held; while it's held, a claim of any of its digests, even an unclaimed one, is refused or waits, and other templates are unaffected. It fails when a digest lookup ignores the template entry, and when a template claim ignores its digests.

• digest_claim::delete_template_refuses_while_one_of_its_builds_is_claimed: DeleteTemplate returns ABORTED and leaves the build on disk. It fails when the template claim is removed, and when a template claim ignores its digests.

• digest_claim::compression_waits_for_the_digest_and_never_restores_a_deleted_build: compression spawned under a held claim writes nothing until the claim goes. A build deleted meanwhile gets no row back, and an untouched one is compressed and recorded. It fails when the compressor's claim is removed.

• cargo nextest run -p pplx-space-template-mgr: 326 passed.

• cargo clippy --workspace --all-targets --locked -- -D warnings: clean.

• Full workspace run (ci/scripts/run-rust-checks.sh tests-unprivileged): 12,343 passed, 76 skipped. Its doctests, feature-isolation and SDK checks passed too.

• CI on this head: every check passes, including the 12-shard Kind Firecracker smoke and the privileged test lane.

Not validated: a deployed run.

Not covered

• Startup recovery's transient-directory cleanup (import and republish staging, .replaced restores) and its pass that prunes rows with missing artifacts don't take the claim, and still run after the socket binds. The next PR in the stack moves the cleanup ahead of serving.
• A registration's source and staging directories sit under the template's directory, so a DeleteTemplate that runs while a registration of the same template is hashing or waiting for its claim removes them, and the registration fails instead of waiting.
• Eviction keeps its own coordination (the eviction gate and the row's deleting state) rather than the claim.

Dependencies

None. This merges on its own: it's the bottom of the stack described under Stack, and the PRs above it build on its claim.

Severity and risk

Severity: medium. Each race loses a template build or leaves a ready row over missing artifacts on one node, and creates from that build then fail there until the build is pulled again or re-registered.

Risk: low. A DeleteTemplateBuild or DeleteTemplate that used to run alongside a pull, import, registration or compression now returns ABORTED. Compression holds its build's claim from the moment it's spawned, including while it queues behind other compressions for the node's single compression slot, so a delete of a build that was just registered or imported can be refused until it finishes, and an import or re-registration of the same digest waits for it. Build-manager's prune of old local builds right after a new registration can hit that refusal while an older build's compression is still queued; it logs a warning, and a later build's prune removes the old build. A caller whose deadline expires sees DEADLINE_EXCEEDED while its delete keeps running, and its immediate retry is refused with ABORTED until that first attempt finishes. Build-manager's prune of old local builds logs a warning and moves on, and the admin node-template delete surfaces the error to the operator, who retries. An import or registration can wait for a delete of the same digest, which is bounded by the delete itself. Startup recovery can skip a busy build on its one pass. A caller that cancels a registration, import or delete no longer stops it; it runs to completion. Roll back by reverting this commit; there's no schema or wire-format change.

AI assistance: Claude Code ported this change from the larger PR above, adapted it to the current single-partition schema, and drafted the tests and this description, under my review.

[coderabbitai]

Automatic approval withheld

Automatic approval is disabled by the configured approval policy for this pull request. Review comments and requests for changes continue normally.

Commit: 6324f02fbb42.

No actionable comments were generated in the recent review.

Walkthrough

The change adds shared digest and whole-template claims to coordinate pulls, registration, imports, compression, deletion, and startup recovery. Operations that cannot acquire a required claim wait or return Aborted, depending on the operation. RPC handlers run delegated work in spawned tasks and map task-join failures to internal errors. New tests cover claim conflicts, operation ordering, cancellation, and recovery.

Possibly related PRs

• perplexityai/SPACE#1075: Adds digest-claim coordination for partitioned build paths, while this PR applies related coordination to the legacy build paths.

Merge Risk: Moderate · up to 6324f

Build operations now wait, skip work, or return retryable errors when claims conflict. The coordination has test coverage, but its effect on build operations warrants acceptance of moderate merge risk.

Security architecture risk: Low · up to 6324f

The change reduces races between concurrent template operations and preserves existing authorization checks. Accepted operations can now continue after callers disconnect, so execution lifetime and recovery behavior remain important operational considerations.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly evidenced mutation scope is the serving node's template artifacts and catalog. Digest ownership isolates conflicting work for one name/hash, while whole-template deletion excludes all digests of that name. Resource contention can affect the broader serving process; downstream tenant reachability and environment-wide exposure were not established.

Trust Boundaries and Controls

• observed — The four changed mutating RPC adapters move the original request and cloned service into a spawned task, preserving consumer authorization rather than introducing a new principal or endpoint. The changed boundary is execution ownership: caller cancellation no longer cancels the service operation. The server still configures request timeout and rate limiting, but those settings alone do not establish detached-task lifetime bounds.

Resilience and Maintainability Implications

• observed — Claim release marks terminal state under a mutex, removes only the matching ownership entry, and notifies waiters. Waiting enables notification while checking terminal state under the same mutex. These controls support safe ownership handoff; they do not make filesystem publication and catalog updates one atomic transaction, so partial-failure recovery remains necessary.

Hardening Proposals

• proposed — Consider task-owned admission limits and explicit tracking of cancellation-surviving mutations through shutdown. Such controls should preserve ownership until blocking work is quiescent rather than aborting a task and releasing its claim prematurely. This is a hardening proposal, not a verified denial-of-service finding.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title meets the format requirements and describes the main change.
Description Follows The Pr Template		The description includes all four required items with content: Summary, Validation, Severity and risk, and AI assistance.
Bug Fixes Name A Regression Test		The Validation section names startup_recovery::tests::delete_template_build_waits_for_a_pull_of_the_digest and states that it fails when the delete claim is removed. The PR adds that test and the cl…
Weakened Safeguards Are Justified		The diff does not remove or weaken tests, assertions, lints, CI checks, or repository guards. Existing test changes preserve their assertions.
Public-Safe Content		The title, description, and reviewed diff contain no identified content that matches the stated failure conditions. The added hashes and paths are test fixtures or relative paths.

Full details: Bug Fixes Name A Regression Test

Explanation

The Validation section names startup_recovery::tests::delete_template_build_waits_for_a_pull_of_the_digest and states that it fails when the delete claim is removed. The PR adds that test and the claim it checks.

Recent review info

Automatic approval policy

Commit: 6324f02fbb42.

Setting	Effective value	Source
Automatic approvals	Disabled	Override 4
Merge risk	Low	Override 2
Security risk	Not set	Global configuration

Matching overrides (numbered as in the resolved configuration):

• Override 2 — any author; paths: **, !ui/**; merge risk: Low.
• Override 4 — any author; paths: .coderabbit.yaml, .coderabbit.yml, coderabbit.yaml, coderabbit.yml, .coderabbit.config.ts, **/AGENTS.md, **/CLAUDE.md, .agents/**, .claude/**, .claude-plugin/**, CONTRIBUTING.md, docs/versioning.md, docs/rust/space-conventions.md, docs/rust/space-review-lessons.md, .github/**, ci/**, !ci/scripts/setup-nextest-*.sh, scripts/source_guards/**, scripts/check*, clippy.toml, deny.toml, .gitleaks.toml, .pre-commit-config.yaml, lefthook.yaml, rust-toolchain.toml, .cargo/**, supply-chain/**, pyproject.toml, uv.lock, **/package.json, **/package-lock.json, **/go.mod, **/go.sum, **/requirements*.txt, **/requirements*.in, proto/**, db/**, crates/l40_api/api/src/auth.rs, crates/l40_api/api/src/auth/**, crates/l40_api/api/src/middleware/auth*, crates/l40_api/api/src/middleware/auth/**, crates/l40_api/api/src/routes/authz.rs, crates/l10_foundation/bearer/**, crates/l10_foundation/rustls/**, crates/l10_foundation/guest/**, crates/l10_foundation/pii/**, shim/**, docker/**, !docker/Dockerfile.rust-build-kind, templates/**, infra/space/aws/**, infra/space/cluster/**, infra/space/identity/**, infra/space/observability/**, infra/space/packer/**, infra/space/kind/charts/**, LICENSE, SECURITY.md; automatic approvals: disabled.

Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

• Source mode: Auto
• Repository: perplexityai/SPACE
• Branch: gigi/pla-6921-reland-template-cache-partitions-1
• File: .coderabbit.yaml
• Commit: 6324f02
• Layers: Repository YAML; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Enterprise
• Run ID: d43cb996-6c7e-45a4-b08a-f9963f066486

Commits

Reviewing files that changed from the base of the PR and between 956d917 and 6324f02.

Files selected for processing: 14

• crates/l40_api/template/mgr/server/src/service.rs
• crates/l40_api/template/mgr/server/src/service/adapter.rs
• crates/l40_api/template/mgr/server/src/service/lookup.rs
• crates/l40_api/template/mgr/server/src/service/startup_recovery.rs
• crates/l40_api/template/mgr/server/src/service/startup_recovery/tests.rs
• crates/l40_api/template/mgr/server/src/service/templates.rs
• crates/l40_api/template/mgr/server/src/service/tests.rs
• crates/l40_api/template/mgr/server/src/service/tests/digest_claim.rs
• crates/l40_api/template/mgr/server/src/service/tests/import_rpc.rs
• crates/l40_api/template/mgr/server/src/service/tests/register_race.rs
• crates/l40_api/template/mgr/server/src/service/transfer.rs
• crates/l40_api/template/mgr/server/src/volume.rs
• crates/l40_api/template/mgr/server/src/volume/tests.rs
• proto/template/manager/v1/manager.proto

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.

[nikhilbirmiwal-pplx]

Reviewed head 6324f02fbb42ec7303b3c24004a8df5e39d978ce. No merge-blocking findings.

This is a useful, independently mergeable concurrency fix:

• Reuses the pull-flight map to give pulls, imports, registration, compression, and explicit build/template deletes one owner per (name, content_hash).
• Adds an atomic whole-template claim so deleting a template cannot miss a concurrently claimed or newly introduced digest.
• Deletes return retryable ABORTED rather than deleting a busy build; writers wait and retry after the current owner releases its claim.
• Keeps mutating RPC work alive across caller cancellation, so a detached blocking disk/DB write cannot outlive the ownership guard.
• Releases registration/import ownership before base-snapshot reconciliation, avoiding a long or reentrant hold.
• Adds behavioral coverage for both registration/delete orderings, import ownership, cancellation, whole-template claims, and compression after deletion.

Validation: all 313 pplx-space-template-mgr tests passed locally, with --run-ignored all, SPACE_CI_REQUIRE_TEST_DEPS=1, the repository's disposable btrfs fixture, and Postgres setup enabled. No tests were skipped. GitHub checks show 37 successful checks, no failed or pending checks, and three skipped checks.

Scope: this PR does not migrate SQLite tables, repartition the cache, or change the on-disk naming scheme. This approval applies to #1571 only, not to the later cache-layout migration in the stack.