[Decision][Epic 953][Stage I][chat-api] Preserve account deletion across Cloud migration

[hanakannzashi]

Tracks #380

Resolved — no Chat API code change required

The existing Chat account-deletion contract remains unchanged in Stage I:

• DELETE /v1/users/me remains the authenticated asynchronous account-deletion endpoint.
• Accepted jobs keep the established worker behavior, including Cloud Conversation/File cleanup followed by local finalization.
• Cloud #943 retains DELETE /v1/conversations/{conversation_id} and DELETE /v1/files/{file_id} as the explicit Stage I compatibility exception, so the worker continues to use its existing API-key/workspace-scoped calls.
• This is a retained public resource-delete surface, not a new trusted Chat-only token or internal Cloud path. Other Cloud Conversation/File mutations remain retired.

Why this PR is closed

The earlier worker-change direction—removing Cloud cleanup calls while retaining local finalization—would have produced partial deletion and is intentionally not used. With the retained Cloud DELETE endpoints, no Chat worker or user-facing API change is required for the selected Stage I plan.

This closed PR is retained as the decision record. Account-deletion verification is tracked by #380 and must be included in the Cloud/Chat rollout.

Out of scope

• Changing or disabling the user-facing account-deletion contract.
• Removing worker cleanup or local finalization.
• Frontend changes, archive/tombstone work, or Stage III cleanup.

[claude]

Review

✅ Approved — no blocking issues.

The change matches #380's scope and acceptance criteria. Local finalization still deletes the conversations and files rows inside the same transaction (crates/database/src/repositories/user_repository.rs:341-347), so removing the cloud-cleanup prerequisite does not strand local user data, and the deletion path no longer contacts the retired endpoints at all — so a 410 cannot produce retrying/failed_needs_review for new runs.

Notes below are non-blocking.

1. Rollout ordering vs. the interactive delete paths

The same upstream calls are still live outside the worker:

• delete_file_from_openai → DELETE files/{id} (crates/services/src/file/service.rs:128), reached from DELETE /v1/files/{file_id} (crates/api/src/routes/api.rs:2168)
• delete_conversation_from_openai → DELETE conversations/{id} (crates/services/src/conversation/service.rs:365), reached from delete_conversation

Both map a non-200 to a 502 for the user. Leaving them is correct (#379 owns them), but if this ships ahead of #379, user-initiated deletes break while account deletion works. Worth stating the intended order explicitly in the rollout notes.

2. Records already parked by the pre-rollout worker

The acceptance criterion holds for new runs, but anything the old worker already pushed to failed_needs_review with a "failed to delete cloud conversation" / "failed to delete provider file" last_error is dropped by the new worker (crates/api/src/bin/task_worker.rs:241-248 returns Ok(()) and acks the message) and will never retry on its own. During a rolling update an old worker can still push records past MAX_ACCOUNT_DELETION_ATTEMPTS into that state.

The admin retry endpoint recovers them (crates/api/src/routes/admin.rs:3660 → retry_failed_account_deletion, which resets attempt_count = 0), so this is operational rather than a code fix — just include a sweep of those rows in the rollout checklist.

3. normalize_legacy_account_deletion_progress is lossy, not preserving

It rebuilds the object from scratch and drops every key other than the two legacy lists — the unit test asserts exactly that on unrelated_legacy_value. No practical impact today (the worker is the only writer of progress, and only ever wrote those two keys), but the doc comment "Safely preserves legacy Cloud cleanup markers" reads stronger than the behavior. Either merge into the existing object:

let mut normalized = progress.as_object().cloned().unwrap_or_default();
normalized.insert(
    "cloud_deleted_conversation_ids".into(),
    json!(progress_string_ids(progress, "cloud_deleted_conversation_ids")),
);
normalized.insert(
    "cloud_deleted_file_ids".into(),
    json!(progress_string_ids(progress, "cloud_deleted_file_ids")),
);

…or tighten the comment to say it is a normalize-and-narrow that is slated for removal with #381. Minor side effect: failure paths now stamp both legacy keys as empty arrays onto records that never had those fields, adding noise to the failed_needs_review audit trail.

4. Dead surface left behind (fine to defer to #381, tracking only)

• AccountDeletionError::ConversationCleanupIncomplete and AccountDeletionError::FileCleanupIncomplete are now unconstructible but still mapped in crates/api/src/routes/users.rs:192-204.
• list_owned_conversation_ids / list_owned_file_ids have no production callers left (user/ports.rs:255,258, user/service.rs:219-227, user_repository.rs:714,725).
• The _cloud_deleted_* params are dead at their only call site, which now passes two empty slices.

Testing & privacy

Coverage lines up with the issue's ask — legacy file rows at crates/api/tests/user_account_deletion_tests.rs:389, legacy conversation row at :154, plus the malformed-JSON unit test. No new log statements touch conversation content, titles, or credentials; IDs only, per CLAUDE.md.

One caveat: cargo check was not runnable in my environment, so I relied on the clippy/test results reported in the PR body. I did verify manually that Bytes, TryStreamExt, and Method in conversation/service.rs are still used by delete_conversation_from_openai, so removing delete_file_from_provider leaves no unused imports there.

[ironloopai]

🧭 IronLoop Run · Review

This comment updates in place as the Run moves through its stages.

🟩 Final result · Completed

🟨 Queued → 🟦 Working → 🟦 Posting results → 🟩 Completed

_{Automatic trigger · attempt 1 of 3 · completed in 45s}

IronLoop completed the review and posted it to GitHub.

🔗 Result

Open submitted review →

Run details

Run: f854bb98-a09a-41ae-994f-aec09866abd5
Base: main at 686d724
Head: codex/issue-380-worker-retired-cloud at 02cc9ba
Created: 2026-08-20 03:26 UTC
Updated: 2026-08-20 03:27 UTC

[ironloopai]

🔍 IronLoop review

🟢 No actionable findings

No actionable defects found. The change removes retired upstream cleanup calls while preserving transactional deletion of local conversation and file rows, and legacy progress data remains non-blocking for retries.

Validation

• ✅ Account-deletion flow inspection — Verified the worker no longer constructs Cloud/VPC deletion dependencies or invokes provider cleanup, while local account finalization still deletes conversations and files transactionally.
• ✅ Diff integrity — The complete change contains no whitespace errors.

Review details

• Run: f854bb98-a09a-41ae-994f-aec09866abd5
• Workflow: Review
• Attempts: 1

[hanakannzashi]

Stage I rollout clarification

Cloud API's Conversation and File mutation endpoints are write-disabled in Stage I. This worker change is therefore a Stage I compatibility prerequisite, not final stateful-surface removal: account deletion must not retry those upstream deletes or fail because they return 410 Gone.

It preserves local account-deletion finalization and does not alter the temporary read/export views. Deploy it before or with the Stage I proxy boundary and Cloud API #943; final removal remains Stage III work.

[hanakannzashi]

Update: the Stage I decision is now to disable self-service account deletion, not to preserve local finalization behind an archive/deferred choice. This Draft must be reworked to return 410 Gone before task creation and safely recover existing in-flight deletion requests; it must not delete the ownership manifest.

[hanakannzashi]

Clarification: the Stage I decision is not to cancel accepted account-deletion jobs. New requests become 410 before task creation; pre-cutoff jobs keep their legacy cleanup path and must drain before Cloud disables its delete endpoints. This supersedes the prior Draft comment about recovering existing jobs.

[hanakannzashi]

Reworked at 62c5d71 on the current #382 base. New DELETE /v1/users/me (including /me/) returns authenticated 410 Gone with Cache-Control: no-store before any deletion row or task is created. The legacy worker, Cloud/local cleanup path, and admin retry remain unchanged so pre-cutoff jobs can drain normally. OpenAPI and regression tests now cover the cutoff; CI has been requeued.

[claude]

Review: Stage I account-deletion cutoff

No blocking correctness or safety defects found. The handler is genuinely inert (no AppState, no repository/queue access), the trailing-slash route is needed because nest("/v1/users", ...) would otherwise send DELETE /v1/users/me/ to the SPA fallback, auth still applies to /me/ via the router-level auth_middleware layer, and the logging follows CLAUDE.md (user_id only, no content). Rolling-update safe: mixed old/new pods only differ in whether a new request is accepted, and the worker path is untouched so accepted jobs still drain.

Non-blocking items worth addressing:

1. Coverage regression on still-live guards (highest-value item)

delete_account_blocks_non_terminal_subscription_statuses and delete_account_blocks_non_deleted_instance were the only tests exercising BlockingSubscriptions / InstancesNotDeleted. Those preconditions are still live code — delete_user_account validates them (crates/database/src/repositories/user_repository.rs:258) and the worker branches on them (crates/api/src/bin/task_worker.rs:426) to move a job to failed_needs_review. That is exactly the path the backlog you are draining runs through, and it now ships untested.

Suggest keeping the two cases at repository level rather than deleting them, e.g.:

// insert non-terminal subscription, then:
let err = db.user_repository()
    .delete_user_account(user.id, &[], &[])
    .await
    .expect_err("non-terminal subscription must block finalization");
assert!(matches!(err, AccountDeletionError::BlockingSubscriptions { .. }));

2. No accepted path for new erasure requests during the window

There is no admin equivalent — /v1/admin/account-deletions is list-only and /retry only handles failed_needs_review. So for the duration of Stage I there is no self-service or operator mechanism to accept a new deletion request. That looks intentional per the PR body, but it should be backed by a written manual/DB runbook before deploy, since erasure requests do not stop arriving during the window.

3. Orphaned code left behind

• UserAccountDeletionResponse + its From<AccountDeletion> impl (crates/api/src/models.rs:47-64) now have zero references and were dropped from the OpenAPI components.
• UserService::create_account_deletion_request / delete_account_deletion_request have no production callers left (tests only).
• allow_delete_retry in crates/api/src/middleware/auth.rs:301 is now dead intent: it exists to let an already-deleting account re-hit DELETE /v1/users/me, which can no longer do anything. Side effect: a deleting user gets 410 here but 403 on every other endpoint.

None of these break anything; each is a trap for the next reader. A comment tying them to #380 would do if you want to keep them for Stage III.

4. Minor

• tracing::warn! fires on every call to a client-reachable endpoint — any authenticated user can generate warn-level volume at will. info! fits the event better.
• OpenAPI documents only 410/401, but auth middleware can still return 403 (account deleting) on this path; /v1/users/me/ is not documented at all.

✅

[hanakannzashi]

Final Stage I decision: normal DELETE /v1/users/me and its existing worker remain active; Cloud #943 retains both resource DELETE routes. This supersedes the earlier cutoff/410 and worker-removal comments above. No code from this closed PR is to be merged; #380 owns end-to-end rollout verification.