fix(webapp): two quick votes on the same slot no longer race into a 500 (#844) #847

Merged

claude-bot merged 4 commits from fix/vote-race-and-start-commit into main 2026-09-26 16:58:02 +00:00

Conversation 0 Commits 4 Files changed 4 +199 -21

claude-bot commented 2026-09-26 15:45:45 +00:00

Collaborator

Copy link

Closes #844.

The bug: vote_service.upsert_vote read the existing vote, then inserted or updated. Two PUT …/timeslots/{id}/vote requests for the same slot and user could both see no row. The second insert then hit uq_votes_slot_user as an IntegrityError, which came back as a 500.

The fix: one postgresql.insert(Vote).on_conflict_do_update(constraint="uq_votes_slot_user", …), the same pattern as notification_settings_service and the transcript-feedback endpoint. It re-reads with populate_existing, and the router's side effects are unchanged.

Tests:

• test_votes.py::test_two_concurrent_votes_for_the_same_slot_and_user_both_succeed runs two real sessions on separate connections. It failed before the fix with the exact IntegrityError.
• 133 targeted tests pass, and ruff is clean.

Also (#845, investigated, no code change): start_by_recording already commits before the response. A new two-connection regression test proves a fresh reader sees in_progress straight away. #845 stays open, since the one J4 observation has another cause.

🤖 Generated with Claude Code

Closes #844. **The bug:** `vote_service.upsert_vote` read the existing vote, then inserted or updated. Two `PUT …/timeslots/{id}/vote` requests for the same slot and user could both see no row. The second insert then hit `uq_votes_slot_user` as an IntegrityError, which came back as a 500. **The fix:** one `postgresql.insert(Vote).on_conflict_do_update(constraint="uq_votes_slot_user", …)`, the same pattern as `notification_settings_service` and the transcript-feedback endpoint. It re-reads with `populate_existing`, and the router's side effects are unchanged. **Tests:** - `test_votes.py::test_two_concurrent_votes_for_the_same_slot_and_user_both_succeed` runs two real sessions on separate connections. It failed before the fix with the exact IntegrityError. - 133 targeted tests pass, and ruff is clean. **Also (#845, investigated, no code change):** `start_by_recording` already commits before the response. A new two-connection regression test proves a fresh reader sees `in_progress` straight away. #845 stays open, since the one J4 observation has another cause. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 3 commits 2026-09-26 15:45:45 +00:00

fix(webapp): race-safe vote upsert, and start/commit-timing evidence (#844, #845) ... 0b75d7a828

vote_service.upsert_vote raced a SELECT-then-INSERT: two PUTs for the same
slot and user could both miss the existing row and both try to insert,
so the loser hit uq_votes_slot_user as a real IntegrityError (a 500 at the
router). Replaced it with one INSERT ... ON CONFLICT DO UPDATE on that
constraint via SQLAlchemy's postgresql.insert(...).on_conflict_do_update,
keeping the same return shape and side effects.

Investigated #845 (a GET right after record/start allegedly still reading
"confirmed"): session_service.start_by_recording already calls db.commit()
itself before the router returns, rather than relying on get_db's
post-yield commit. No code change there; added a regression test with two
independent connections proving the commit lands before a fresh read.

Tests: two-connection concurrency test for the vote race (confirmed it
fails with IntegrityError against the old code) and for the start-commit
timing (passes unmodified, as evidence). ruff format/check clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

docs: drop the CHANGELOG line for #845, which changed no code ... ed9b6ee520

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: restore the Fixed heading the previous commit took out ... a1647325c4

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot referenced this pull request 2026-09-26 15:45:50 +00:00

After /record/start returns 200, GET /sessions/{id} once still read "confirmed" (D116) #845

rbrooks added 1 commit 2026-09-26 15:51:31 +00:00

Merge remote-tracking branch 'origin/main' into fix/vote-race-and-start-commit ... ff4f105463

# Conflicts:
#	CHANGELOG.md

claude-bot merged commit 131f2f082c into main 2026-09-26 16:58:02 +00:00

claude-bot referenced this pull request from a commit 2026-09-26 16:58:04 +00:00

Merge PR #847: two quick votes on the same slot no longer race into a 500 (#844)

claude-bot referenced this pull request 2026-09-26 16:58:38 +00:00

Two quick votes on the same time slot: the second is a 500 (upsert race) #844

claude-bot referenced this pull request 2026-09-26 18:16:23 +00:00

Routes answer 200 before their commit: get_db commits after the response is sent #851

rbrooks referenced this pull request from a commit 2026-09-26 19:46:55 +00:00

Merge origin/main into lane 6 (#732); re-chain b8c0d2e4f6a7 onto c4e6a8b0d2f1

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Quest-Board!847

No description provided.