feat(hutch): self-serve account deletion (App Store 5.1.1(v))

[FagnerMartinsBrack]

Why

The iOS app supports in-app account creation, so App Store Guideline 5.1.1(v) requires users to be able to initiate account deletion from within the app. Neither the app nor readplace.com offered self-serve deletion — the privacy policy told users to email, which Apple explicitly rejects. This adds end-to-end self-serve deletion.

What

Server pipeline (mirrors the existing export / cancel-subscription workers):

• DeleteAccountCommand + publisher + EventBridge/in-memory wrappers.
• A DLQ-backed, idempotent delete-account worker that scrubs every user-owned store: auth row + Gmail uniqueness claim, sessions, OAuth grants (+ paired refresh rows), saved articles, digest queue, reader-ready state, iOS onboarding, inbox emails/links (+ their RAW_EMAIL and CONTENT S3 objects) and forwarding addresses, S3 exports, password-reset tokens, and a live Stripe subscription + customer. Founding-member (no row) and trialing (no subscriptionId) accounts are first-class branches with no Stripe call. Per-user EventBridge schedules are deleted so none fires at a dead account.
• Synchronous POST /account/delete teardown so the account is unusable the instant the user confirms: destroy every session → revoke every OAuth token → clear the session cookie → publish the command → full-page redirect to the logged-out home.
• Per-store bulk delete/revoke primitives beside each store, mirrored in the in-memory fixtures, unit-tested against the fake-DynamoDB-client pattern.

Clients:

• Siren: a class field on SirenAction, a new account resource with a bare destructive delete-account action, and an account discovery link on the collection.
• Web: an /account "Danger zone" behind an hx-confirm, plus a privacy-policy rewrite pointing at the self-serve path.
• iOS: a delete-account presentation case, the destructive-confirm generalized from row controls to any destructive collection affordance (keyed on isDestructive, never the action name), and forceLogout after a successful account-ending invoke.

Also:

• Real Apple-grant revocation — Sign in with Apple was released to all users (feat(auth): release Sign in with Apple to all users #933) while this PR was in review, so the originally planned no-op seam shipped its Phase 6 in-PR: the code exchange now requires and persists Apple's refresh_token on the users row (stored at signup and refreshed on every returning Apple login, which backfills accounts minted before persistence), and the delete worker revokes it via POST https://appleid.apple.com/auth/revoke with a freshly minted ES256 client secret — throwing on non-2xx so the SQS record redrives (queue maxReceiveCount raised to 12 to ride out an Apple outage). Password/Google accounts have no stored token and skip the call.
• A webhook-race fix: a late SubscriptionCancelled event on a just-deleted account is now an idempotent no-op instead of poisoning the queue into the DLQ.

Product decisions (plan defaults — please confirm)

#	Decision	Applied
D1	Inbox forwarding addresses	Tombstone — PII stripped, hash kept reserved so it can never be re-minted and leak another user's mail
D2	Stripe customer	Deleted (cancels + detaches all cards); Stripe still retains invoices as an immutable tax ledger
D3	Raw .eml audit trail	Deleted on closure
D4	Password-reset tokens	Scan-deleted by email (that table has no TTL, unlike its siblings)
D5	Confirmation	Native / hx-confirm destructive confirm; no forced re-auth for v1
D6	PITR + deletion-protection retention	Documented as out-of-band governance

Testing

• Provider primitives: unit tests (fake DynamoDBDocumentClient) + mirrored in-memory fixtures.
• Worker: 7-case handler test — full scrub, active/trialing/founding billing branches, idempotency (run twice), batchItemFailures on a store error, IdP-revocation seam.
• Route + Siren + danger zone: *.route.test.ts (supertest + parseHTML).
• iOS: presentation-case + endsSession tests. Note: Swift can't be compiled in this Linux CI environment — the iOS changes need an iOS-side (make test) run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzGzig54ovfhxFHM9FyG3

---

Generated by Claude Code

[FagnerMartinsBrack]

@claude CI has FAILED. Use your Read tool to read the local file .github/workflows/claude-PR-CI-failure-fixer.md for instructions. The file is in the checked-out repository.

Context

• Attempt: 1 of 5
• Run ID: 28746111876
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Branch: claude/account-deletion-app-store-rasc6i

Failed Jobs

• web-tests: Run pnpm check

[claude]

Claude finished @FagnerMartinsBrack's task in 16m 31s —— View job

---

CI Failure Fix — Attempt 1 ✅

• Read PR commit history to understand intent
• Pull failed CI logs (web-tests: Run pnpm check)
• Identify root cause
• Implement fix
• Verify with pnpm check
• Commit & push

Root cause

CI compiles the PR merged into main's tip (refs/remotes/pull/940/merge → Merge b7771b75 into 59d3328b), not the PR head alone. Main had advanced by 6 commits since this branch forked — including #871 "Add 'Last crawled at' bookmark to article readers", which added new events: {} fixtures to queue.reader.route.test.ts and view.route.test.ts. Those fixtures predate this PR's new EventsBundle.publishDeleteAccountCommand field, so they list every property except that one (jumping straight from publishExportUserDataCommand to publishCancelSubscriptionCommand).

This PR makes publishDeleteAccountCommand required. Git merged the two sides cleanly as text, but the merged tree failed to type-check — a semantic merge conflict:

queue.reader.route.test.ts:106 - error TS2741: Property 'publishDeleteAccountCommand' is missing … but required in type 'EventsBundle'.
view.route.test.ts:144         - error TS2741: Property 'publishDeleteAccountCommand' is missing … but required in type 'EventsBundle'.

The ENOENT: … disposable-email-domains.txt later in the log was a red herring — a cascade from tsc failing before copy-static-assets.js could run.

Fix

Merged origin/main into the branch (clean, no textual conflicts) and added the one missing line to each of the two fixtures #871 introduced, in canonical EventsBundle order:

publishExportUserDataCommand: fixture.events.publishExportUserDataCommand,
publishDeleteAccountCommand: fixture.events.publishDeleteAccountCommand,   // ← added
publishCancelSubscriptionCommand: fixture.events.publishCancelSubscriptionCommand,

I did not weaken EventsBundle (e.g. make the field optional) — the required field is the point of the PR, and every other call site already wires it.

Verification

pnpm check — full pass locally:

NX Successfully ran target check for 37 projects and 48 tasks they depend on

hutch:test-with-coverage 2891 tests passed · coverage thresholds met · pre-commit hook passed.

Pushed b7771b75..5ba5cf72 (merge commit c0af98e4 + fix 5ba5cf72). CI will re-test the new merge, which now type-checks.
· branch claude/account-deletion-app-store-rasc6i

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Commit: 5ba5cf7

[claude]

Claude finished @FagnerMartinsBrack's task in 8m 49s —— View job

---

Code Review — self-serve account deletion (App Store 5.1.1(v))

Reviewed the server pipeline, the synchronous teardown, the per-store primitives, the infra wiring, and the Siren/iOS/web clients against the repo guidelines. This is a large, carefully-built change: the worker's idempotency model is mostly sound, the webhook-race fix is precise, IAM is tightly scoped, and the test suite is genuinely thorough. Two idempotency gaps in the async scrub are worth addressing because both can leave a deletion stuck or incomplete, which is exactly what this compliance feature must not do.

High Priority Issues (Must Fix)

None found.

Medium Priority Issues

1. Inbox .eml / body S3 objects are orphaned (PII not erased) on a partial or redriven scrub

In delete-account-handler.ts processCommand, the inbox step deletes the DynamoDB email rows first and only then the S3 objects:

const { receivedAtMessageIds, rawEmailS3Keys, bodyS3Keys } = await deps.deleteAllInboxEmails(userId); // deletes rows, returns keys
await deps.deleteAllInboxLinks(userId, receivedAtMessageIds);
await deps.deleteRawEmailObjects(rawEmailS3Keys);   // ← only reachable if the line above didn't throw
await deps.deleteEmailContentObjects(bodyS3Keys);

deleteAllEmailsByUserId (dynamodb-inbox-email.ts:68) reads rawEmailS3Key/bodyS3Key out of the rows and deletes those rows in the same pass, returning the keys only on full success. The S3 keys (inbound/<messageId>, content/<…>) have no userId prefix, so they are discoverable only through those rows.

If anything between that call and the two S3 deletes fails — deleteAllInboxLinks throwing on a transient DynamoDB error, a Lambda timeout, or the record redriving — the retry re-runs deleteAllEmailsByUserId, finds the rows already gone, returns empty arrays, and the raw .eml (full message content) plus rendered body sit in S3 permanently, unreferenced. That directly violates the handler's own stated invariant ("re-running against a partially scrubbed account must converge") and, for a deletion feature, silently retains user PII.

Every other store converges on redrive (articles/digest/onboarding are keyed by userId; exports re-list from S3 by prefix), so this ordering is the one exception. Fix: delete the S3 objects before the rows that hold their keys — read the keys via query, delete the S3 objects, then delete the email + link rows — so a redrive re-derives the keys instead of losing them. Fix this →

2. Stripe re-cancel on redrive can poison the queue into the DLQ

The billing block runs cancelStripeSubscription → deleteStripeCustomer → deleteSubscription (local row):

if (subscription.subscriptionId) {
    await deps.cancelStripeSubscription({ subscriptionId: subscription.subscriptionId });
}
if (subscription.customerId) {
    await deps.deleteStripeCustomer({ customerId: subscription.customerId });
}
await deps.deleteSubscription({ userId });

If cancelStripeSubscription succeeds but deleteStripeCustomer then fails (a network call — plausible), the record redrives. On redrive findSubscriptionByUserId still returns the row (the local delete never ran), so cancelImmediately fires again on a subscription that is already canceled but still exists in Stripe (immediate-cancel does not delete the object). cancelImmediately (stripe-subscriptions.ts:60) treats only HTTP 404 as idempotent success; Stripe returns 400 ("subscription already canceled") for a repeat cancel, and the test at stripe-subscriptions.test.ts:92 confirms a 400 throws. So the retry throws forever → alerting DLQ, deletion never completes.

The idempotency test only exercises the redrive where the first run fully deleted the local row (so the second run skips billing), so this interleaving isn't covered. Cleanest fix: delete the Stripe customer before cancelling the subscription — customer deletion cascades cancellation, and because every row with a subscriptionId also has a customerId (upsertActive sets both), the subsequent cancelImmediately then hits a deleted customer and 404s through the existing idempotent path. (Alternatively, treat an already-canceled subscription as success in cancelImmediately.) Fix this →

Low Priority Suggestions

1. iOS forceLogout() runs even if the delete invoke failed

In ReadingListView.swift confirmDestructive, the endsSession branch calls session.forceLogout() unconditionally after await viewModel.invokeCollection(action). If the POST /account/delete fails (network/500), the user is still logged out locally with no error surfaced, then can log back into an account that still exists. Consider gating forceLogout() on a successful invoke so a failed attempt keeps the user in place with the error banner. (Noted in the PR that Swift isn't compiled in this CI.)

2. POST /account/delete has no error path

The synchronous teardown (account.page.ts:337) has no try/catch — a throw in destroyUserSessions/revokeAllUserOAuthTokens 500s the request. The comment explains the sessions-before-tokens ordering is a deliberate safe-retry choice, and it's consistent with /oauth/revoke, so this is fine as-is; just flagging that the user sees a raw 500 rather than a friendly retry. (CSRF on this destructive POST is adequately covered — the hutch_sid cookie is SameSite=Lax, so a cross-site POST carries no session and requireAuth bounces it; consistent with every other account POST.)

This PR has issues that should be addressed before merging.
· branch claude/account-deletion-app-store-rasc6i

[FagnerMartinsBrack]

Code Review — self-serve account deletion (App Store 5.1.1(v))

Reviewed the server pipeline, the synchronous teardown, the per-store primitives, the infra wiring, and the Siren/iOS/web clients against the repo guidelines. This is a large, carefully-built change: the worker's idempotency model is mostly sound, the webhook-race fix is precise, IAM is tightly scoped, and the test suite is genuinely thorough. Two idempotency gaps in the async scrub are worth addressing because both can leave a deletion stuck or incomplete, which is exactly what this compliance feature must not do.

High Priority Issues (Must Fix)

None found.

Medium Priority Issues

1. Inbox .eml / body S3 objects are orphaned (PII not erased) on a partial or redriven scrub

In delete-account-handler.ts processCommand, the inbox step deletes the DynamoDB email rows first and only then the S3 objects:

const { receivedAtMessageIds, rawEmailS3Keys, bodyS3Keys } = await deps.deleteAllInboxEmails(userId); // deletes rows, returns keys
await deps.deleteAllInboxLinks(userId, receivedAtMessageIds);
await deps.deleteRawEmailObjects(rawEmailS3Keys);   // ← only reachable if the line above didn't throw
await deps.deleteEmailContentObjects(bodyS3Keys);

deleteAllEmailsByUserId (dynamodb-inbox-email.ts:68) reads rawEmailS3Key/bodyS3Key out of the rows and deletes those rows in the same pass, returning the keys only on full success. The S3 keys (inbound/<messageId>, content/<…>) have no userId prefix, so they are discoverable only through those rows. If anything between that call and the two S3 deletes fails (a transient DynamoDB error in deleteAllInboxLinks, a Lambda timeout, or the record redriving), the retry finds the rows already gone, returns empty arrays, and the raw .eml plus rendered body sit in S3 permanently, unreferenced — violating the handler's stated convergence invariant and retaining user PII. Fix: delete the S3 objects before the rows that hold their keys.

2. Stripe re-cancel on redrive can poison the queue into the DLQ

The billing block runs cancelStripeSubscription → deleteStripeCustomer → deleteSubscription. If cancel succeeds but deleteStripeCustomer then fails, the record redrives; findSubscriptionByUserId still returns the row, so cancelImmediately fires again on an already-canceled but still-existing subscription. cancelImmediately treats only HTTP 404 as idempotent success, and Stripe returns 400 for a repeat cancel (stripe-subscriptions.test.ts:92 confirms 400 throws), so the retry throws forever → DLQ, deletion never completes. The idempotency test doesn't cover this interleaving. Cleanest fix: delete the Stripe customer before cancelling (customer deletion cascades cancellation, and every row with a subscriptionId has a customerId), so the follow-up cancel 404s through the existing idempotent path.

Low Priority Suggestions

1. iOS forceLogout() runs even if the delete invoke failed

In ReadingListView.swift:142 confirmDestructive, the endsSession branch calls session.forceLogout() unconditionally after invokeCollection(action). A failed POST /account/delete still logs the user out locally with no error surfaced. Consider gating forceLogout() on success. (Swift isn't compiled in this CI.)

2. POST /account/delete has no error path

The synchronous teardown (account.page.ts:337) has no try/catch — a throw 500s the request. The sessions-before-tokens ordering is a deliberate safe-retry choice consistent with /oauth/revoke, so this is fine as-is; just flagging the raw 500. CSRF is adequately covered — the hutch_sid cookie is SameSite=Lax, so cross-site POSTs carry no session and requireAuth bounces them.

[FagnerMartinsBrack]

@claude Fix the review issues. Use your Read tool to read the local file .github/workflows/claude-PR-code-review-auto-apply.md for instructions. The file is in the checked-out repository.

Context

• Attempt: 1 of 5
• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Branch: claude/account-deletion-app-store-rasc6i

Review to Address

See the review comment above this one for the issues to fix.

[claude]

Claude finished @FagnerMartinsBrack's task in 28m 0s —— View job

---

Fixing Review Issues — Attempt 1 ✅

Both Medium priority issues are fixed and pushed as cd906eeb. (The two Low-priority suggestions are intentionally out of scope per the auto-apply instructions — fix High/Medium only.)

• M1 — Inbox .eml / body S3 objects orphaned on partial/redriven scrub
• M2 — Stripe re-cancel on redrive poisons the queue into the DLQ
• Provider + handler tests updated, + redrive tests proving both fixes
• pnpm check — full pass (37 projects / 48 tasks, coverage thresholds met, functions 100%)
• Committed & pushed

---

M1 — S3 PII no longer orphaned on redrive

deleteAllEmailsByUserId both read the S3 keys out of the email rows and deleted those rows in one pass, returning the keys only on full success. Because the S3 keys (inbound/…, content/…) carry no userId, the email rows are their only index — so any failure between that call and the S3 deletes (a transient error, a Lambda timeout, a redrive) lost the keys and left the raw .eml + rendered body in S3 permanently.

Fix: split the email store into two passes and reorder the scrub so the index is deleted last:

• listDeletionReferencesByUserId — read-only, runs first, rows survive.
• deleteAllEmailsByUserId — now returns void, runs last, after the S3 objects and link rows it indexes are gone.

A redrive re-reads the surviving rows and re-derives the keys instead of orphaning them. This also closes the same latent gap for link rows (no userId index), which the old order shared.

M2 — Stripe re-cancel DLQ poison eliminated

The review's suggested reorder (delete customer then cancel) still re-runs cancelImmediately on the redrive, so it only works if a post-deletion cancel returns 404. I verified Stripe's docs instead: deleting a customer "immediately cancels any active subscriptions" and "prevents any further operations to be performed". So re-canceling after the customer is deleted is exactly a "further operation" Stripe blocks — the reorder rests on an unverified 404 assumption.

Fix (fewer assumptions, provably idempotent): delete the Stripe customer — which cascades the cancellation and detaches every card — and remove the redundant explicit cancelStripeSubscription. That explicit re-cancel was the sole source of the poison. deleteCustomer is already 404-idempotent, so a redrive converges. The invariant that makes this safe holds in code: upsertActive sets subscriptionId and customerId together, so every sub we'd cancel belongs to a customer we delete. cancelImmediately stays defined + unit-tested (no runtime consumer, same as origin/main) for future use.

Tests added (the interleavings the review noted were uncovered)

• M1: a raw-email S3 delete that throws once → first delivery fails with rows intact → redrive re-derives the keys and deletes the objects (not orphaned), then removes the rows.
• M2: a local-row delete that throws once → redrive re-runs the billing block → customer delete fires again idempotently, no re-cancel, account fully scrubbed.

Verification

pnpm check — full pass: 37 projects / 48 tasks, hutch unit suite 2896 passed, coverage functions 100% · branches 97.21% · statements 99.53% · lines 99.53%. Pre-commit hook passed.
· branch claude/account-deletion-app-store-rasc6i · commit cd906eeb

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Commit: cd906ee

[claude]

Claude finished @FagnerMartinsBrack's task in 7m 55s —— View job

---

Code Review — self-serve account deletion (App Store 5.1.1(v)) · re-review at cd906eeb

Re-reviewed the two medium issues from the prior pass and swept the fix commit for regressions. Both mediums are fixed correctly, and the fixes are genuinely sound rather than plausible-but-fragile — I traced each convergence path and confirmed the external Stripe behavior the fix rests on.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verification of the two prior mediums (now resolved):

M1 — S3 .eml/body PII no longer orphaned on redrive. The email store is split into a read-only listDeletionReferencesByUserId (runs first, rows survive) and a void deleteAllEmailsByUserId (runs last). The scrub is reordered so every S3 object and link row is deleted before the email rows that index them (delete-account-handler.ts:113-119). Because all S3 objects for all emails are removed before any email row is deleted, a mid-scrub failure (transient error, Lambda timeout, redrive) leaves the rows intact and the redrive re-derives the keys — it converges instead of orphaning PII. This also closes the same latent gap for link rows (no userId index). The in-memory fixture mirrors the read-only/void split exactly (in-memory-inbox-email.ts:24-41), and the new "redrive after a failed raw-email S3 delete…" test proves the rows survive the first failure and the keys are re-derived (not orphaned) on retry.

M2 — Stripe re-cancel DLQ poison eliminated. The explicit cancelStripeSubscription is removed; the billing block is now just deleteStripeCustomer (when customerId present) → deleteSubscription (delete-account-handler.ts:90-96). I confirmed the three things this rests on:

• Cascade + idempotency (Stripe docs): deleting a customer "immediately cancels any active subscriptions", and a repeat DELETE on an already-deleted customer "raises a 404" — which deleteCustomer already treats as success, so the redrive converges.
• Code invariant: upsertActive sets customerId and subscriptionId together while upsertTrialing removes both, so every row carrying a live Stripe sub also carries the customerId we delete — no orphaned subscription for founding (no row) or trialing (no customerId) accounts.
• Webhook race: the customer deletion cascade-fires a customer.subscription.deleted webhook that lands on a row deleteSubscription may have just removed; handle-subscription-cancelled-handler.ts:28-40 now catches the ConditionalCheckFailedException and no-ops it. Complements the fix cleanly.

The "redrive after a failed local subscription delete…" test proves deleteCustomer runs twice with no re-cancel and the account fully scrubs.

Low Priority Suggestions

Both carried over from the prior review, both non-blocking. Noting them for completeness — neither is a merge blocker.

1. iOS forceLogout() still runs even if the delete invoke failed

ReadingListView.swift:142-146 confirmDestructive calls session.forceLogout() unconditionally after await viewModel.invokeCollection(action). A failed POST /account/delete (offline/500) still logs the user out locally with no error surfaced; they can then log back into an account that still exists. Gating forceLogout() on a successful invoke would keep a failed attempt in place with the error banner. (Swift isn't compiled in this CI, so this needs an iOS-side check.) Fix this →

2. POST /account/delete surfaces a raw 500 on a transient teardown failure

account.page.ts:337-349 has no try/catch, unlike the sibling /reactivate and /subscribe routes. I confirmed Express is 5.2.1, so an async throw auto-forwards to logAndRespondOnError (a clean 500 JSON — it does not hang), and the deliberate sessions-before-tokens ordering leaves the account safely retryable rather than half-torn-down. So this is fine as-is; only flagging that the user sees a raw 500 rather than a friendly retry on the rare transient failure. CSRF is adequately covered (the hutch_sid cookie is SameSite=Lax, so a cross-site POST carries no session and requireAuth bounces it). Fix this →

cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· branch claude/account-deletion-app-store-rasc6i

[FagnerMartinsBrack]

Code Review — self-serve account deletion (App Store 5.1.1(v)) · re-review at cd906eeb

Re-reviewed the two medium issues from the prior pass and swept the fix commit for regressions. Both mediums are fixed correctly, and the fixes are genuinely sound rather than plausible-but-fragile — I traced each convergence path and confirmed the external Stripe behavior the fix rests on.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verification of the two prior mediums (now resolved):

M1 — S3 .eml/body PII no longer orphaned on redrive. The email store is split into a read-only listDeletionReferencesByUserId (runs first, rows survive) and a void deleteAllEmailsByUserId (runs last). The scrub is reordered so every S3 object and link row is deleted before the email rows that index them (delete-account-handler.ts:113-119). Because all S3 objects for all emails are removed before any email row is deleted, a mid-scrub failure leaves the rows intact and the redrive re-derives the keys — it converges instead of orphaning PII. This also closes the same latent gap for link rows (no userId index). The in-memory fixture mirrors the read-only/void split exactly, and the new redrive test proves the rows survive the first failure and the keys are re-derived on retry.

M2 — Stripe re-cancel DLQ poison eliminated. The explicit cancelStripeSubscription is removed; the billing block is now just deleteStripeCustomer (when customerId present) → deleteSubscription. Confirmed the three things this rests on:

• Cascade + idempotency (Stripe docs): deleting a customer immediately cancels active subscriptions, and a repeat DELETE on a deleted customer 404s — which deleteCustomer already treats as success, so the redrive converges.
• Code invariant: upsertActive sets customerId and subscriptionId together while upsertTrialing removes both, so every row carrying a live Stripe sub also carries the customerId we delete.
• Webhook race: the cascade-fired customer.subscription.deleted webhook landing on a just-deleted row is now caught (ConditionalCheckFailedException → no-op) in handle-subscription-cancelled-handler.ts:28-40.

The redrive test proves deleteCustomer runs twice with no re-cancel and the account fully scrubs.

Low Priority Suggestions

Both carried over from the prior review, both non-blocking.

1. iOS forceLogout() still runs even if the delete invoke failed

ReadingListView.swift:142-146 calls session.forceLogout() unconditionally after invokeCollection(action). A failed POST /account/delete still logs the user out locally with no error surfaced. Gating on a successful invoke would keep a failed attempt in place with the error banner. (Swift isn't compiled in this CI.)

2. POST /account/delete surfaces a raw 500 on a transient teardown failure

account.page.ts:337-349 has no try/catch, unlike sibling routes. Express is 5.2.1, so an async throw auto-forwards to logAndRespondOnError (a clean 500 — not a hang), and the sessions-before-tokens ordering leaves the account safely retryable. Fine as-is; only flagging the raw 500 vs a friendly retry. CSRF is adequately covered (SameSite=Lax).

[FagnerMartinsBrack]

@claude — I reviewed this PR strictly for App Store 5.1.1(v) compliance completeness and for scope (that it does nothing more than Apple compliance).

Verdict: the web deletion path works and satisfies 5.1.1(v) — in-app Account link → authenticated in-app webview → /account "Danger zone" → real hard-delete (closeUserAccount deletes the row, not a soft flag) + full synchronous server teardown, and the privacy policy's email-only-deletion language is removed. No hard submission blocker today. But please address the following before this is truly done and scoped.

🔴 Compliance robustness

1. Sign in with Apple revocation is a no-op — latent rejection trap.
SIWA is dark-launched (the Apple button is gated behind ?feature=apple, and nothing in production ever sets it — web/feature-toggle.ts, auth.page.ts:159/235; feature=apple appears only in tests), so a reviewer never sees SIWA and Apple's SIWA-deletion requirement isn't triggered for this submission. The trap: delete-account/revoke-external-idp-tokens.ts is a log-only no-op wired at delete-account.main.ts:119, and providers/apple-auth/apple-token.ts:5 parses only id_token so Apple's refresh_token is discarded at sign-in. The moment SIWA is un-gated (button shown by default, or the iOS app starts sending feature=apple) without fixing this, in-app deletion silently leaves the app in the user's Settings → Sign in with Apple list → rejection under the exact guideline this PR targets.

• Preferred (small, in-scope): make it fail-closed — add a guard/test asserting "if SIWA is reachable by a normal user ⟹ revokeExternalIdpTokens is the real Apple /auth/revoke impl and exchangeAppleCode persists the refresh_token." It turns CI red the day SIWA un-gates without revocation.
• Or implement now: persist Apple's refresh_token in apple-token.ts, POST https://appleid.apple.com/auth/revoke (reuse the existing apple-client-secret.ts JWT) in a real RevokeExternalIdpTokens, wire it at delete-account.main.ts:119 (treat already-revoked as success for SQS redrive idempotency).
• Also fix the false comment at App/AffordancePresentation.swift:120 ("The server has already revoked everything server-side") — it hasn't, for Apple.

🟠 Scope — the one thing that does MORE than compliance

2. The native iOS delete-account subsystem is unreachable dead code.
delete-account is advertised only on the /account Siren entity (api/account-siren.ts); the queue collection — the only Siren resource iOS fetches — carries just an account link (api/collection-siren.ts:44), which ToolbarRoute routes to .open → opens /account in the webview. So the whole native apparatus never fires: the delete-account AffordancePresentation case, Affordance.endsSession, the isDestructive collection branch + invokeCollection + forceLogout in ReadingListView.dispatch/confirmDestructive, the account-specific confirm message, the two AffordancePresentationTests, the delete-account action in account-siren.ts, and the new class?: string[] field on SirenAction (api/siren.ts) are all dead. The web-link path already provides in-app deletion.

• Preferred (compliance-only): delete the native apparatus and rely on the web-link path. Keep the shared row-delete confirm infra (pendingDestructive, the article confirm path, activate) — that's live.
• Or: if native in-app deletion is actually the intended UX, advertise the delete-account action on the collection so the native flow goes live, and drop the web-link route. Don't ship both half-wired.

3. No logout after the in-webview deletion completes.
On the live web path, POST /account/delete redirects the webview to /, but the app keeps its TokenStore tokens and cached reading-list UI until a later 401 (session.forceLogout() only fires on the dead native path, ReadingListView.swift:145; ReaderNavigation.decide returns .close only for readplace://reader/close, which this flow never emits). Immediately after confirming deletion the app still looks signed in — a reviewer can read that as "deletion didn't take effect in the app." Fix: emit a deterministic done-signal (e.g. redirect the post-delete page to readplace://reader/close, which ReaderNavigation already maps to .close) and have that closure call session.forceLogout() + dismiss the sheet. (Resolving #2 via the native path gives this for free.)

🟡 Data remnants (deletion must actually erase)

4. Password-reset scrub misses mixed-case emails — permanent PII leak. deleteTokensByEmail (providers/password-reset/dynamodb-password-reset.ts:87) filters email = :e where :e is the normalized (lowercased) users-table PK, but rows are written with the raw email (ForgotPasswordSchema uses a bare z.email(), no normalize; stored verbatim at dynamodb-password-reset.ts:52). A reset requested as John@Example.com is never matched, and that table has no TTL → it survives forever, defeating the exact scan this PR added. Fix: normalize on write (and in the filter); add a mixed-case integration test.

5. verification-tokens table is not scrubbed. For a signed-up-but-never-verified account, {token, userId, email} survives up to ~9 days post-deletion (TTL + DynamoDB lag) — the same remnant class you scrubbed for password-reset. Add a deleteVerificationTokensBy… (Scan by email / by userId), wire it into the worker before closeUserAccount, and add its table to the worker's IAM grant in infra/index.ts.

6. (note) pending-signups has no TTL — abandoned-checkout rows keep the deleted user's email+userId permanently; import-sessions (TTL'd) keep userId+imported URLs until expiry. Decide explicitly whether to scrub (pending-signups is a permanent leak).

7. (note) The sync route kills sessions/tokens but the identity row is deleted by the async worker, so re-login is briefly possible in the gap (self-healing). Either delete/flag the row synchronously in the route, or soften the "immediately unusable" comment to "existing sessions/tokens killed synchronously; full erasure async."

✅ Confirmed in-scope — do NOT change

The async DeleteAccountCommand worker + infra Lambda/SQS/DLQ + every per-store delete/tombstone primitive (actual erasure is required by 5.1.1(v)); the account link + SirenAction.class (the entry-point mechanism); the ~15 queue/*, view/*, save-* route-test and fixture one-line additions (compile-forced by the new required publishDeleteAccountCommand dependency and the new collection link — not gratuitous); the handle-subscription-cancelled ConditionalCheckFailed → no-op (caused by deletion racing the Stripe customer.subscription.deleted webhook — would otherwise DLQ-poison); the redirectFullPage rename; and the privacy-policy rewrite (removing the email-only-deletion language is itself a compliance requirement). The inbox-address tombstone (PII stripped, hash reserved) is a defensible non-retention choice, not soft deletion.

---

Reviewed for 5.1.1(v) completeness + scope only; findings verified against the PR head. Priorities: #1 (fail-closed the Apple trap) and #2 (resolve the dead-vs-web path into one) are the load-bearing ones; #4 is a real permanent-PII bug.

[claude]

Claude finished @FagnerMartinsBrack's task in 33m 47s —— View job

---

5.1.1(v) compliance & scope review — addressed

Verified every finding against the code and implemented the server-side data/robustness fixes. Pushed as 10b7e080. pnpm check green (37 projects / 48 tasks · hutch 2903 tests · functions 100% · all coverage thresholds met).

✅ Implemented

• Redesign login view and add logout functionality 👍 #4 🟡 Password-reset mixed-case PII leak — confirmed real: rows stored the raw address (ForgotPasswordSchema is bare z.email()) but the scrub filters the normalized users-table PK, and that table has no TTL, so a John@Example.com reset survived forever. Now normalized on write and in the delete filter (dynamodb-password-reset.ts) + mixed-case tests, mirrored in the in-memory fixture.
• Add keyboard shortcut to save current tab to reading list #5 🟡 verification-tokens not scrubbed — added deleteTokensByUserId (scan by userId, sidesteps casing), wired into the worker before closeUserAccount, added the table to the worker's DynamoDB IAM grant + env in infra/index.ts, mirrored the fixture, tested.
• feat(firefox-extension): add right-click context menu to save pages and links #6 🟡 pending-signups permanent leak — decided to scrub (it has no TTL — worse than Add keyboard shortcut to save current tab to reading list #5): added deleteByUserId, wired + IAM + env + fixture + tests. import-sessions is TTL'd, so its {userId, imported URLs} self-expires — accepting that via TTL rather than adding a scan.
• Add AWS Lambda deployment infrastructure with Pulumi IaC #1 🔴 SIWA revocation trap — added a fail-closed guard in auth.route.test.ts that pins two facts together: "SIWA reachable by a normal user" and "Apple refresh_token persisted" (probed via the now-exported AppleTokenResponse schema). The only accepted states are {reachable:false, persisted:false} (today) and {reachable:true, persisted:true}; un-gating SIWA without wiring the real POST /auth/revoke turns CI red and points at the seam. Strengthened the revoke-external-idp-tokens.ts comment and corrected the false "server has already revoked everything" Swift comment.
• feat(hutch): add URL display toggle for article cards #7 — softened the /account/delete "immediately unusable" comment to state accurately that existing sessions/tokens die synchronously while identity-row erasure is async (self-healing).

⏭️ Deferred to an iOS-side change — #2 & #3 (with exact steps)

Both require editing Swift, which this Linux CI cannot compile or test — a wrong edit ships a broken iOS build silently. I confirmed #2 is real: the queue collection carries only an account link (collection-siren.ts:44) that ToolbarRoute sends to .open, so the delete-account action on /account Siren (account-siren.ts) — and the whole native apparatus (endsSession, the isDestructive collection→forceLogout branch, the two AffordancePresentationTests) — never fires.

#2 (preferred, compliance-only) — delete the native apparatus, keep the web-link path:

• AffordancePresentation.swift: remove the case "delete-account": block and the endsSession property.
• ReadingListView.swift: fold the else if pending.affordance.endsSession { … forceLogout } branch in confirmDestructive into the plain collection-invoke else, and drop the endsSession ternary in the confirm message. Keep the generic isDestructive routing + pendingDestructive + the article/activate row path — those are live.
• AffordancePresentationTests.swift: remove the two delete-account cases.
• api/account-siren.ts (+ test): remove the delete-account action; if nothing else GETs /account as Siren, drop the wantsSiren branch too. One open call for you: the review lists SirenAction.class as both dead (Add reading list view with URL filtering to popup #2) and "in-scope, do NOT change" — keep it as a forward-looking primitive, or remove it with the action.
• Or (make native live): advertise delete-account on the collection and drop the account-link→webview route. Don't ship both half-wired.

#3 — logout after in-webview deletion (if keeping the web path): POST /account/delete redirects to /, but the webview keeps its TokenStore/UI. Emit a deterministic done-signal only for the webview (it needs webview detection — a plain browser must still get /, not a readplace:// scheme) redirecting to readplace://reader/close, which ReaderNavigation.decide already maps to .close; have that closure call session.forceLogout() + dismiss. Resolving #2 via the native path gives this for free.

I can apply the #2/#3 changes in a follow-up if you point me at an iOS-side runner (make test), or confirm the #2 direction and the SirenAction.class call.

_{· branch claude/account-deletion-app-store-rasc6i · commit 10b7e080}

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Commit: 10b7e08

[claude]

Claude finished @FagnerMartinsBrack's task in 8m 48s —— View job

---

Code Review — self-serve account deletion (App Store 5.1.1(v)) · re-review at 10b7e080

Re-reviewed the six compliance-completeness items from the prior 5.1.1(v) pass and swept the fix commit for regressions. I traced each new scrub down to its IAM grant, env pairing, handler ordering, and idempotency/convergence behaviour rather than just confirming the code is present. All five server-side items are implemented correctly and are genuinely sound. The two iOS items remain (as the author flagged) and are the only open work — both need an iOS-side compile/test this Linux CI cannot run.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verified — the five server-side compliance fixes:

#4 Password-reset mixed-case scrub. createPasswordResetToken now stores normalizeEmail(email) (dynamodb-password-reset.ts:57) and deleteTokensByEmail filters normalizeEmail(email) (:93). Because both sides normalize and the function is idempotent, the match holds regardless of the casing findEmailByUserId returns — a John@Example.com reset is now scrubbed. verifyPasswordResetToken returns the (now normalized) stored email, consistent with the normalized users-table PK, so no reset-flow regression. In-memory fixture mirrors both sides. (One forward-only caveat in Low #2.)

#5 Verification tokens. New deleteTokensByUserId scans by userId and deletes each by the token PK, correctly aliased #tk (TOKEN is a DynamoDB reserved word). Wired into the worker before closeUserAccount (delete-account-handler.ts:140) and keyed by userId — not by the soon-deleted identity row — so a redrive re-derives and converges. IAM grant (verificationTokensTable, includeIndexes:false, correct for a Scan) + DYNAMODB_VERIFICATION_TOKENS_TABLE env are both present, and the actions list already carries Scan+DeleteItem (infra/index.ts:547, 553-554).

#6 Pending-signup. Same shape: deleteByUserId scans by userId, deletes by the checkoutSessionId PK, wired before closeUserAccount (:141), IAM + env paired. import-sessions correctly left to its TTL.

#1 SIWA fail-closed guard. The new auth.route.test.ts test couples "SIWA reachable without a flag" to "AppleTokenResponse retains refresh_token" and pins both false. Because z.object strips unknown keys, the proxy reads false today and flips only when someone adds refresh_token to the schema — and the hard assertion turns CI red on any drift, including the dangerous un-gate-without-revocation state {reachable:true, persisted:false}. The seam comment and the corrected Swift comment now both tell the truth about Apple not being revoked yet. (Comment-vs-assertion nuance in Low #1.)

#7 Comment accuracy. The /account/delete doc-comment now states erasure is eventual (existing credentials die synchronously; the identity row is removed async, self-healing) rather than "immediately unusable." Accurate.

Low Priority Suggestions

1. SIWA guard comment describes a two-state acceptance the assertion doesn't implement. The comment says the accepted states are {false,false} (today) and {true,true} (SIWA live + revocation wired), but the assertion is a hard .toEqual({false,false}), so the correct {true,true} future state also fails CI. That's a defensible "revisit on any change" tripwire (the schema-field proxy is imperfect, so forcing a human touch when SIWA ships is arguably safer), but the comment over-promises. Either tighten the comment to "pins today's state; revisit when SIWA ships," or encode the coupling as expect(siwaReachableByDefault).toBe(appleRefreshTokenPersisted) — which passes for both stated states and still fails the dangerous {true,false}. Fix this →

2. Password-reset normalize is forward-only. normalizeEmail-on-write fixes new rows, but a mixed-case row written before this deploys is still stored raw and won't match the normalized scrub. Since this table has no TTL (the premise of #4), such a row for a later-deleted account persists. A one-off backfill/normalization of existing rows would fully close the leak the fix targets.

3. (iOS — needs an iOS-side change) Native delete-account apparatus is still unreachable. The queue collection carries only an account link → webview (collection-siren.ts:44), so the native delete-account action, endsSession, the isDestructive→forceLogout branch, and the two AffordancePresentationTests never fire. Either delete the native apparatus (compliance-only, keep the live web-link path) or advertise delete-account on the collection to make it live — don't ship both half-wired. Requires Swift compile/test this CI can't run; the author already documented the exact steps.

4. (iOS — needs an iOS-side change) No logout after in-webview deletion. POST /account/delete redirects the webview to / (account.page.ts:351), but the app keeps its TokenStore + cached UI until a later 401 — right after confirming deletion the app still looks signed in, which a reviewer can read as "deletion didn't take." Emit a deterministic done-signal for the webview (e.g. readplace://reader/close, which ReaderNavigation already maps to .close) and have that closure forceLogout() + dismiss. Server-only detection is fragile (a plain browser must still get /), so this also needs an iOS-side change.

cc @FagnerMartinsBrack - No high or medium priority issues found. The five server-side 5.1.1(v) items are landed and verified; the only open work is the two iOS-side items (Low #3/#4), which need an iOS compile/test this CI can't run and (for #3) a product decision. This PR is ready for human review.

_{· branch claude/account-deletion-app-store-rasc6i · commit 10b7e080}

[FagnerMartinsBrack]

Code Review — self-serve account deletion (App Store 5.1.1(v)) · re-review at 10b7e080

Re-reviewed the six compliance-completeness items from the prior 5.1.1(v) pass and swept the fix commit for regressions. I traced each new scrub down to its IAM grant, env pairing, handler ordering, and idempotency/convergence behaviour rather than just confirming the code is present. All five server-side items are implemented correctly and are genuinely sound. The two iOS items remain (as the author flagged) and are the only open work — both need an iOS-side compile/test this Linux CI cannot run.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verified — the five server-side compliance fixes:

#4 Password-reset mixed-case scrub. createPasswordResetToken now stores normalizeEmail(email) (dynamodb-password-reset.ts:57) and deleteTokensByEmail filters normalizeEmail(email) (:93). Because both sides normalize and the function is idempotent, the match holds regardless of the casing findEmailByUserId returns — a John@Example.com reset is now scrubbed. verifyPasswordResetToken returns the (now normalized) stored email, consistent with the normalized users-table PK, so no reset-flow regression. In-memory fixture mirrors both sides. (One forward-only caveat in Low #2.)

#5 Verification tokens. New deleteTokensByUserId scans by userId and deletes each by the token PK, correctly aliased #tk (TOKEN is a DynamoDB reserved word). Wired into the worker before closeUserAccount (delete-account-handler.ts:140) and keyed by userId — not by the soon-deleted identity row — so a redrive re-derives and converges. IAM grant (verificationTokensTable, includeIndexes:false, correct for a Scan) + DYNAMODB_VERIFICATION_TOKENS_TABLE env are both present, and the actions list already carries Scan+DeleteItem (infra/index.ts:547, 553-554).

#6 Pending-signup. Same shape: deleteByUserId scans by userId, deletes by the checkoutSessionId PK, wired before closeUserAccount (:141), IAM + env paired. import-sessions correctly left to its TTL.

#1 SIWA fail-closed guard. The new auth.route.test.ts test couples "SIWA reachable without a flag" to "AppleTokenResponse retains refresh_token" and pins both false. Because z.object strips unknown keys, the proxy reads false today and flips only when someone adds refresh_token to the schema — and the hard assertion turns CI red on any drift, including the dangerous un-gate-without-revocation state {reachable:true, persisted:false}. The seam comment and the corrected Swift comment now both tell the truth about Apple not being revoked yet. (Comment-vs-assertion nuance in Low #1.)

#7 Comment accuracy. The /account/delete doc-comment now states erasure is eventual (existing credentials die synchronously; the identity row is removed async, self-healing) rather than "immediately unusable." Accurate.

Low Priority Suggestions

1. SIWA guard comment describes a two-state acceptance the assertion doesn't implement. The comment says the accepted states are {false,false} (today) and {true,true} (SIWA live + revocation wired), but the assertion is a hard .toEqual({false,false}), so the correct {true,true} future state also fails CI. That's a defensible "revisit on any change" tripwire (the schema-field proxy is imperfect, so forcing a human touch when SIWA ships is arguably safer), but the comment over-promises. Either tighten the comment to "pins today's state; revisit when SIWA ships," or encode the coupling as expect(siwaReachableByDefault).toBe(appleRefreshTokenPersisted) — which passes for both stated states and still fails the dangerous {true,false}.

2. Password-reset normalize is forward-only. normalizeEmail-on-write fixes new rows, but a mixed-case row written before this deploys is still stored raw and won't match the normalized scrub. Since this table has no TTL (the premise of #4), such a row for a later-deleted account persists. A one-off backfill/normalization of existing rows would fully close the leak the fix targets.

3. (iOS — needs an iOS-side change) Native delete-account apparatus is still unreachable. The queue collection carries only an account link → webview (collection-siren.ts:44), so the native delete-account action, endsSession, the isDestructive→forceLogout branch, and the two AffordancePresentationTests never fire. Either delete the native apparatus (compliance-only, keep the live web-link path) or advertise delete-account on the collection to make it live — don't ship both half-wired. Requires Swift compile/test this CI can't run; the author already documented the exact steps.

4. (iOS — needs an iOS-side change) No logout after in-webview deletion. POST /account/delete redirects the webview to / (account.page.ts:351), but the app keeps its TokenStore + cached UI until a later 401 — right after confirming deletion the app still looks signed in, which a reviewer can read as "deletion didn't take." Emit a deterministic done-signal for the webview (e.g. readplace://reader/close, which ReaderNavigation already maps to .close) and have that closure forceLogout() + dismiss. Server-only detection is fragile (a plain browser must still get /), so this also needs an iOS-side change.

[FagnerMartinsBrack]

@claude address all low prio issues

[claude]

Claude finished @FagnerMartinsBrack's task in 28m 1s —— View job

---

Low-priority issues — three fixed, one documented follow-up

Pushed as e26ea17a. pnpm check fully green: 37 projects / 48 tasks · hutch 2901 tests · functions 100% · branches 97.21% · statements/lines 99.53%, knip/biome/tsc all pass, pre-commit hook passed.

• Low Add AWS Lambda deployment infrastructure with Pulumi IaC #1 — SIWA guard coupling
• Low Add reading list view with URL filtering to popup #2 — password-reset forward-only normalize
• Low Add dynamic icon status based on saved URLs #3 — dead native delete-account apparatus
• Low Redesign login view and add logout functionality 👍 #4 — logout after in-webview deletion — documented follow-up (needs an iOS-side change; reasoning below)

---

✅ Low #1 — SIWA fail-closed guard now matches its comment

Replaced the hard .toEqual({false,false}) with an equality coupling so the assertion implements the two-state acceptance the comment already described:

expect(siwaReachableByDefault).toBe(appleRefreshTokenPersisted);

{false,false} (today) and {true,true} (SIWA live + revocation wired) both pass; the dangerous {true,false} — un-gating SIWA without wiring Apple revocation — still turns CI red. Comment reworded to say it couples the two as an equality.

✅ Low #2 — password-reset PII closed for existing rows too (no backfill)

The review named the root cause: "this table has no TTL." Rather than a one-off operational backfill (which only a human can run and can't be CI-verified), I enabled DynamoDB TTL on the expiresAt the row already writes — the exact config its four sibling token tables (sessions, oauth, verification-tokens, import-sessions) already use. This auto-erases every pre-normalization mixed-case row (John@Example.com) within ~48h with no ops step, while the synchronous normalized scrub still erases new rows immediately. It also resolves the standing D4 quirk ("no TTL, unlike its siblings"). Commented inline because it reverses a documented decision for a non-obvious compliance reason.

✅ Low #3 — removed the dead native apparatus (your preferred, compliance-only direction)

You left this as "Preferred: delete the native apparatus… Or: make it live" and said "address all," so I took the stated preference. Deleted (server, verified here): account-siren.ts + its test and the unreached GET /account Siren branch in account.page.ts (+ its route test). Deleted (Swift): the delete-account presentation case, endsSession, and the two AffordancePresentationTests; folded the endsSession/forceLogout branch out of confirmDestructive and the confirm-message ternary.

Kept (all live / per your instructions): the web Danger Zone (account.view-model.ts + template → POST /account/delete), the account link, the generic isDestructive row/collection confirm infra (pendingDestructive, article path, activate), and SirenAction.class (your "do NOT change" — it's now a forward-looking primitive with no producer; I confirmed knip does not flag it, so keeping it stays green). Easily reverted if you actually want the native path live instead.

⚠️ The Swift edits are deletions only and I re-read all three files to confirm they're coherent, but this Linux CI cannot compile Swift — they still need an iOS-side make test.

---

⏭️ Low #4 — deterministic logout after in-webview deletion (documented, not landed)

I traced the full path: the account link opens in an in-app WKWebView (open(link:) → ReaderPresentation → ReaderWebView running ReaderNavigation.decide), so a done-signal is interceptable. But a correct fix is a coordinated iOS + server change that can't be built or tested from this Linux CI, and shipping either half alone would be exactly the "don't ship both half-wired" you called out. Today the path self-heals: POST /account/delete kills the session + revokes tokens server-side, so the app's bearer is already dead and the next API call 401s → onSessionExpired → forceLogout().

Exact steps for the iOS-side change (where make test can verify it):

1. iOS — append a platform marker when opening the account link (the account webview currently loads /account with no marker, unlike openReader's ?platform=ios), so the server can tell the webview from a plain browser at delete time.
2. Server (account.page.ts) — propagate that marker onto the Danger Zone form action, and in POST /account/delete redirectFullPage to a readplace://account/deleted signal when it's present (plain browsers still get /).
3. iOS — add a ReaderNavigationDecision case for that URL (distinct from reader/close, which must keep not logging out) that dismisses the sheet and calls session.forceLogout(); cover it in ReaderNavigationTests + an account.route.test.ts marker case.

Happy to implement #4 as its own commit if you point me at an iOS runner, or confirm you want it landed blind (Swift unverified here).

_{· branch claude/account-deletion-app-store-rasc6i · commit e26ea17a}

[FagnerMartinsBrack]

@claude apple signin is meant to be released

[claude]

Claude finished @FagnerMartinsBrack's task in 48m 28s —— View job

---

Apple revocation landed — SIWA stays live, guard green ✅

Direction taken per "apple signin is meant to be released": keep SIWA live and satisfy the fail-closed guard the honest way — wire real Apple token revocation into account deletion (not re-gate SIWA, not weaken the guard).

CI is now green on the branch (run 28795763543 — success). The guard now sees {reachable: true, persisted: true}.

What happened

While I was implementing this, you pushed an equivalent implementation yourself — c2a53a79 feat: revoke the Sign in with Apple grant at account deletion. CI merged the latest origin/main (which released SIWA via #933) and ran green against it.

My independent implementation converged on the same design as yours, which is a good sign the approach is sound:

• apple-token.ts requires Apple's refresh_token and returns it on AppleTokenResult.
• Persist it on the user row (the passwordHash precedent) — the users userId-index projects ALL, so the worker reads it by userId, and closeUserAccount scrubs it. No new storage table/IAM/env.
• Real revoke seam POSTs the token to appleid.apple.com/auth/revoke with a freshly minted ES256 client secret; absent token → no-op; revoke precedes closeUserAccount so a redrive can retry.
• APPLE_LOGIN_* env added to the delete worker Lambda.
• Server-side only (SIWA is web OAuth; iOS just opens the web flow), so no iOS change and fully CI-verifiable with the Apple API faked.

I did not overwrite your commit — my local duplicate is discarded and the branch stands on your c2a53a79. Your version also adds two nice touches mine didn't: the delete-account queue maxReceiveCount 3 → 12 (~3h of retries before the DLQ), and a dedicated dynamodb-auth.test.ts.

One optional observation (non-blocking)

initRevokeExternalIdpTokens asserts response.ok and relies on Apple returning 200 for an already-revoked token (RFC 7009) so a redrive converges. That's the documented behavior and the maxReceiveCount bump covers a transient outage. The only edge worth keeping in mind: if Apple ever answers a re-revoke with 400 invalid_grant (some report this) rather than 200, that record would redrive to the DLQ. Treating a 400 invalid_grant as idempotent success too would be belt-and-suspenders — entirely your call, and easy to add later if it ever shows up in the DLQ.

• Merge latest origin/main; reproduce CI's merged tree
• Confirm SIWA flow is server-side only (no iOS work)
• Independently implement Apple revocation; pnpm check green locally (functions 100%)
• Discover your equivalent c2a53a79 on the remote; do not overwrite it
• Adopt your version locally; confirm branch CI is green

Nothing further from me — the compliance gap is closed and the PR is unblocked.
· branch claude/account-deletion-app-store-rasc6i

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Commit: c2a53a7

[claude]

Claude finished @FagnerMartinsBrack's task in 8m 30s —— View job

---

Code Review — Apple grant revocation at account deletion · re-review at c2a53a79

The only new commit since the last green review (8423c4de) is c2a53a79, which replaces the no-op Apple-revocation seam with the real thing so the fail-closed SIWA guard can pass in the {reachable:true, persisted:true} state. I traced the change end-to-end — schema, persistence, the revoke call, handler ordering, infra/IAM, and the guard — rather than just confirming the code is present. It is correctly and completely wired, and genuinely sound.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verified — the Apple revocation is wired end-to-end:

• Exchange schema is fail-closed and safe. AppleTokenResponse now requires refresh_token (apple-token.ts:12). The exchange uses grant_type: "authorization_code" (apple-token.ts:40), for which Apple always returns a refresh_token, so requiring it does not risk breaking a legitimate login — and an exchange that somehow omits it now fails the login (tested: "rejects a token response that is missing the refresh_token") rather than minting an unrevokable account. Correct compliance trade-off.
• Persistence covers signup + every returning login (backfill). createAppleUser stores the token at signup; saveAppleRefreshToken (conditional attribute_exists(email) update, dynamodb-auth.ts) stores the freshest grant on each returning Apple login — including the existing-account and both create-race-fallback branches (apple-auth.page.ts:204/245/288), so accounts minted during the feat(auth): release Sign in with Apple to all users #933→this gap self-heal on next login. Google/password rows stay token-free (tested). Read back via findAppleRefreshTokenByUserId over userId-index, which is hashKey: userId, projectionType: "ALL" (hutch-storage.ts:112-115) — so the token is actually projected — and deleted with the row by closeUserAccount. The in-memory fixture mirrors both the conditional-update throw and the index read exactly, and every path is unit-tested (provider + fixture + route).
• Revoker converges on redrive. initRevokeExternalIdpTokens returns early for a null token (password/Google → no Apple call, tested), else POSTs https://appleid.apple.com/auth/revoke with a freshly minted ES256 client secret and token_type_hint: "refresh_token", and assert(response.ok, …) throws on non-2xx so the SQS record redrives. Apple returns 200 for an already-revoked token (RFC 7009), so redrives converge. All three branches tested, plus an end-to-end handler test proving the stored grant is revoked and the account fully scrubbed.
• Ordering keeps the token readable across redrives. Revocation (delete-account-handler.ts:148) runs before closeUserAccount (:151), so the users row — and its appleRefreshToken — survives until the very end; a mid-scrub failure or an Apple outage re-reads and re-revokes on redrive.
• Infra matches. The worker Lambda gets the four APPLE_LOGIN_* env vars (index.ts:627-630); its IAM grants usersTable with includeIndexes: true (index.ts:535) and dynamodb:Query (:552), so the userId-index read is permitted; dlqMaxReceiveCount rises 3→12 (:597, ≈3h) so an Apple outage rides out in-queue instead of hitting the email-alarmed DLQ after the default 3 attempts.
• Guard flips correctly. expect(siwaReachableByDefault).toBe(appleRefreshTokenPersisted) (auth.route.test.ts:1284) is now {true,true} and still turns CI red on the dangerous {true,false} (dropping refresh_token while SIWA stays reachable). The renamed initRevokeExternalIdpTokens has no dangling references to the old no-op.

Low Priority Suggestions

1. The guard's symmetric coupling also fails the harmless {reachable:false, persisted:true} state. .toBe couples the two as equality, so a future re-dark-launch of SIWA that keeps the token-persistence code trips CI even though a persisted-but-unused token behind a gated button is not itself a 5.1.1(v) risk. The rewritten comment frames this as "a re-dark-launch that also retires the token," i.e. it deliberately treats the two as moving together. That's a defensible tripwire (it was chosen over a one-directional assert in the e26ea17a review), so this is only a confirm-intent note — the coupling is stricter than compliance strictly requires, and that's on purpose.

2. (iOS — needs an iOS-side change, unchanged by this commit) No deterministic logout after in-webview deletion. The prior review's Low #4 remains the only substantive open item: POST /account/delete redirects the webview to / while the app keeps its TokenStore/cached UI until a user-initiated call 401s, so right after confirming deletion the app can still look signed in. This commit is entirely server-side (SIWA is web OAuth opened in Chrome; no Swift touched), so it neither introduces nor resolves that item — carry the corrected "trigger a converge/ping on web-sheet dismissal → 401 → onSessionExpired → forceLogout" spec into the iOS follow-up, verified with make test.

cc @FagnerMartinsBrack - No high or medium priority issues found. The Apple grant revocation is correctly and completely wired end-to-end (schema → persistence → revoke → redrive → infra/IAM → guard), fully covered, and the fail-closed SIWA guard now passes honestly in {true,true}. The only open item is the iOS deterministic-logout follow-up, which is out of scope for this server-side commit and needs an iOS make test. This PR is ready for human review.
· branch claude/account-deletion-app-store-rasc6i

[FagnerMartinsBrack]

Code Review — Apple grant revocation at account deletion · re-review at c2a53a79

The only new commit since the last green review (8423c4de) is c2a53a79, which replaces the no-op Apple-revocation seam with the real thing so the fail-closed SIWA guard can pass in the {reachable:true, persisted:true} state. I traced the change end-to-end — schema, persistence, the revoke call, handler ordering, infra/IAM, and the guard — rather than just confirming the code is present. It is correctly and completely wired, and genuinely sound.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verified — the Apple revocation is wired end-to-end:

• Exchange schema is fail-closed and safe. AppleTokenResponse now requires refresh_token (apple-token.ts:12). The exchange uses grant_type: "authorization_code" (apple-token.ts:40), for which Apple always returns a refresh_token, so requiring it does not risk breaking a legitimate login — and an exchange that somehow omits it now fails the login (tested: "rejects a token response that is missing the refresh_token") rather than minting an unrevokable account. Correct compliance trade-off.
• Persistence covers signup + every returning login (backfill). createAppleUser stores the token at signup; saveAppleRefreshToken (conditional attribute_exists(email) update, dynamodb-auth.ts) stores the freshest grant on each returning Apple login — including the existing-account and both create-race-fallback branches (apple-auth.page.ts:204/245/288), so accounts minted during the feat(auth): release Sign in with Apple to all users #933→this gap self-heal on next login. Google/password rows stay token-free (tested). Read back via findAppleRefreshTokenByUserId over userId-index, which is hashKey: userId, projectionType: "ALL" (hutch-storage.ts:112-115) — so the token is actually projected — and deleted with the row by closeUserAccount. The in-memory fixture mirrors both the conditional-update throw and the index read exactly, and every path is unit-tested (provider + fixture + route).
• Revoker converges on redrive. initRevokeExternalIdpTokens returns early for a null token (password/Google → no Apple call, tested), else POSTs https://appleid.apple.com/auth/revoke with a freshly minted ES256 client secret and token_type_hint: "refresh_token", and assert(response.ok, …) throws on non-2xx so the SQS record redrives. Apple returns 200 for an already-revoked token (RFC 7009), so redrives converge. All three branches tested, plus an end-to-end handler test proving the stored grant is revoked and the account fully scrubbed.
• Ordering keeps the token readable across redrives. Revocation (delete-account-handler.ts:148) runs before closeUserAccount (:151), so the users row — and its appleRefreshToken — survives until the very end; a mid-scrub failure or an Apple outage re-reads and re-revokes on redrive.
• Infra matches. The worker Lambda gets the four APPLE_LOGIN_* env vars (index.ts:627-630); its IAM grants usersTable with includeIndexes: true (index.ts:535) and dynamodb:Query (:552), so the userId-index read is permitted; dlqMaxReceiveCount rises 3→12 (:597, ≈3h) so an Apple outage rides out in-queue instead of hitting the email-alarmed DLQ after the default 3 attempts.
• Guard flips correctly. expect(siwaReachableByDefault).toBe(appleRefreshTokenPersisted) (auth.route.test.ts:1284) is now {true,true} and still turns CI red on the dangerous {true,false} (dropping refresh_token while SIWA stays reachable). The renamed initRevokeExternalIdpTokens has no dangling references to the old no-op.

Low Priority Suggestions

1. The guard's symmetric coupling also fails the harmless {reachable:false, persisted:true} state. .toBe couples the two as equality, so a future re-dark-launch of SIWA that keeps the token-persistence code trips CI even though a persisted-but-unused token behind a gated button is not itself a 5.1.1(v) risk. The rewritten comment frames this as "a re-dark-launch that also retires the token," i.e. it deliberately treats the two as moving together. That's a defensible tripwire (it was chosen over a one-directional assert in the e26ea17a review), so this is only a confirm-intent note — the coupling is stricter than compliance strictly requires, and that's on purpose.

2. (iOS — needs an iOS-side change, unchanged by this commit) No deterministic logout after in-webview deletion. The prior review's Low #4 remains the only substantive open item: POST /account/delete redirects the webview to / while the app keeps its TokenStore/cached UI until a user-initiated call 401s, so right after confirming deletion the app can still look signed in. This commit is entirely server-side (SIWA is web OAuth opened in Chrome; no Swift touched), so it neither introduces nor resolves that item — carry the corrected "trigger a converge/ping on web-sheet dismissal → 401 → onSessionExpired → forceLogout" spec into the iOS follow-up, verified with make test.

[FagnerMartinsBrack]

@claude address this low prio issue, do not report the other again: 2. (iOS — needs an iOS-side change, unchanged by this commit) No deterministic logout after in-webview deletion. The prior review's Low #4 remains the only substantive open item: POST /account/delete redirects the webview to / while the app keeps its TokenStore/cached UI until a user-initiated call 401s, so right after confirming deletion the app can still look signed in. This commit is entirely server-side (SIWA is web OAuth opened in Chrome; no Swift touched), so it neither introduces nor resolves that item — carry the corrected "trigger a converge/ping on web-sheet dismissal → 401 → onSessionExpired → forceLogout" spec into the iOS follow-up, verified with make test.

[claude]

Claude encountered an error —— View job

---

iOS follow-up — deterministic logout after in-webview account deletion

Implemented the corrected spec: re-validate the session on collection-web-sheet dismissal → 401 → onSessionExpired → forceLogout, so the app stops showing a signed-in reading list after the user deletes their account in the /account webview.

• Trace the in-webview account-deletion → dismissal path
• Implement the re-validation on collection web-sheet dismissal (iOS)
• Add iOS unit tests
• Commit & push (pre-commit pnpm check runs)

Why onDismiss (not onClose)

The /account page opens via open(link:) as a full-chrome collection webview (no ?platform=ios), so it's dismissed by swipe — which nils the .sheet(item:) binding without calling onClose (that only fires for the chromeless reader's readplace://reader/close). The deterministic hook is the sheet's onDismiss.

What I changed (projects/ios-readplace)

• ReadingListViewModel — new webSheetDismissed(): on dismissal of a collection web sheet (account/save, articleId == nil) it re-reads the queue. A still-valid session just reconciles the list; a deleted account's dead bearer fails to refresh → APIError.unauthorized → handle → onSessionExpired → forceLogout. It deliberately skips convergeWithServer's !hasPaginated guard so the 401-surfacing read fires even on a deep-scrolled list (the successful page is still not adopted while deep-scrolled, so position holds).
• Scoped via a presentedCollectionWebSheet flag set in open(link:) (true) / openReader (false), because .sheet(item:) nils the presentation before onDismiss runs. Article-reader closes stay network-free — the common read-and-close path is untouched.
• ReadingListView — .sheet(item:onDismiss:) calls viewModel.webSheetDismissed().
• Tests — 3 cases: dead session on account-sheet dismiss → logs out; still-valid session → one reconcile read, no logout; article-reader dismiss → no request, no logout.

⚠️ This Linux CI cannot compile or make test Swift. These edits are statically coherent and mirror the existing view-model test patterns, but they still need an iOS-side make test before merge.

_{· branch claude/account-deletion-app-store-rasc6i}

[FagnerMartinsBrack]

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Branch: claude/account-deletion-app-store-rasc6i

[claude]

Claude finished @FagnerMartinsBrack's task in 17m 47s —— View job

---

Merge conflicts resolved ✅

Merged origin/main into the PR branch and resolved all conflicts. pnpm check is green and the merge is pushed as 01e7ad2f.

• Read conflict-fixer instructions
• Understand PR intent from commit history
• Merge origin/main — 4 conflicts, all in subscription-providers
• Resolve each conflict preserving both feat(conversion): pre-expiry trial reminder email (R1) #939 and this PR
• Verify with pnpm check — full pass
• Commit & push

---

What conflicted and why

The only advancing change on main that overlapped was #939 feat(conversion): pre-expiry trial reminder email. It added a markTrialReminderEmailSent mutation to the same subscription-providers store this PR extended with a deleteSubscription mutation (for the account-deletion scrub). Both sides appended a sibling method/test at the same spot, so Git flagged 4 files — every other overlapping file (events.ts, infra/index.ts, account.page.ts, auth.route.test.ts, apple-auth.page.ts, server.ts) auto-merged cleanly. The two main crawl-bookmark style commits touched CSS/PNG/E2E files this PR doesn't.

Resolution — kept both sides everywhere

File	Conflict	Resolution
dynamodb-subscription-writes.ts	type sig + impl + return	kept markTrialReminderEmailSent and deleteSubscription; fixed the auto-merged doc comment seven→**eight mutations** (base was six; each PR independently bumped the text to "seven", so the textual merge missed that it's now eight)
dynamodb-subscription-providers.test.ts	interleaved describe blocks	split back into two complete describe blocks
in-memory-subscription-providers.ts	type sig + impl + return	kept both methods
in-memory-subscription-providers.test.ts	interleaved it blocks	split back into all four it blocks (2 per side)

Auto-merged contracts verified to carry both PRs' symbols: DeleteSubscription + MarkTrialReminderEmailSent, and SubscriptionRecord.trialReminderEmailSentAt. No intentional change from either side was reverted.

Merge vs. rebase

I used merge, not rebase, deliberately: this branch is already maintained with merge commits from origin/main (c0af98e4, 36981484), a rebase would re-conflict index.ts / auth.route.test.ts / account.page.ts across 3–4 replayed commits (they're each touched by multiple PR commits), and it would rewrite every commit hash the review thread references (e.g. the last review is "re-review at c2a53a79"). The merge resolves each conflict once and keeps those hashes valid. It's a clean fast-forward push — no history rewrite.

Note: the conflict-fixer prompt suggests gh pr comment for the summary, but the harness restricts me to updating this single comment, so the summary lives here.

Verification

pnpm check — full pass: Successfully ran target check for 37 projects and 48 tasks. The pre-commit hook re-ran it on commit (81/85 nx cache hits) and passed. The SIWA revocation guard, the delete-account worker, and #939's trial-reminder pipeline all compile and test green together.
· branch claude/account-deletion-app-store-rasc6i · commit 01e7ad2f

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Commit: 01e7ad2

[claude]

Claude finished @FagnerMartinsBrack's task in 7m 14s —— View job

---

Code Review — self-serve account deletion (App Store 5.1.1(v)) · re-review at 01e7ad2f

• Read reviewer instructions
• Establish what changed since the last green review (c2a53a79) — the origin/main merge (feat(conversion): pre-expiry trial reminder email (R1) #939) + confirmed the iOS follow-up never landed
• Review the merge-conflict resolution (subscription-providers)
• Sweep the merge for semantic-merge gaps (per-user resources feat(conversion): pre-expiry trial reminder email (R1) #939 introduced)
• Post review

The last green review was at c2a53a79; the only new commit since is the merge 01e7ad2f, which brings in #939 feat(conversion): pre-expiry trial reminder email. I focused on the merge: the 4-file conflict resolution and — the higher-risk class — whether #939 introduced any new per-user resource the delete worker must now also scrub. It did, and the worker misses it.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

1. Merge gap — the new trial-reminder per-user schedule (#939) is not scrubbed by the delete-account worker

The merge added a fourth per-user EventBridge schedule, trial-reminder-${userId} (aws-trial-scheduler.ts:206 createTrialReminderSchedule), created for every trialing user at signup / trial-start (auth.page.ts:352, apple-auth.page.ts:327, google-auth.page.ts:261, account.page.ts:395).

The delete-account worker deletes only three schedules (delete-account-handler.ts:104-106 — trial-end, deferred-cancellation, trial-feedback), and its comment asserts the invariant:

// Delete every per-user schedule so a later fire can't dispatch a command at a deleted account. All three are ResourceNotFound-idempotent.

After the merge there are four, and the reminder is the uncovered one — so both the handler's stated invariant and the PR-body goal ("Per-user EventBridge schedules are deleted so none fires at a dead account") no longer hold. The handler test wires/asserts only the same three, so nothing catches it.

Impact is bounded but real. A user who deletes their account while trialing leaves an orphaned trial-reminder-<userId> schedule (its Name and Input carry the userId) live at AWS until its natural fire time. It does not email the deleted user and does not poison the queue — processReminder (send-trial-feedback-email-handler.ts:148-155) no-ops when the subscription row is gone (deleted synchronously by deleteSubscription), and the schedule self-deletes on fire (ActionAfterCompletion: "DELETE"). But a compliance-driven deletion feature leaving a user-keyed resource behind is exactly the gap this PR exists to close (the same reason the author explicitly added the verification-token and pending-signup scrubs), and the handler's own comment now misstates the count.

The obligation is already established in-tree: cancel-subscription-handler.ts:84 deletes this very schedule on a trialing cancel — "A cancelling trialist must not receive the pre-expiry subscribe nudge … deleting the schedule is defence-in-depth plus scheduler hygiene." Account deletion is the strictly stronger case; the merge just didn't carry the new schedule into the pre-#939 delete worker.

Fix is small — no IAM change. The worker's scheduler:DeleteSchedule grant (infra/index.ts:269-270) is a group-level wildcard (…:schedule/<group>*), so deleting the reminder schedule is already permitted. Add deleteTrialReminderSchedule: DeleteTrialReminderSchedule to DeleteAccountHandlerDependencies, call await deps.deleteTrialReminderSchedule({ userId }) alongside the other three, correct the comment to "all four", wire trialScheduler.deleteTrialReminderSchedule at delete-account.main.ts:159, and extend the handler test (wire + assert the reminder delete, including on the redrive cases). Fix this →

Merge-conflict resolution itself is correct — dynamodb-subscription-writes.ts carries both markTrialReminderEmailSent (#939) and deleteSubscription (this PR), the doc-comment correctly reads "eight mutations", the in-memory fixture mirrors both, and the interleaved describe/it blocks in the two test files are cleanly split. No leftover conflict markers anywhere. The trialReminderEmailSentAt column #939 adds lives on the subscription row, which deleteSubscription already removes — so the schedule above is the only new user-owned resource the merge left unscrubbed.

Low Priority Suggestions

1. (iOS — needs an iOS-side change) Deterministic logout after in-webview deletion is still open

The webSheetDismissed follow-up from 2026-07-06 never landed (its job errored before pushing; git grep confirms it's absent from the tree), so this remains the one open non-blocker: POST /account/delete redirects the webview to / while the app keeps its TokenStore + cached reading-list UI until a user-initiated call 401s. No server data is at risk (every call 401s), but right after confirming deletion the app can still look signed in. The corrected spec — trigger a converge/authenticated ping on web-sheet dismissal → 401 → existing onSessionExpired → forceLogout — needs a coordinated iOS+server change verified with make test, which this Linux CI can't run.

(Per the maintainer's 2026-07-06 instruction, not re-reporting the SIWA symmetric-coupling confirm-intent note.)

This PR has issues that should be addressed before merging.
· branch claude/account-deletion-app-store-rasc6i · commit 01e7ad2f

[FagnerMartinsBrack]

High Priority Issues (Must Fix)

None found

Medium Priority Issues

1. Merge gap — the new trial-reminder per-user schedule (#939) is not scrubbed by the delete-account worker

The last green review was at c2a53a79; the only new commit is the merge 01e7ad2f, which brings in #939 feat(conversion): pre-expiry trial reminder email. That merge added a fourth per-user EventBridge schedule, trial-reminder-${userId} (aws-trial-scheduler.ts:206 createTrialReminderSchedule), created for every trialing user at signup / trial-start (auth.page.ts:352, apple-auth.page.ts:327, google-auth.page.ts:261, account.page.ts:395).

The delete-account worker deletes only three schedules (delete-account-handler.ts:104-106 — trial-end, deferred-cancellation, trial-feedback), and its comment asserts: "Delete every per-user schedule so a later fire can't dispatch a command at a deleted account. All three are ResourceNotFound-idempotent." After the merge there are four, and the reminder is the uncovered one — so both the handler's invariant and the PR-body goal ("Per-user EventBridge schedules are deleted so none fires at a dead account") no longer hold. The handler test wires/asserts only the same three, so nothing catches it.

Impact is bounded but real. A user who deletes their account while trialing leaves an orphaned trial-reminder-<userId> schedule (its Name and Input carry the userId) live at AWS until its natural fire time. It does not email the deleted user and does not poison the queue — processReminder (send-trial-feedback-email-handler.ts:148-155) no-ops when the subscription row is gone (deleted synchronously by deleteSubscription), and the schedule self-deletes on fire (ActionAfterCompletion: "DELETE"). But a compliance-driven deletion feature leaving a user-keyed resource behind is exactly the gap this PR exists to close (same rationale as the verification-token and pending-signup scrubs the author added), and the handler's comment now misstates the count.

The obligation is already established in-tree: cancel-subscription-handler.ts:84 deletes this very schedule on a trialing cancel — "A cancelling trialist must not receive the pre-expiry subscribe nudge … deleting the schedule is defence-in-depth plus scheduler hygiene." Account deletion is the strictly stronger case; the merge just didn't carry the new schedule into the pre-#939 delete worker.

Fix is small — no IAM change. The worker's scheduler:DeleteSchedule grant (infra/index.ts:269-270) is a group-level wildcard (…:schedule/<group>*), so deleting the reminder schedule is already permitted. Add deleteTrialReminderSchedule: DeleteTrialReminderSchedule to DeleteAccountHandlerDependencies, call await deps.deleteTrialReminderSchedule({ userId }) alongside the other three, correct the comment to "all four", wire trialScheduler.deleteTrialReminderSchedule at delete-account.main.ts:159, and extend the handler test (wire + assert the reminder delete, including on the redrive cases).

Note — the merge-conflict resolution itself is correct. dynamodb-subscription-writes.ts carries both markTrialReminderEmailSent (#939) and deleteSubscription (this PR), the doc-comment correctly reads "eight mutations", the in-memory fixture mirrors both, the interleaved describe/it blocks are cleanly split, and there are no leftover conflict markers. The trialReminderEmailSentAt column #939 adds lives on the subscription row, which deleteSubscription already removes — so the schedule above is the only new user-owned resource the merge left unscrubbed.

Low Priority Suggestions

1. (iOS — needs an iOS-side change) Deterministic logout after in-webview deletion is still open

The webSheetDismissed follow-up from 2026-07-06 never landed (its job errored before pushing; git grep confirms it's absent from the tree), so this remains the one open non-blocker: POST /account/delete redirects the webview to / while the app keeps its TokenStore + cached reading-list UI until a user-initiated call 401s. No server data is at risk (every call 401s), but right after confirming deletion the app can still look signed in. The corrected spec — trigger a converge/authenticated ping on web-sheet dismissal → 401 → existing onSessionExpired → forceLogout — needs a coordinated iOS+server change verified with make test, which this Linux CI can't run.

(Per the maintainer's 2026-07-06 instruction, not re-reporting the SIWA symmetric-coupling confirm-intent note.)

[FagnerMartinsBrack]

@claude Fix the review issues. Use your Read tool to read the local file .github/workflows/claude-PR-code-review-auto-apply.md for instructions. The file is in the checked-out repository.

Context

• Attempt: 3 of 5
• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Branch: claude/account-deletion-app-store-rasc6i

Review to Address

See the review comment above this one for the issues to fix.

[claude]

Claude finished @FagnerMartinsBrack's task in 17m 28s —— View job

---

Fixing Review Issues — Attempt 3 ✅

Pushed as fb3d8bb7. The review at 01e7ad2f found one Medium priority issue — now fixed. (Per the auto-apply instructions I fix High/Medium only; the Low iOS deterministic-logout item is out of scope — it needs a coordinated iOS+server change this Linux CI can't make test.)

• M1 — delete-account worker now scrubs the trial-reminder-<userId> schedule (feat(conversion): pre-expiry trial reminder email (R1) #939 merge gap)
• Verify facts (contract type, scheduler impl, sibling obligation, IAM wildcard)
• Add deleteTrialReminderSchedule to handler deps + call site + comment "all four"
• Wire it at the composition root (delete-account.main.ts)
• Extend the handler test (branch asserts + redrive-convergence assertion)
• pnpm check full pass
• Commit & push

---

M1 — the trial-reminder per-user schedule is now deleted on account closure

The origin/main merge (01e7ad2f) brought in #939, which added a fourth per-user EventBridge schedule — trial-reminder-${userId}, created for every trialing user at signup/trial-start. The delete-account worker deleted only the original three (trial-end, deferred-cancellation, trial-feedback), and its comment asserted "All three are ResourceNotFound-idempotent" — so an account deleted mid-trial left an orphaned, user-keyed reminder schedule live at AWS until its natural fire time. That's exactly the user-owned remnant this compliance PR exists to erase (same rationale as the verification-token / pending-signup scrubs), and the handler's own comment misstated the count.

Fix (delete-account-handler.ts, delete-account.main.ts):

• Added deleteTrialReminderSchedule: DeleteTrialReminderSchedule to DeleteAccountHandlerDependencies and the await deps.deleteTrialReminderSchedule({ userId }) call alongside the other three schedule deletes.
• Corrected the comment All three → All four.
• Wired trialScheduler.deleteTrialReminderSchedule at the composition root.

No IAM change — verified the worker's scheduler:DeleteSchedule grant (cancelSubscriptionSchedulerManagePolicy, attached at index.ts:638) is a group-level wildcard (…:schedule/<group>*), which already permits the reminder schedule name. The DeleteTrialReminderSchedule contract type and the deleteTrialReminderSchedule impl already existed (added by #939); the sibling cancel-subscription-handler.ts:84 already deletes this same schedule for a trialing cancel, so account deletion is the strictly stronger case.

Tests (delete-account-handler.test.ts):

• Wired the reminder delete into the in-memory subject (new trialReminderCalls collector).
• Asserted it fires in the trialing branch (title updated all three → all four) and the founding-member branch, mirroring the existing three.
• Added a redrive-convergence assertion to the raw-email redrive test: the schedule deletes run before the injected S3 failure, so the reminder delete fires on both the failed first delivery and the redrive ([userId, userId]), proving it's ResourceNotFound-idempotent and leaves no orphan after a retry.

Verification

pnpm check — full pass: 37 projects / 48 tasks, hutch coverage functions 100% · branches 97.05% · statements/lines 99.49%. The pre-commit hook re-ran the full check on commit and passed.

_{· branch claude/account-deletion-app-store-rasc6i · commit fb3d8bb7}

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat(hutch): self-serve account deletion (App Store 5.1.1(v)) #940
• Commit: fb3d8bb

[claude]

Claude finished @FagnerMartinsBrack's task in 4m 50s —— View job

---

Code Review — self-serve account deletion (App Store 5.1.1(v)) · re-review at fb3d8bb7

• Read reviewer instructions
• Establish what changed since the last review (01e7ad2f → fb3d8bb7)
• Review the trial-reminder schedule scrub fix (M1 from last review)
• Verify handler ordering / idempotency / test coverage
• Sweep for regressions
• Post review

The only new commit since the last review (01e7ad2f) is fb3d8bb7, which resolves the one Medium from that pass — the trial-reminder-<userId> schedule (#939) left unscrubbed by the delete worker. I traced the fix down to its contract, impl idempotency, handler ordering, IAM grant, and the exhaustiveness of the schedule set rather than just confirming the line is present. The fix is correct, complete, and sound. CI is green on this commit (run 28832966885).

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verified — the trial-reminder schedule is now scrubbed:

• Wired end-to-end. DeleteTrialReminderSchedule is added to DeleteAccountHandlerDependencies and called at delete-account-handler.ts:109 alongside the other three, and the composition root passes trialScheduler.deleteTrialReminderSchedule (delete-account.main.ts:160). The stale // All three … comment is corrected to // All four … (:105).
• Idempotent on redrive. deleteTrialReminderSchedule (aws-trial-scheduler.ts:236-250) swallows ResourceNotFoundException exactly like its three siblings, so the comment's "all four are ResourceNotFound-idempotent" holds and an at-least-once redrive converges.
• Ordering is right. The four schedule deletes run early (:106-109), before the inbox/store scrub, so a mid-scrub failure re-runs them harmlessly on the retry — consistent with the handler's stated convergence invariant.
• Exhaustive — no fifth schedule missed. There are exactly four per-user schedule-name helpers (trial-end, cancel-, trial-feedback-, trial-reminder-), and the worker now deletes all four; the reminder was the only gap the feat(conversion): pre-expiry trial reminder email (R1) #939 merge introduced.
• No IAM change needed — verified. The worker's scheduler:DeleteSchedule grant is a group-level wildcard …:schedule/<group>* (infra/index.ts:269-270), which already matches trial-reminder-<userId>. The obligation is also established in-tree: the sibling cancel-subscription-handler.ts:84 deletes this same schedule on a trialing cancel; account deletion is the strictly stronger case.
• Test coverage is complete. The trialing branch (title updated all three→all four) and the founding-member branch both assert trialReminderCalls === [userId], and the raw-email redrive test asserts [userId, userId] — proving the delete fires on both the failed first delivery and the redrive and is idempotent. Both handler construction sites (production + test) carry the new required dep, so nothing compiles missing it.

Low Priority Suggestions

1. (iOS — needs an iOS-side change) Deterministic logout after in-webview deletion is still the one open item

Unchanged by this server-only commit and carried over: POST /account/delete redirects the webview to / while the app keeps its TokenStore + cached reading-list UI until a user-initiated call 401s, so right after confirming deletion the app can still look signed in. No server data is at risk (every call 401s). The corrected follow-up spec — trigger a converge/authenticated ping on web-sheet dismissal → 401 → existing onSessionExpired → forceLogout — needs a coordinated iOS+server change verified with make test, which this Linux CI can't run.

(Per the maintainer's 2026-07-06 instruction, not re-reporting the SIWA symmetric-coupling confirm-intent note.)

cc @FagnerMartinsBrack — No high or medium priority issues found. The #939 merge gap is closed correctly and the only open work is the iOS deterministic-logout follow-up, which needs an iOS make test this CI can't run. This PR is ready for human review.
· branch claude/account-deletion-app-store-rasc6i · commit fb3d8bb7

[FagnerMartinsBrack]

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Verified — the trial-reminder schedule scrub fix (fb3d8bb7, the M1 from the last review at 01e7ad2f):

• Wired end-to-end. DeleteTrialReminderSchedule is added to DeleteAccountHandlerDependencies and called at delete-account-handler.ts:109 alongside the other three schedule deletes, and the composition root passes trialScheduler.deleteTrialReminderSchedule (delete-account.main.ts:160). The stale // All three … comment is corrected to // All four … (delete-account-handler.ts:105).
• Idempotent on redrive. deleteTrialReminderSchedule (aws-trial-scheduler.ts:236-250) swallows ResourceNotFoundException exactly like its three siblings, so an at-least-once redrive converges.
• Ordering is right. The four schedule deletes run early (:106-109), before the inbox/store scrub, so a mid-scrub failure re-runs them harmlessly on retry — consistent with the handler's convergence invariant.
• Exhaustive — no fifth schedule missed. Exactly four per-user schedule-name helpers exist (trial-end, cancel-, trial-feedback-, trial-reminder-), and the worker now deletes all four; the reminder was the only gap the feat(conversion): pre-expiry trial reminder email (R1) #939 merge introduced.
• No IAM change needed — verified. The worker's scheduler:DeleteSchedule grant is a group-level wildcard …:schedule/<group>* (infra/index.ts:269-270), which already matches trial-reminder-<userId>. The sibling cancel-subscription-handler.ts:84 deletes this same schedule on a trialing cancel; account deletion is the strictly stronger case.
• Test coverage complete. The trialing and founding-member branches both assert trialReminderCalls === [userId], and the raw-email redrive test asserts [userId, userId] — proving the delete fires on both the failed first delivery and the redrive and is idempotent. Both handler construction sites (production + test) carry the new required dep. CI is green on the commit (run 28832966885).

Low Priority Suggestions

1. (iOS — needs an iOS-side change) Deterministic logout after in-webview deletion is still the one open item

Unchanged by this server-only commit and carried over: POST /account/delete redirects the webview to / while the app keeps its TokenStore + cached reading-list UI until a user-initiated call 401s, so right after confirming deletion the app can still look signed in. No server data is at risk (every call 401s). The corrected follow-up spec — trigger a converge/authenticated ping on web-sheet dismissal → 401 → existing onSessionExpired → forceLogout — needs a coordinated iOS+server change verified with make test, which this Linux CI can't run.

(Per the maintainer's 2026-07-06 instruction, not re-reporting the SIWA symmetric-coupling confirm-intent note.)