fix(backend): unconfigured recordings bucket deadlocks account deletion (6 users stuck on prod)#9632
Merged
kodjima33 merged 1 commit intoJul 13, 2026
Conversation
…t deletion
BUCKET_MEMORIES_RECORDINGS was never wired into the backend charts, so
`memories_recordings_bucket` resolves to None in prod. delete_all_conversation_recordings
then calls .bucket(None), and listing raises ValueError("Cannot determine path without
bucket name").
That purge is a *required* step, so the raise aborts background_wipe_user_data before
users_db.delete_user_data(): every account-deletion request fails, is marked wipe_failed,
and is re-enqueued forever with the user's data still in Firestore. Six uids are stuck in
that loop in prod right now.
An unconfigured bucket now means "nothing to purge" — uploads resolve the same name, so a
deployment without it cannot have stored recordings. A real GCS failure still raises and
still blocks the irreversible wipe (that contract is unchanged and covered by
test_gcs_failure_blocks_firestore_wipe).
Verification (real path, live prod GCS, prod service account):
- Replaying the pre-fix body with the bucket unset and the actual stuck uid reproduces the
prod log line byte-for-byte: ValueError: Cannot determine path without bucket name.
- With the fix, the same call returns cleanly, so the wipe proceeds to the Firestore delete.
- With the bucket configured, the purge still lists gs://memories-recordings and deletes the
uid prefix (exercised against a uid with no blobs, so nothing was deleted).
- New regression test fails without the guard, passes with it. backend/test.sh: 564 files, 0 failures.
NOTE: the underlying config gap remains — BUCKET_MEMORIES_RECORDINGS is absent from
prod/dev Helm values and .env.template, so conversation recordings are not being purged on
account deletion (one legacy uid still has objects in gs://memories-recordings). Wiring the
env var is deploy config and is left for a maintainer.
This was referenced Jul 14, 2026
Git-on-my-level
added a commit
that referenced
this pull request
Jul 15, 2026
## Summary Closes #9760. - Makes production account deletion fail closed: a deletion intent is durably recorded with an opaque wipe job ID before the wipe can be dispatched, and production startup rejects inline or incomplete Cloud Tasks configuration. - Dispatches only the opaque job ID to the dedicated account-deletion queue; the handler resolves the user server-side and drops unknown, ambiguous, terminal, and unsupported legacy payloads before any mutation. - Adds reconciliation for persisted-but-undispatched intents, validates the GKE/Cloud Run deployment contract, and gives deletion tasks an account-deletion-specific OIDC audience. ## Root cause and recurrence The deletion workflow previously relied on inline execution as a reachable deployment mode and accepted a payload that could identify a user directly. A crash or configuration drift between the delete request and its side effects could therefore leave a deletion incomplete without a durable, safely replayable dispatch contract. This follows the same dispatch/reliability failure class addressed in #5088, #6750, #7640, #8405, #9117, and #9632. The durable guard is now the persisted wipe-job identity plus production-only Cloud Tasks configuration validation, queue payload minimization, dedicated OIDC audience verification, and reconciliation through the same dispatcher. ## Verification - `cd backend && ENCRYPTION_SECRET=local-test-secret BACKEND_PYTEST_WORKERS=1 bash test.sh` — passed all 603 isolated unit-test files. - `cd backend && PATH="/tmp/omi-predeploy-9760/bin:$PATH" bash scripts/pre-deploy-check.sh` — passed (69 tests); validates dev/prod runtime and deploy contracts. - `make preflight` — passed. - Exercised the controlled recovery path: persisted intent -> injected enqueue crash -> reconciler dispatch -> first handler completion -> repeat delivery no-op. This is covered by `test_persisted_wipe_recovers_after_enqueue_crash_and_handler_runs_once`. No live Cloud Task was dispatched because this PR does not authorize a deployment mutation. ## Review Sol review completed before opening. It identified an OIDC audience-isolation issue; the fix uses `ACCOUNT_DELETION_TASKS_OIDC_AUDIENCE` (defaulting only to the dedicated deletion handler URL) for both task creation and verification. A second independent Sol review after rebasing identified the deploy-order compatibility path for queued legacy UID tasks; the resolved branch accepts the former sync audience only for those legacy payloads and drops job-ID payloads before resolution or mutation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug (live on prod, right now)
Account deletion is failing for every user who requests it, and their data is being retained.
BUCKET_MEMORIES_RECORDINGSwas never wired into the backend Helm charts (it's absent fromprod_omi_backend_listen_values.yaml, the dev values, and.env.template), somemories_recordings_bucketresolves toNonein prod.delete_all_conversation_recordingscalls.bucket(None), and listing raises:That purge is a required step, so the raise aborts
background_wipe_user_databeforeusers_db.delete_user_data(). The wipe is markedwipe_failed, the reconciler re-enqueues it, and it fails again — forever. Prod logs (last 24h):Six uids are stuck in that loop. Their Firestore data (conversations, memories, chats) is still there after they asked to be deleted.
Root cause
An unconfigured bucket was being treated as a purge failure. A permanent config gap therefore became a permanent block on an irreversible-but-requested deletion.
The fix
An unconfigured bucket now means "nothing to purge" and returns cleanly — uploads resolve the same bucket name, so a deployment without it cannot have stored recordings.
A real GCS failure still raises and still blocks the irreversible Firestore wipe. That contract is deliberate (retry rather than partially wipe) and is unchanged —
test_gcs_failure_blocks_firestore_wipestill passes untouched.Verification (real path, live prod GCS, prod service account)
ValueError: Cannot determine path without bucket name.required_failuresis empty → the wipe proceeds to the Firestore delete.gs://memories-recordingsand deletes the{uid}/prefix (exercised against a uid with no objects, so nothing was deleted).backend/test.sh: 564 test files, 0 failures.AGENTS.mdrequires explicit user sign-off for data deletion changes, and this one enables an irreversible Firestore wipe that is currently blocked. I've verified it end-to-end, but I'm not merging that class of change autonomously. The blast radius is only users who already requested deletion — this does not change who gets wiped, only whether the recordings step can veto the wipe.BUCKET_MEMORIES_RECORDINGSis still missing from the Helm values. Consequences:gs://memories-recordings— if they delete their account, this fix skips their.wavfiles. (None of the 6 currently-stuck uids have objects there, so nothing is left behind for them.)upload_conversation_recording(consent-gated onstore_recording_permission) and thehas_recordingendpoint both hit the same unset name.Wiring the env var into the charts restores the purge and the feature. I left it alone because it's deploy config.
🤖 automated by hourly watchdog; opened for review, not merged.