Concurrent issue creation assigns duplicate numbers, then 500s on read
Background
Discovered while bulk-creating test issues in parallel (40 concurrent
muse hub issue create calls) for issue #139's layout verification: 20
issues landed with badly duplicated numbers — 13 issues numbered #3, 5
numbered #2, 2 numbered #1, none numbered higher despite 20 rows
existing.
Root cause, traced in code:
_next_issue_number() (musehub/services/musehub_issues.py:104-110):
async def _next_issue_number(session: AsyncSession, repo_id: str) -> int:
"""Return the next sequential issue number for the given repo (1-based)."""
stmt = select(func.max(MusehubIssue.number)).where(
MusehubIssue.repo_id == repo_id
)
current_max: int | None = (await session.execute(stmt)).scalar_one_or_none()
return (current_max or 0) + 1
Classic read-then-write race: two concurrent create_issue() calls can both
read the same MAX(number) before either commits, so both compute and
insert the same number. Nothing prevents this — ix_musehub_issues_repo_number
(musehub_social_models.py:49) is a plain index, not a UniqueConstraint.
The duplicate then breaks reads. get_issue()
(musehub/services/musehub_issues.py:239-258) does:
row = (await session.execute(stmt)).scalar_one_or_none()
.scalar_one_or_none() raises sqlalchemy.exc.MultipleResultsFound when more
than one row matches (repo_id, number) — unhandled, surfaces as a bare
500 to the client. Confirmed live: opening any of the duplicated issues in
the browser threw exactly this (htmx logged Response Status Error Code 500 from /gabriel/musehub/issues/3).
Impact
Low likelihood in normal single-user usage (issue creation is rarely truly concurrent), but real: any race — two agents filing issues at the same moment, a retry-on-timeout double-submit, a script bulk-creating issues in parallel — can silently corrupt numbering and then 500 on every subsequent read of the affected issue(s) until manually fixed in the DB.
Fix
- Add a DB-level
UniqueConstraint("repo_id", "number")onmusehub_issues(Alembic migration) — turns silent duplication into a loud, retryable integrity error instead of corrupted data. - Make
_next_issue_number+ insert atomic under that constraint: catch the integrity error and retry with a freshMAX+1, or move number assignment to a DB-side mechanism (e.g. a per-repo sequence/counter row updated withSELECT ... FOR UPDATE, or an atomicINSERT ... ON CONFLICTloop) so concurrent creates serialize instead of racing. - Defensively,
get_issue(and any other.scalar_one_or_none()call keyed on(repo_id, number)) should use.first()orLIMIT 1so a pre-existing duplicate degrades to "shows one of them" instead of a hard 500 — belt-and-suspenders, not a substitute for fixing the constraint.
Repro
# Fire N concurrent creates against the same repo; inspect resulting numbers
for i in $(seq 1 20); do
muse hub issue create --title "race test $i" --hub <hub-url> --json &
done
wait
muse hub issue list --hub <hub-url> --state all --limit 200 --json \
| jq -r '.issues[].number' | sort -n | uniq -c
# Expect: no count > 1. Currently: heavy duplication.