feat(oabctl): programmatic delete API with explicit target contract

[chaodu-agent]

Summary

Designs and implements the programmatic delete contract that the #1404 review intentionally deferred ("until their programmatic contracts are designed independently"). Stacked on refactor/oabctl-lib.

let report = oabctl::delete_services(
    &aws,
    &[DeleteTarget::new("prod", "nest-my-oab")],
    &DeleteOptions::new("oab"),
).await?;

Symmetric with apply_manifests:

• DeleteOptions requires an explicit cluster; the library path never reads ~/.oabctl/config.toml
• Bucket resolution shares apply's exact chain (explicit → OAB_CONTROL_PLANE_BUCKET → oab-control-plane-{account}) — delete always cleans the bucket apply wrote to, closing the round-2 bucket-mismatch concern for good
• Contract guards fire before any AWS call: non-empty target set, non-empty cluster, non-blank namespace/name — regression-tested without AWS
• Typed DeleteError { Validation | Target | Teardown } preserving the DeleteReport for targets completed before a failure; teardown stays resumable (re-run the same target to continue)
• Best-effort ingress skips surface in DeletedService.warnings; S3 cleanup failures still fail the target (safe to retry)
• delete.rs output now routes through the shared progress gate: CLI rendering unchanged, programmatic calls silent

CLI

No behavior change: oabctl delete <name> and delete -f keep their existing flow (config-file cluster resolution, printed progress, aggregate failure reporting).

Motivation

NEST's manager needs /delete (tenant deprovision) — webhook removal and Telegram-side disclosure stay in the control plane, while ECS/ingress/S3 teardown reuses this API in-process, matching how it already consumes apply_manifests.

Testing

On 7a8cdbe: cargo clippy --all-targets -- -D warnings clean; cargo test 83 passed + doc-test (6 new delete contract tests); CLI surface untouched.

[chaodu-agent]

Pre-merge verification — code review + real-AWS E2E at 1bd61e8

Code review of the identity-checkpoint path (prepare_identity / classify_service_identity / validate_checkpoint):

• The checkpoint is minted from the authoritative DescribeServices response (service ARN, canonical cluster ARN, createdAt incarnation, exact registry ARN, resolved API id) and persisted before any teardown starts — dependent cleanup is never addressed by name alone. The old first-name-match Cloud Map fallback is gone (round-2 F1 resolved at the root).
• classify_service_identity is fail-closed on every ambiguous case: empty response without an explicit MISSING failure, INACTIVE without a checkpoint, status without identity, multiple services, multiple registries. A checkpoint only authorizes "gone" for exactly zero services + one MISSING failure, or INACTIVE after full ARN/incarnation validation.
• Checkpoint validation binds version + namespace/name + requested cluster + bucket + partition/account/region; it is deleted only after S3 cleanup completes, so a same-name recreate cannot inherit a stale identity.

E2E against real AWS (synthetic tenant nest-e2e-1415, account 903779448426, cluster oab; harness calls apply_manifests/delete_services in-process):

Scenario	Result
Apply (no checkpoint — same as any pre-checkpoint deployment) then delete while the service was still provisioning	✅ Refused dependent cleanup after the drain wait: checkpointed ECS service did not reach an unambiguous absent state; dependent cleanup was not started (safe to retry) — checkpoint retained for resume
Retry delete (checkpoint resume path)	✅ Full teardown (ECS + Cloud Map + API GW + S3), zero warnings, checkpoint removed after success — verified via S3/ECS/API-GW state, not just the report
Delete the now-gone tenant again (INACTIVE visible, no checkpoint)	✅ Failed closed: ECS returned INACTIVE without a matching delete checkpoint — no false success, no cleanup of unowned resources
Bonus: apply with an invalid Telegram token	✅ webhook registration failure surfaced as a structured AppliedService.warnings entry, apply succeeded

Also verified earlier on this branch: CI operator check green (clippy 1.97 -D warnings, 96 unit tests + doc-test).

Verdict: good to merge. The delete contract does what the two review rounds demanded — checkpointed exact identity, fail-closed ambiguity handling, resumable teardown with no false success — and it holds up against real AWS, including the failure paths.

[chaodu-agent]

Important

CHANGES REQUESTED ⚠️ — Exact checkpoints now fail closed on ambiguous AWS responses, but the logical target is still non-unique and apply can race the post-ECS cleanup, allowing deletion of another or newly recreated deployment.

What This PR Does

This PR adds a public in-process delete_services API for deleting OAB services by explicit namespace + name, with an explicit ECS cluster, structured partial results, shared control-plane bucket resolution, silent library execution, and durable exact-resource checkpoints for resumable teardown.

How It Works

Each target is validated, resolved to its live ECS service, and checkpointed in S3 with canonical cluster/service ARNs, the ECS createdAt incarnation, registry ARN, API ID, caller boundary, and bucket. Delete then drains the checkpointed ECS incarnation, removes checkpointed ingress dependencies, deletes the target's manifest/artifacts, and removes the checkpoint last. Initial missing or ambiguous ECS responses fail closed; retries continue only from a matching checkpoint.

Findings

#	Severity	Finding	Location
1	🔴 Critical	namespace and name are concatenated with -, so distinct logical targets can resolve to the same ECS/API/Cloud Map identity; the initial lookup can checkpoint and delete the other target's live resources.	operator/src/delete.rs:415
2	🔴 Critical	After the old ECS incarnation is observed gone, delete performs dependency and name-keyed S3 cleanup without a fence shared with apply; a concurrent recreate can reuse those resources and then have them deleted.	operator/src/delete.rs:1032
3	🟢 Praise	The exact ARN/incarnation checkpoint, explicit-MISSING retry rule, fatal dependent cleanup, and structured partial report substantially improve fail-closed and resumable teardown behavior.	—

Finding Details

🔴 F1: The logical target does not map injectively to its physical identity

Validation rejects only blank components, while ECS and Cloud Map use oab-{namespace}-{name} and API Gateway uses oab-webhook-{namespace}-{name}. For example, prod/team-bot and prod-team/bot both resolve to oab-prod-team-bot and oab-webhook-prod-team-bot.

The checkpoint does not close this initial ambiguity. If prod/team-bot is deployed and a caller requests deletion of prod-team/bot, prepare_identity describes the same physical ECS service. Its ARN and createdAt are valid for the derived name, so the code mints a new checkpoint carrying the requested—but wrong—logical pair and proceeds to delete that service and its exact dependencies. Separate S3 paths only mean the actual deployment's manifest may remain after its ECS/API/Cloud Map resources are removed.

Requested change: define one injective canonical resource identity and verify the logical identity before minting a checkpoint. A reversible encoding or immutable namespace/name tags/metadata can preserve both components; validation alone is sufficient only if it proves the delimiter cannot occur. Add a regression proving that two distinct component pairs can never resolve to the same delete identity.

🔴 F2: Delete and apply have no shared target fence

delete_checkpointed_ecs protects mutations of the checkpointed ECS incarnation, but once it returns RetryGone, run_with_bucket immediately deletes the checkpointed API/Cloud Map identities and then unconditionally deletes manifests/{namespace}/{name}.yaml plus artifacts/{namespace}/{name}/. There is no final incarnation check, conditional S3 generation/ETag, or target lease, and apply.rs never reads delete-checkpoints.

A same-target apply can therefore start after the old ECS incarnation is observed gone but before cleanup completes. If it starts before dependency cleanup, ensure_api and ensure_cloud_map reuse the still-existing API ID and registry ARN that delete is about to remove. Regardless of ingress timing, apply can write a new manifest/artifact generation that cleanup_s3 then deletes by name. Keeping the checkpoint until the end preserves retry identity but does not fence a concurrent writer.

Requested change: add a target-scoped fencing protocol shared by apply and delete—such as a lease/tombstone with a generation token that apply must reject or supersede explicitly, plus conditional S3 mutation. Revalidation alone only narrows the race. Add an interleaving test where apply recreates the target after ECS drain completion and prove delete cannot remove the new incarnation or its desired state.

🟢 F3: Exact checkpointing closes the earlier ambiguous-cleanup paths

The current code now requires one identified ECS service before creating a checkpoint, binds retries to canonical ARNs plus createdAt, treats drain exhaustion as failure, and preserves the checkpoint on dependent/S3 errors. These are strong foundations once logical identity and cross-operation serialization are fixed.

Baseline Check

• PR opened: 2026-07-16
• Reviewed head: 1bd61e825e3f2e1308bde3be6294e10b0e3bead6
• Base branch: refactor/oabctl-lib at 6dd5285; main checked at 30bc145
• Main already has: no programmatic delete API matching this contract
• Net-new value: explicit typed delete targets/options/results plus exact resumable teardown identity
• Validation: git diff --check passed; the exact-head operator CI job passed. Local cargo test/cargo clippy could not run because this review environment has no Cargo toolchain.

Previous Review Feedback

• ✅ Drain timeout could report success — fixed: exhaustion now returns an error before dependent cleanup.
• ✅ Ingress failures/warnings were lost and library calls emitted progress — fixed: dependent failures are fatal or structured, and ingress rendering now uses the shared progress gate.
• ✅ Initial missing ECS responses and name-only Cloud Map fallback were unsafe — fixed: initial ambiguity fails closed and dependent IDs come from the checkpointed ECS identity.

Addressing External Reviewer Feedback

@chaodu-agent (pre-merge verification at 1bd61e8)

“The checkpoint is minted from the authoritative DescribeServices response” and the real-AWS retry completed exact teardown with the checkpoint removed last.

✅ Confirmed for the exercised scenarios: the current code and reported E2E demonstrate fail-closed missing/ambiguous responses, safe retry of the same checkpointed incarnation, and successful complete teardown.

⚠️ Two cases remain unexercised: the E2E used one canonical namespace/name pair, so it did not test a different pair with the same derived physical name (F1). It also did not interleave a new apply after the old ECS incarnation became absent but before dependency/S3 cleanup (F2). Removing the checkpoint last supports delete retries, but apply does not consult that checkpoint, so it is not a cross-operation fence.

Review Coverage

Reviewer	Focus	Result
Reviewer A	Correctness / target identity	F1
Reviewer B	Architecture / API contract	F1, F2
Reviewer C	Safety / operability	F2
Reviewer D	Testing / regression coverage	Collision and apply/delete interleaving tests required
Reviewer E	Independent exact-SHA audit	Confirmed the missing apply/delete fence; final aggregation strengthened the initial-lookup collision analysis

What's Good (🟢)

• Contract validation and bucket selection are explicit and library-friendly.
• DeleteError retains both the failed target and successfully completed prior targets.
• Exact ARN, caller-boundary, registry, and createdAt checks eliminate the earlier wrong-cluster/name-only cleanup behavior.
• Dependent and S3 failures retain the durable checkpoint for safe retry.
• The author supplied useful real-AWS evidence for initial delete, retry, and already-gone failure paths.

5️⃣ Three Reasons We Might Not Need This PR

1. The CLI already owns teardown orchestration — if control-plane callers can safely invoke it, a second public contract adds a long-lived compatibility surface.
2. Logical identity is not yet durable enough for a destructive API — a persisted deployment ID or immutable tagged identity may be safer than reconstructing ownership from two free-form strings.
3. Apply/delete serialization may belong in the control plane — if this crate cannot provide a shared fence, exposing raw concurrent operations could create more risk than an orchestrated deprovision workflow.

[chaodu-agent]

Important

CHANGES REQUESTED ⚠️ — The logical delete target is non-unique, and post-ECS cleanup is not fenced against a concurrent apply.

Consolidated review: #1415 (comment)

GitHub event: COMMENT — self-review delivery only; this is not an approval.

[chaodu-agent]

🔴 F1 — Make the target-to-resource identity injective

Blank-only validation allows distinct targets such as prod/team-bot and prod-team/bot to derive the same ECS, API Gateway, and Cloud Map names. On the first delete, prepare_identity can therefore resolve the other logical target’s live ECS service and mint a fully valid exact-resource checkpoint for the wrong pair.

Requested change: use a reversible/injective physical identity (or immutable logical-identity metadata verified before checkpoint creation) and add a regression proving distinct namespace/name pairs cannot collide.

[chaodu-agent]

🔴 F2 — Fence post-ECS cleanup against concurrent apply

Once this call observes the old incarnation gone, the code immediately deletes checkpointed API/Cloud Map resources and name-keyed S3 state. apply does not read delete-checkpoints and can reuse those dependency IDs or write a new manifest in this window, after which this delete removes the recreated deployment’s resources/state.

Requested change: introduce a target-scoped generation/lease/tombstone protocol shared by apply and delete, with conditional S3 mutation, and test an apply interleaving after ECS drain completion.