[zibi-pplx]

Same-generation pod replacement must not inherit an earlier worker identity or return its stale cached endpoint. Fence execution and pool observations with materialization tokens, Pod UIDs, and recorded Node UIDs. Preserve main's materialization history and repair missing inventory outside sandbox locks before revalidation.

This revision addresses the prior cleanup and readiness feedback: exact-created-pod cleanup uses its UID despite scheduler resource-version changes; obsolete-observation cleanup still requires UID/resourceVersion ownership. Rejected metadata fences cannot return stale IPs. Pool readiness rechecks Node UID, and compatibility adoption leaves execution identity unknown. Token construction is shared with the placement writer and is never accidental through Default.

Deploy token-aware compatibility writers before enabling fenced mode. Once tokens are issued, rollback requires a token-aware compatibility binary. Depends on the schema prerequisite.

Validation: stale endpoint and cleanup regressions failed before correction and passed afterward; all 56 warm-pool tests passed, including Node UID replacement during readiness. The Kubernetes substrate run passed 479 tests, with the changed history assertion corrected and its four-test suite passing; four native failover tests require Linux loopback behavior unavailable on macOS. Workspace all-target/all-feature Clippy and affected aarch64 Linux musl compilation/lint pass. Public diff review and Gitleaks passed.

Stack (land in order): #318 schema → #319 materialization ownership → #345 backup caller identity → #346 upload evidence → #283 retained-artifact recovery.

Rebuilt against d1abc66fe48bccaaa715e09593f875f7c6c4d11d. The schema is separate because current CI requires migration-only PRs. All existing PRs are reused.

Scope boundary: no file overlap with #828 (2a4988b6), #829 (1b696768), or #837 (58ceff04). Both compositions—#828+#829 and #837+#829—passed aarch64 Linux musl Clippy and all 98 selected snapshot/teardown tests each. These are component tests, not live Firecracker recovery evidence. Capture, encrypted sealing, teardown handoff, and source cleanup remain with those workstreams. Linux/KVM held-volume recovery, manager-restart/handoff failure, and live restore of the same artifact have not been validated here; issuance must remain disabled until that deployment evidence exists.

AI assistance: Codex contributed implementation, tests, and this description.

[zibi-pplx]

Implemented the reviewed writer fixes. Warm-pool UID lookup blocks only promotion, so timeout cleanup still runs. Each authorized materialization has a token reused across its create retries; create responses, adoption, re-drive, and observers must match it before recording pod identity. Recreation clears both node fields, and cleanup is fenced by the expected materialization and observed pod UID. Legacy unknown identity is not backfilled.

Placement releases sandbox locks before an independent inventory repair, then revalidates identity under shared node locks. The regression test forces the terminator/placement contention and verifies the sandbox lock is released. Pool claims preserve the incarnation captured at readiness.

Validation: all 400 Kubernetes lifecycle cases passed across the final suite and a focused retry of one PostgreSQL connection-reset fixture. Workspace lint, doctests, feature isolation, SDK conformance, and migration isolation passed. The workspace gateway TLS timeout reproduces on the untouched base. This remains draft; no deployment was performed.

[shrikant-pplx]

Reviewed together with the rest of stack #345 → #318 → #319 → #346, in two passes (correctness, then security). No P0/P1 findings. Two P2s and two P3s are inline. The token fencing holds up for observation, adoption, the claim CAS, obsolete cleanup and hydration ordering.

One more P3, in a file this PR doesn't touch:

• P3 [func] The placement-machine execution insert sets no token — crates/libs/placement/machine/src/postgres.rs:204 inserts into sandbox_k8s_executions without materialization_id. It has no production caller today. Once the wake cutover routes through it, though, every generation it creates stays token-less, even in fenced mode. Before that cutover, bind initial_token() or route the insert through insert_active_execution_claim_in.

1. Fixes the stated problem and root cause? Partly. Fenced rows are fenced correctly. But compatibility mode isn't safe for pool-bound rows, and cleanup of pods this request created regressed (both inline P2s).
2. Covers every affected caller and path? Partly. Every POST, metadata and placement path is updated, and the compiler enforces that. Mode gating on the pool bind and the placement-machine writer were missed.
3. Every change necessary, no scope creep? Yes. Everything serves the token/incarnation feature. The only extras are insta metadata churn and test-only wrappers.
4. Do the tests reproduce the failure and prove the fix and edge cases? Partly. The store, adoption, cache-miss, hydration and obsolete-cleanup tests would fail without the fix. Gaps:
   • Mock POST responses carry no resourceVersion, so the 409 on created-pod cleanup is never exercised.
   • No test changes the node UID during the readiness probe.
   • No test covers an old writer during a mixed-version rollout.
5. Regression? Yes. Two regressions:
   • A created pod can leak when its cleanup DELETE hits a resourceVersion conflict.
   • A pool-bound sandbox that an old replica re-materializes during rollout gets persistent 503s.

Checked and found no problem:

• Tokens never go from Some to None within a generation.
• Lock order matches the terminator: sandbox, then execution, then FOR SHARE on nodes/node_k8s, with a rollback before the inventory upsert.
• Every token/UID predicate is NULL-safe.
• The GET on a reflector miss prevents rotating the token over a live pod.
• A 409 with a matching token adopts the existing pod; a foreign token goes through the revalidated obsolete delete.
• The snapshot diffs are only the new annotation plus insta metadata.

[shrikant-pplx]

P2 [func] Cleanup of a pod this request created can 409 on a stale resourceVersion and leak the pod

delete_created_materialization_pod is called with the POST response (ensure_pod.rs:1160, redrive.rs:712 and redrive.rs:723). It goes through delete_observed_pod, which uses that response's resourceVersion as a precondition. Scheduler binding and the first kubelet status write bump the RV shortly after creation.

The failure sequence:

1. Persist returns Ok(None) because ownership was revoked, for example by a teardown racing a slow create.
2. The DELETE returns 409, which maps to Unavailable("re-observe before retrying").
3. No caller re-observes. The execution row is already retired or replaced, so nothing references the pod again.

I found no GC that reaps it. The orphaned-sandbox GC works from DB rows, and the restore-pod and ContainerCreating GCs only cover those pod states. Before this PR, these paths deleted by name. The tests miss this because the mock POST responses carry no resourceVersion.

Fix:

• Use a UID-only precondition for cleanup of pods this request created; this function's doc already names the UID as the authority. That matches the persist-error path at ensure_pod.rs:1191 (Some((pod_uid, None))).
• Keep the RV precondition for the obsolete-token branch of delete_execution_pod.
• Add a test where the POST response's RV is older than the live pod's.

[shrikant-pplx]

P2 [func] Compatibility mode still records k8s_node_uid on pool binds; a mixed-version rollout can then wedge the execution

This insert writes the slot's node UID on a token-less row without checking MaterializationMode. In the default compatibility rollout:

1. A new replica binds a pool slot, creating the row (token NULL, name N, uid A).
2. The pod dies, and an old replica re-materializes the same generation. Its claim clears only pod_uid/pod_ip, and its placement update sets k8s_node_name whenever the name differs.
3. The row becomes (token NULL, name M, uid A), with M on a different node incarnation.

From then on, on every new replica:

• try_persist_observed_pod_node_placement hits uid != node_uid and returns Unavailable("observed pod node differs…").
• update_active_execution_node_name can't repair the row, because its UPDATE only touches rows where k8s_node_uid IS NULL.

So access/start/wake on that sandbox return 503 until its pod dies. That contradicts this PR's own rule in update_active_execution_node_name: old writers may move a name only when "neither token nor incarnation was ever recorded".

Fix:

• Bind k8s_node_uid here only when the store is in Fenced mode, and NULL otherwise.
• Add a Postgres test where an old writer changes the name on a token-less row that has a UID.

[shrikant-pplx]

P3 [check] A NULL node_k8s.k8s_node_uid now fails placement instead of repairing inventory

k8s_node_uid is nullable. The internal-api node bootstrap inserts node_k8s with k8s_node_uid = NULL until node sync fills it in. Decoding that row into node_uid: String returns a database error, so the repair branch in persist_observed_pod_node_placement never runs. Before this PR, the placement transaction upserted the UID itself. This fails closed and only lasts until node sync writes the UID, but placement errors during that window.

Fix: make it node_uid: Option<String>, and treat None like a missing row (roll back and take the existing repair path).

[shrikant-pplx]

P3 [test] The "node changed during the readiness probe" branch is untested

warming_slots_without_node_identity_still_timeout only covers a missing identity. No test changes the cached node UID between the snapshot and this re-check.

Fix: in observe_warming_slots_retries_not_started_probe_and_promotes_slot, apply a node event with a new UID between probe attempts. Assert that no slot is marked ready and that timeout cleanup still runs.

[shrikant-pplx]

P2 [func] Reject stale pod addresses after the materialization fence fails

If another API replica recreates the pod on the same node while this replica still caches its predecessor, the node-name comparison above skips placement validation. persist_observed_pod_metadata correctly rejects the predecessor's UID/materialization token with Ok(None), but this branch only logs that rejection and the function still returns the predecessor's pod_ip. Runtime connections and sandbox access callers can therefore use a stale endpoint despite the database fence.

I reproduced this with PostgreSQL and a synthetic pod cache: the successor's database identity stayed intact, while resolve_pod_ip returned the predecessor's IP. Please return None or a retryable error when the identity fence rejects the observation, and add a resolver regression test for same-node recreation with a stale cached predecessor.

[the-gigi-pplx]

Review — 5 findings

Medium

1. pod_ops/delete.rs:165 — Cleanup of a pod this request just created adds its POST-response resourceVersion to the DELETE precondition.
   delete_created_materialization_pod routes through delete_observed_pod, which always sends resource_version: pod.resource_version(). For a freshly POSTed pod, the scheduler bind and kubelet status writes move that version forward while persist_materialized_pod_metadata runs. The DELETE can then 409 and come back as Unavailable. In the ownership-revoked arm (ensure_pod.rs:1160, and the redrive OwnershipLost arm) that propagates via ? after the claim is already gone. The redrive persist-error arm only logs it. Either way the unowned pod keeps running until some other path removes it. The RV precondition earns its place on the obsolete-token path (delete.rs:121-127), where the annotation can change on the same UID. A pod we created ourselves is identified by its UID alone, which is what the sibling persist-error arm in ensure_pod.rs already does with Some((pod_uid, None)). I suggest a UID-only precondition for the created-pod helper and keeping UID+RV for the obsolete path.

2. store.rs:62 / ensure_pod.rs:89-104 — Rolling back past this PR after fenced mode is on makes the next roll-forward delete running pods.
   Once SPACE_POD_MATERIALIZATION_MODE=fenced has minted tokens, a pre-PR binary keeps them: the base claim_active_execution_pod_materialization only resets state, pod_uid and pod_ip. It then creates pods without the materialization annotation. When this binary returns, stale_pod_reason_for_execution classifies any same-generation pod whose annotation doesn't match the row's token as ObsoleteMaterialization. That check runs before any phase check, so delete_execution_pod's obsolete branch deletes a healthy Running pod. The one-line comment at store.rs:62 ("rolling back needs the same rule") is the only place this constraint is written down. Please state it in the rollout plan in the description, i.e. after enabling fenced mode, roll back only to compatibility mode on a token-aware binary. Or, if a pre-PR rollback has to stay safe, make the obsolete classification require positive evidence, a different valid token, rather than a missing one.

Low

3. sandbox_pod_view.rs materialization_id() — A malformed annotation becomes SubstrateError::Failed at every ? site (ensure_pod.rs:455/618/825, wait_ready.rs:124, read.rs:179/437, substrate_adapter.rs:422). delete.rs:135-136 explicitly treats a damaged annotation as reachable. On a token-less row, though, the adoption paths turn the same damage into a terminal failure rather than the conflicting-evidence or restore_in_flight handling a possibly-live pod gets elsewhere. It's only reachable through an outside edit to the pod, hence Low.

4. types/sandbox.rs:2796-2861 — PodMaterializationId hand-writes the sqlx Type/Encode/Decode impls that postgres_uuid! in types/postgres.rs already generates (it only needs as_uuid/from_uuid, which the type has). Also, impl Default mints a fresh v7 token. Nothing calls it today, but an unwrap_or_default() or a derived Default on a containing struct would silently create a token the database never issued. I'd drop the Default.

5. node_drain/store.rs:3152-3159 — The drain comment still says placement "upserts this same nodes row (INSERT … ON CONFLICT (id) DO UPDATE) before it writes sandboxes, so it already holds this lock." After this PR the placement transaction takes the sandbox lock first and then FOR SHARE OF n, k, and the upsert moved to the separate repair transaction. FOR SHARE still conflicts with the drain's FOR UPDATE, so the serialization holds, but the stated mechanism and order no longer match the code.

Verified against e5a0b2582a1306e004e008b2cb9a2a7464459d04, reviewing the diff against pr318. I read the delete, ensure-pod, redrive, claim, placement and drain paths, and checked both rollback findings against the base branch's claim SQL. Nothing was executed: CI is green on this head and I didn't re-run the suites. I also looked at and set aside: stale claim.row.state in legacy_cleanup_matches (this PR now refreshes it after reset in redrive.rs), a lock-order deadlock between placement's FOR SHARE and node termination (delete_terminating_node_and_record_tombstone never locks sandboxes, and there's no FK from sandboxes to nodes), and the removed tracking cleanup on the ownership-revoked path (the generation that would be cleaned no longer owns the row).

[the-gigi-pplx]

Approved. 5 non-blocking findings in the review comment above.