feat(profile-identity): profiles could be soft-deleted but never restored - #361
Conversation
…ored
ADR-0058 §A. `sql/003` gave `awcms_profiles` `restored_at`/`restored_by` and an
index on `(tenant_id, deleted_at)`; `party-directory.ts` exported
`softDeleteParty` with nothing to undo it. No code path in the repo could write
either column, so a soft-deleted profile was permanent — while
`profile_management.restore` sat seeded in the catalogue, granted to every
tenant owner, and enforced by nothing.
`restoreParty` + `POST /api/v1/profiles/{id}/restore` (defineTenantRoute,
guarded, audited, Idempotency-Key required).
Three shape decisions, each mutation-proven:
- the precondition is the `WHERE … deleted_at IS NOT NULL`, never a read before
the write. A read-then-write reads identically at review and lets two
concurrent restores both succeed and audit two restorations of one profile.
Probed against real Postgres: the second UPDATE reports `UPDATE 0`;
- `delete_reason` survives. Clearing it alongside `deleted_at` looks tidier and
destroys the only record of why the profile was deleted; `restored_at` is
what says the deletion no longer holds;
- "no such profile" and "that profile is not deleted" answer ONE 404. A
distinguishable answer turns this route into an oracle for which profile ids
exist, in the module that masks identifiers everywhere else.
There is no 23505 path, and that is a schema property rather than luck:
`awcms_profiles` has no unique constraint at all, and the partial unique a
restore would normally collide with is on `awcms_profile_identifiers`, which
`softDeleteParty` never cascaded to.
Also corrects ADR-0058: its first edition accused `profile-identity/README.md`
of documenting a `merge` guard. The README is correct — that sentence was in
the enforcement gate's own exception text, fixed in #359. Quoting a note about
a file instead of the file is the same mechanism that kept two false
accusations alive in §1, so the correction is recorded rather than silently
edited.
Coverage: 201/205 with 4 exceptions -> 202/205 with 3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6bccd3673
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!restored) { | ||
| return fail( | ||
| 404, | ||
| "RESOURCE_NOT_FOUND", | ||
| "Profile not found, or is not soft-deleted." |
There was a problem hiding this comment.
When two requests concurrently restore the same profile with the same Idempotency-Key, both can initially find no key; the first transaction restores and records the response, while the second waits on the profile row and then gets zero updated rows because deleted_at is already null. This branch immediately returns 404 instead of replaying the now-committed idempotency record, so a duplicate submission receives a false failure despite the required idempotency contract. Recheck the key after a zero-row update or claim it before performing the mutation.
AGENTS.md reference: AGENTS.md:L86-L86
Useful? React with 👍 / 👎.
| if (!profileId) { | ||
| return fail(400, "VALIDATION_ERROR", "Profile id is required."); | ||
| } |
There was a problem hiding this comment.
For a nonempty malformed path such as /api/v1/profiles/not-a-uuid/restore, this presence-only check passes and restoreParty compares the UUID column to invalid input, causing PostgreSQL 22P02; withTenant rethrows that error, so the route produces a server error rather than the documented validation response. Validate the UUID format before opening the mutation path, as the analogous media-object restore route does.
AGENTS.md reference: AGENTS.md:L89-L89
Useful? React with 👍 / 👎.
1 participant
ADR-0058 §A — yang pertama dari empat. Nol migrasi.
Lubangnya
sql/003memberiawcms_profileskolomrestored_at/restored_bydan index(tenant_id, deleted_at).party-directory.tsmengeksporsoftDeleteParty— tanpa pasangan. Tidak ada jalur kode mana pun di repo ini yang bisa menulis kedua kolom itu, jadi profil yang di-soft-delete permanen, sementaraprofile_management.restoreduduk di katalog, ter-grant ke tiap owner tenant, dan tak digerbangi apa pun.Bentuk yang sama dengan yang ADR-0056 §B tutup untuk objek media.
Tiga keputusan bentuk, ketiganya mutation-proven
WHERErestoreParty puts its precondition in the WHEREdelete_reason = NULLikut dibersihkanrestoreParty clears the deletion but keeps its reasonmissing and not-deleted answer the SAME 404Prasyarat di
WHERE, bukan baca-lalu-tulis. Baca-dulu terbaca identik saat review, lolos setiap functional test, dan membuat dua restore bersamaan sama-sama lanjut lalu menulis dua baris audit untuk satu pemulihan. Diprobe ke Postgres nyata:UPDATEkedua melaporUPDATE 0.delete_reasondipertahankan. Membersihkannya bersamadeleted_atterlihat lebih rapi dan menghancurkan satu-satunya catatan mengapa profil itu dihapus.restored_atyang menyatakan penghapusan tak lagi berlaku — bukan hilangnya alasan.Satu 404 untuk dua sebab. "Profil tak ada" dan "profil itu tidak terhapus" menjawab sama. Jawaban yang bisa dibedakan menjadikan rute ini oracle untuk id profil mana yang eksis — di modul yang me-mask identifier di mana-mana.
Tidak ada jalur
23505, dan itu properti skema bukan keberuntunganRefleks yang benar untuk sebuah restore adalah mencurigai unique parsial. Diverifikasi ke skema:
awcms_profilestak punya satu pun constraint unique; index parsial… WHERE deleted_at IS NULLada padaawcms_profile_identifiers, dansoftDeletePartytak pernah cascade ke sana — identifier profil terhapus tetap hidup, jadi tak ada yang perlu di-insert ulang. Dicatat di kode dan di ADR agar tak ditemukan ulang sebagai kejutan.Koreksi terhadap ADR-0058 edisi pertama
ADR itu menuduh
profile-identity/README.mdmendokumentasikan guardmerge. README-nya benar ({read,create,update,delete}) — kalimat "merge" ada di teks alasan pengecualian gate, dan sudah dikoreksi di #359. Saya mengutip catatan gate lalu mengatribusikannya ke README tanpa membuka README-nya.Dikoreksi sebagai catatan di dalam ADR, bukan diedit diam-diam, karena ini contoh ketiga dari pola yang sama dalam satu rangkaian kerja: teks salah dari sebuah gate menjadi sumber dokumen berikutnya, lalu terbaca sebagai temuan independen — persis mekanisme yang membuat dua tuduhan palsu §1 bertahan. Aturannya: kutip berkas, bukan catatan tentang berkas.
Verifikasi
check+buildhijau di host.awcms_appPostgres lokal,28P01; job Integration CI menjalankannya hijau). Delta pass tepat +6, yakni test baru file ini, nol regresi.del=t/res=f, setelah restoredel=f/res=tdengandelete_reasonutuh, restore keduaUPDATE 0.Cakupan permission: 201/205 dengan 4 pengecualian → 202/205 dengan 3.
🤖 Generated with Claude Code