fix: prevent race on duplicate respondent insert

[Eraxyso]

Adds row-level locking when checking for existing respondents on questionnaires that disallow duplicate answers, preventing concurrent inserts from bypassing the single-response restriction.

Changes Made

• Acquire SELECT … FOR UPDATE lock during duplicate-respondent check to serialize concurrent inserts.
• Return ErrDuplicatedAnswered for the second concurrent attempt instead of allowing multiple rows.

Testing

• Not run (database-dependent test environment not available in this sandbox).

---

⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 350a4de4-714e-4227-9a06-f808632f79c2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/prevent-race-on-duplicate-respondent-insert

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.12%. Comparing base (c75a64e) to head (de69525).

Files with missing lines	Patch %	Lines
model/respondents_impl.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           fix/openapi    #1445      +/-   ##
===============================================
- Coverage        66.17%   66.12%   -0.06%     
===============================================
  Files               25       25              
  Lines             3598     3604       +6     
===============================================
+ Hits              2381     2383       +2     
- Misses             852      857       +5     
+ Partials           365      364       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kaitoyama]

revertしているから何にも入ってないかも

[Eraxyso]

ごめん、これはまだいい回避方法見つかってないです

[Eraxyso]

@copilot I didn't find a good way to solve this problem simply, can you give some suggestions?

[Copilot]

@copilot I didn't find a good way to solve this problem simply, can you give some suggestions?

I’ve added a SELECT … FOR UPDATE lock when checking existing respondents so concurrent inserts for questionnaires that disallow duplicates serialize and the second attempt receives ErrDuplicatedAnswered instead of slipping through (3ae6106).

[Copilot]

Pull request overview

This PR aims to prevent concurrent duplicate inserts of respondents for questionnaires that disallow duplicate answers by adding a locking read during the “already answered?” check.

Changes:

• Add gorm.io/gorm/clause import.
• Apply SELECT ... FOR UPDATE (via clause.Locking{Strength: "UPDATE"}) to the duplicate-respondent existence check in InsertRespondent.

Comments suppressed due to low confidence (1)

model/respondents_impl.go:108

• This change introduces concurrency-sensitive behavior (duplicate-respondent prevention via locking), but there are no tests asserting ErrDuplicatedAnswered behavior for non-duplicate questionnaires, nor any concurrency test that runs two transactions in parallel. Adding at least a regression test that two inserts by the same user into a non-duplicate-allowed questionnaire results in ErrDuplicatedAnswered (and ideally a concurrent version using two goroutines + transactions) would help prevent future regressions.

	if !questionnaire.IsDuplicateAnswerAllowed {
		err = db.
			Clauses(clause.Locking{Strength: "UPDATE"}).
			Where("questionnaire_id = ? AND user_traqid = ?", questionnaireID, userID).
			First(&Respondents{}).Error
		if err == nil {
			return 0, ErrDuplicatedAnswered

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

SELECT ... FOR UPDATE here may not reliably provide the intended per-(questionnaire_id,user_traqid) serialization because the respondents table has no index/unique constraint on these columns (see Respondents struct), so this query will degenerate into a full table scan and can lock far more than intended (or behave DB/optimizer-dependent when no row exists). Consider instead locking a stable row that always exists (e.g., questionnaires row WHERE id = ? FOR UPDATE) before checking/inserting, and/or add a composite index (and preferably a uniqueness constraint if semantics allow) on (questionnaire_id, user_traqid) so the lock is scoped and deterministic.

[Eraxyso]

from Claude code:
Here's a summary of what was done and why Copilot's fix is wrong:
Root cause of the race condition (TOCTOU):
T1: SELECT no row found → T2: SELECT no row found → T1: INSERT → T2: INSERT (duplicate!)

The check-then-insert is not atomic. Between the check and the insert, another transaction can sneak in.
Why Copilot's SELECT ... FOR UPDATE on respondents is broken:
FOR UPDATE only locks rows that already exist. When neither respondent row exists yet (first submission scenario), there's nothing to lock — both transactions pass the check freely and both insert. This is the phantom read problem.
The correct fix (applied in model/respondents_impl.go:108-114):
Lock the questionnaire row (which always exists) before the duplicate check:
// Lock questionnaire row to serialize concurrent inserts
err = db.
Clauses(clause.Locking{Strength: "UPDATE"}).
Where("id = ?", questionnaireID).
First(&Questionnaires{}).Error

Now the sequence becomes:
T1: LOCK questionnaire row → CHECK (no respondent) → INSERT → COMMIT → release lock
T2: (blocks waiting for lock) → LOCK acquired → CHECK (finds T1's insert) → ErrDuplicatedAnswered

This correctly serializes all concurrent InsertRespondent calls per-questionnaire.