[shehan-pplx]

What

When a create ends early after its pod exists, the create cleanup retires the execution and then deletes the pod by UID. The retire is fenced on generation and activation attempt.

Before this change, the cleanup returned without deleting the pod if the fenced retire matched nothing. Now it checks whether the execution row that names the pod at its generation is retired (K8sStore::execution_generation_retired):

• Retired: delete the pod under its UID precondition. A retired generation is final. Every activation assignment requires active, pod_name names a single generation, and sandbox-manager refuses to start a retired generation.
• Still active: a newer activation owns the pod. Leave it and log that.
• Read error: leave the pod for reconciliation, as other cleanup failures already do.

Why

The fenced retire matches nothing when another writer has already retired the execution. One such writer is the control plane applying a node's activation-rollback receipt, which retires the execution as replaced and never touches the pod.

In that case the cleanup skipped the delete. Kubelet kept retrying the pod's sandbox create against the retired generation, and sandbox-manager kept refusing it. The pod stayed Pending until something else deleted it.

Testing

Unit and integration tests:

• create_ending_early_gives_back_exactly_what_it_claimed::case_8_cancelled_after_execution_retired_elsewhere is a new case in the existing create-cancellation test. It retires the execution as replaced while the create waits for readiness, then drops the create.
  • On main it fails with expected one pod delete, got [].
  • Here it passes, and the delete carries the created pod's UID precondition.
• execution_generation_retired_reads_only_an_inactive_row_naming_the_pod (Postgres) covers the probe. An active row, another pod name, or a missing generation all read as not retired.
• cargo test --locked -p pplx-space-api --features kubernetes,test-hooks -- execution_generation_retired create_ending_early_gives_back_exactly_what_it_claimed: 9 passed.
• cargo fmt --all -- --check, cargo clippy ... -- -D warnings and cargo arch are clean.

E2E on kind with this PR and #1631 deployed: the scenario local.retired_execution_pod_gc_reaps_retired_pods from #1734 passed with no cleanup failures. Its phase "a stranded create whose create ends: its cleanup deletes the Pending pod" took 45.6 s:

• The scenario killed the sandbox-manager on space-local-worker2 while the cold create was in starting. node-ctl published its inventory until the execution row read state=replaced, active=false, with the pod still Pending and no pod IP.
• When the in-flight create ended, the scenario polled for the pod. create_cleanup_deleted_pod_after_seconds: 0.0 means the pod was already gone, while the GC loop was still off.
• In the same run, two stranded creates whose API replicas were force-killed (so no create cleanup ran) kept their Pending pods for the full 60 s GC-off window. That contrast points to the create cleanup as what deleted the first pod. The scenario reads cluster state, not logs, so it does not record which delete call ran.

After the run, kubectl --context kind-space-local -n space get pods showed no space-sbx- pods, and the sandboxes table was empty.

Base of the stack: #1631 adds the cluster-controller backstop, and #1734 adds the kind scenario.

Created with Perplexity

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The change adds a PostgreSQL-backed probe for inactive execution rows matching a sandbox, generation, and pod name. Cancelled-create cleanup uses the probe after fenced retirement returns false. It deletes the pod by UID when the generation is already retired, and leaves the pod when the generation remains active or the probe fails. Tests cover the probe and cancellation after external retirement.

Possibly related PRs

• perplexityai/SPACE#1631: Adds a cluster-controller backstop for retired-execution pods that the cancellation cleanup path does not reach.

Merge Risk: Moderate · up to 54eb0

Cancelled creates can now delete a pod after another actor retires its execution. The targeted test covers this path, but the lifecycle change warrants moderate merge caution.

Security architecture risk: Low · up to 54eb0

Cleanup is expanded only for a previously created pod whose execution is confirmed retired. Active-owner and replacement-pod protections remain in place. The assessed risk is low, with wider recovery behavior not exhaustively verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Cancellation can trigger service-credentialled cleanup, but the inspected destructive operation targets one recorded pod UID in the configured sandbox namespace. The pod name and generation come from the claimed execution, and the UID comes from the pod-create response; the new probe does not add caller selection of a deletion namespace or arbitrary target UID.

Trust Boundaries and Controls

• observed — The database ownership check and Kubernetes object-identity check protect different boundaries. The probe requires the exact sandbox, generation, and pod name to be inactive; the unchanged delete helper supplies the recorded UID as a Kubernetes precondition. Missing UIDs prevent cleanup, and a same-name replacement with another UID cannot satisfy that deletion precondition.

Resilience and Maintainability Implications

• observed — Retirement and pod deletion remain separate operations rather than an atomic cross-system transaction. Cleanup preserves the existing containment behavior: an uncertain pod-create outcome retains the claim, node-manager unreachability defers cleanup, and deletion failures are logged without falling back to an unfenced delete. The new probe similarly leaves uncertain state for reconciliation; eventual recovery was not independently demonstrated.

Pre-merge checks: 6

Check name	Status	Explanation
Title check		The title uses the lowercase area prefix "api:" followed by a colon, then a summary starting with the lowercase word "delete". It has no trailing period and no ticket ID. It names the main change: the…
Description Follows The Pr Template		The description contains all four required items: Summary, Validation with results, Severity and risk, and an AI assistance line naming the tool and its contributions.
Bug Fixes Name A Regression Test		The Validation section names create_ending_early_gives_back_exactly_what_it_claimed::case_8_cancelled_after_execution_retired_elsewhere and states that it fails on main and passes with the fix. The …
Weakened Safeguards Are Justified		The diff does not weaken a test, assertion, lint, CI check, or repository guard. It retains the existing cancellation test assertions and adds a new case.
Agent Tooling Runs No New Commands		The diff changes only Rust application code and tests. It does not change agent manifests, skills, command files, guidelines, or other content that agent tools run.
Public-Safe Content		The title, description, and reviewed diff contain no identified content covered by the failure conditions.

Full details: Title check

Explanation

The title uses the lowercase area prefix "api:" followed by a colon, then a summary starting with the lowercase word "delete". It has no trailing period and no ticket ID. It names the main change: the cancel cleanup deletes the pod of a cancelled create when its execution is already retired. The title matches the summary and objectives, which describe the fenced retire check that deletes the pod by UID.

Full details: Bug Fixes Name A Regression Test

Explanation

The Validation section names create_ending_early_gives_back_exactly_what_it_claimed::case_8_cancelled_after_execution_retired_elsewhere and states that it fails on main and passes with the fix. The diff adds that case and checks the UID-fenced pod deletion.

Recent review info

Automatic approval policy

Commit: 54eb098679dd.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
Merge risk	Moderate	Global configuration
Security risk	Not set	Global configuration

No overrides matched. The global limits apply.

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: shehan/cancel-cleanup-delete-retired-pod
• File: .coderabbit.yaml
• Commit: 54eb098
• Layers: Repository YAML; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Enterprise
• Run ID: db8f4d33-87c2-44c4-86e3-35633095c3d1

Commits

Reviewing files that changed from the base of the PR and between 0c895f3 and 54eb098.

Files selected for processing: 3

• crates/l40_api/api/src/controllers/substrate/kubernetes/store/k8s_store_postgres_tests.rs
• crates/l40_api/api/src/controllers/substrate/kubernetes/store/pod_tracking.rs
• crates/l40_api/api/src/controllers/substrate/kubernetes/tests.rs

Limit details: You’ve used all 12 included reviews currently available.

[shehan-pplx]

/pplx-review 0c895f3

[shehan-pplx]

Reviewed at 0c895f3. No issues found.

• The fallback reads retirement for the exact (sandbox, generation, pod_name) and deletes only under this create's recorded UID. A missing row or an active row leaves the pod alone, and a read error fails closed.
• Cross-PR check against A3 (#1627): API teardown deletes the pod before it retires the row, so this path can't delete a pod ahead of teardown's node-local delete and its refusals. A concurrent stop path that retires first and then deletes issues the same UID-preconditioned delete, so the worst case is a benign NotFound.
• The new test case reproduces the rollback-receipt shape and fails on main.