Repository navigation
Stop task actions and invitation emails deadlocking with a member leaving - #29
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…oing Completing, skipping, reopening, handing over and undoing a task locked the task first and needed the commune later, when the foreign key on the occurrence or activity line they add takes a key-share lock on it. A member leaving locks the commune and then their tasks, so the two could wait on each other until PostgreSQL cancelled one as a deadlock. 0037 patches the three functions in place, as 0027 did for edits and removals: the task's commune is read without a lock and locked, then the task (for undo, the task through its occurrence) is locked and read as before. Every existing check and message is unchanged, and each patch asserts that it applied and that the commune is the first row locked. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
The lock order test now holds a departure between its commune and member locks while Casey completes, skips, hands over or undoes, as it already did for an edit and a removal. Each race must finish without a deadlock, the departure must succeed, and the command either goes ahead on the task as the departure left it or is refused with its usual message. Each session now ends its transaction as soon as its statement settles, so a deadlock fails the test in about a second rather than after a statement timeout. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
The invitation email sender locked the invitation first and needed the commune later, when the foreign key on the delivery row it adds takes a key-share lock on it. An admin leaving, removed or made a member locks the commune and then withdraws their invitations, so the two could wait on each other until PostgreSQL cancelled the sender as a deadlock. The sender now takes a key-share lock on the invitation's commune, found by the token hash, in its own statement first. It then waits for such a change to finish and refuses with the existing "Invitation is unavailable" 404. Accepting an invitation and inviting the same address again already lock the commune before the invitation, so the order now matches them too. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
The invitations test now holds an admin's removal between its commune and member locks while the real sendInvitationEmail runs for that admin's invitation, so the interleaving happens every time. Neither side may fail as a deadlock, the removal must succeed, and the sender must be refused with the existing 404, leaving no delivery row and never calling the mocked provider. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
…notes Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
EmotiveImpact
force-pushed
the
claude/app-review-batch-4
branch
from
October 9, 2026 16:50
9627da6 to
beb52ba
Compare
EmotiveImpact
force-pushed
the
claude/app-review-task-locks
branch
from
October 9, 2026 16:50
f42da1d to
79cdd9b
Compare
Owner
Author
|
The Vercel preview for Generated by Claude Code |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before: if a member left while someone completed, skipped, reopened, handed over or undid one of their tasks, the two could wait on each other until PostgreSQL cancelled one as a deadlock, so either the task action or the departure failed. Emailing an invitation could do the same with an admin who was leaving, being removed or being made a member.
After: each waits for the other to finish. A task action then goes ahead on the task as the departure left it or is refused with its usual message, and an invitation email whose invitation was withdrawn meanwhile is refused with the existing "Invitation is unavailable" message, with nothing sent.
This follows up batch 1, where editing and removing a task were changed to lock the commune first (0027). It is built on #28 (batch 4), so review that first; the diff here shows only this change.
What changed
Releasing
Nothing hosted was touched. Migration 0037 is local only, follows 0036 and needs no client change. The sender change ships with the next reminder worker deploy, in either order with 0037.
How it was checked
task_lock_order.mjsholds a member's departure between its commune and member locks while each of the five task commands runs, as it already did for edits and removals. Each race must finish without a deadlock, the departure must succeed, and the command either goes ahead or is refused with its usual message. Each session ends its transaction as soon as its statement settles, so a deadlock fails the test in about a second.run-local.sh(2 skipped as they need a Neon branch), 18 local API suites, 458 web tests, the worker's tests, typechecks for the web app, local API and worker, and a production build.How
Only the start of each function changes, so each is patched with
pg_get_functiondefandreplace(), and the migration stops if a definition no longer matches. A task never moves to another commune, so the commune read before locking is still the task's once the locks are held. Nothing else locks a task or an occurrence before its commune: adding a task locks the commune first, and the snapshot and search only read.🤖 Generated with Claude Code
https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
Generated by Claude Code