fix: close race conditions in script builder session and slug creation
- script_builder endpoint: pg_advisory_xact_lock on user_id before session count check, preventing concurrent creates from both passing the MAX_SESSIONS_PER_USER guard - script_builder_service send_message: pg_advisory_xact_lock on session_id before message count check, preventing concurrent sends from both passing the MAX_MESSAGES_PER_SESSION guard - script_builder_service save_to_library: replace check-then-insert slug logic with IntegrityError retry loop (3 attempts with fresh UUID suffix); add unique constraint on script_templates.slug (migration 070) - ScriptBuilderPage: add creatingSessionRef to serialize concurrent handleSend calls that would otherwise both call createSession() while session is still null Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,29 @@
|
|||||||
|
"""add unique constraint to script_templates.slug
|
||||||
|
|
||||||
|
Revision ID: 070
|
||||||
|
Revises: 069
|
||||||
|
Create Date: 2026-04-01
|
||||||
|
"""
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
|
||||||
|
revision = "070"
|
||||||
|
down_revision = "069"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.create_unique_constraint(
|
||||||
|
"uq_script_templates_slug",
|
||||||
|
"script_templates",
|
||||||
|
["slug"],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_constraint(
|
||||||
|
"uq_script_templates_slug",
|
||||||
|
"script_templates",
|
||||||
|
type_="unique",
|
||||||
|
)
|
||||||
@@ -3,6 +3,7 @@ from typing import Annotated
|
|||||||
from uuid import UUID
|
from uuid import UUID
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||||
|
from sqlalchemy import text
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.core.database import get_db
|
from app.core.database import get_db
|
||||||
@@ -67,6 +68,12 @@ async def create_session(
|
|||||||
current_user: Annotated[User, Depends(get_current_active_user)],
|
current_user: Annotated[User, Depends(get_current_active_user)],
|
||||||
) -> ScriptBuilderSessionDetail:
|
) -> ScriptBuilderSessionDetail:
|
||||||
"""Start a new Script Builder session."""
|
"""Start a new Script Builder session."""
|
||||||
|
# Acquire per-user advisory lock so concurrent create requests are serialized.
|
||||||
|
# Without this, two simultaneous requests both read count < limit and both
|
||||||
|
# insert, exceeding MAX_SESSIONS_PER_USER.
|
||||||
|
user_lock_key = hash(str(current_user.id)) % (2**62)
|
||||||
|
await db.execute(text("SELECT pg_advisory_xact_lock(:key)"), {"key": user_lock_key})
|
||||||
|
|
||||||
# Enforce max concurrent sessions
|
# Enforce max concurrent sessions
|
||||||
count = await script_builder_service.count_user_sessions(db, current_user.id)
|
count = await script_builder_service.count_user_sessions(db, current_user.id)
|
||||||
if count >= MAX_SESSIONS_PER_USER:
|
if count >= MAX_SESSIONS_PER_USER:
|
||||||
|
|||||||
@@ -48,7 +48,7 @@ class ScriptTemplate(Base):
|
|||||||
UUID(as_uuid=True), ForeignKey("users.id", ondelete="SET NULL"), nullable=True
|
UUID(as_uuid=True), ForeignKey("users.id", ondelete="SET NULL"), nullable=True
|
||||||
)
|
)
|
||||||
name: Mapped[str] = mapped_column(String(200), nullable=False)
|
name: Mapped[str] = mapped_column(String(200), nullable=False)
|
||||||
slug: Mapped[str] = mapped_column(String(200), nullable=False, index=True)
|
slug: Mapped[str] = mapped_column(String(200), nullable=False, unique=True, index=True)
|
||||||
description: Mapped[Optional[str]] = mapped_column(Text, nullable=True)
|
description: Mapped[Optional[str]] = mapped_column(Text, nullable=True)
|
||||||
use_case: Mapped[Optional[str]] = mapped_column(Text, nullable=True)
|
use_case: Mapped[Optional[str]] = mapped_column(Text, nullable=True)
|
||||||
script_body: Mapped[str] = mapped_column(Text, nullable=False)
|
script_body: Mapped[str] = mapped_column(Text, nullable=False)
|
||||||
|
|||||||
@@ -5,7 +5,8 @@ from datetime import datetime, timezone
|
|||||||
from typing import Optional
|
from typing import Optional
|
||||||
from uuid import UUID
|
from uuid import UUID
|
||||||
|
|
||||||
from sqlalchemy import select, func
|
from sqlalchemy import select, func, text
|
||||||
|
from sqlalchemy.exc import IntegrityError
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
from sqlalchemy.orm import selectinload
|
from sqlalchemy.orm import selectinload
|
||||||
|
|
||||||
@@ -169,6 +170,12 @@ async def send_message(
|
|||||||
user_content: str,
|
user_content: str,
|
||||||
) -> ScriptBuilderMessageResponse:
|
) -> ScriptBuilderMessageResponse:
|
||||||
"""Send a user message and get AI response with generated script."""
|
"""Send a user message and get AI response with generated script."""
|
||||||
|
# Acquire per-session advisory lock to prevent concurrent message count races.
|
||||||
|
# Two simultaneous sends to the same session would otherwise both read the same
|
||||||
|
# count, both pass the limit check, and both insert — exceeding the cap.
|
||||||
|
session_lock_key = hash(str(session.id)) % (2**62)
|
||||||
|
await db.execute(text("SELECT pg_advisory_xact_lock(:key)"), {"key": session_lock_key})
|
||||||
|
|
||||||
# Count existing messages for the session
|
# Count existing messages for the session
|
||||||
msg_count_result = await db.execute(
|
msg_count_result = await db.execute(
|
||||||
select(func.count(ScriptBuilderMessage.id)).where(
|
select(func.count(ScriptBuilderMessage.id)).where(
|
||||||
@@ -344,36 +351,48 @@ async def save_to_library(
|
|||||||
raise ValueError("Default 'AI Generated' category not found. Run migrations.")
|
raise ValueError("Default 'AI Generated' category not found. Run migrations.")
|
||||||
resolved_category_id = default_cat
|
resolved_category_id = default_cat
|
||||||
|
|
||||||
# Generate unique slug
|
# Generate slug. Use a UUID suffix on first attempt to prevent concurrent
|
||||||
|
# saves with the same name from hitting the unique constraint on slug.
|
||||||
base_slug = name.lower().replace(" ", "-").replace("_", "-")[:80]
|
base_slug = name.lower().replace(" ", "-").replace("_", "-")[:80]
|
||||||
base_slug = re.sub(r"[^a-z0-9\-]", "", base_slug)
|
base_slug = re.sub(r"[^a-z0-9\-]", "", base_slug)
|
||||||
slug = base_slug
|
|
||||||
# Check uniqueness
|
|
||||||
existing = await db.execute(
|
|
||||||
select(ScriptTemplate.id).where(ScriptTemplate.slug == slug)
|
|
||||||
)
|
|
||||||
if existing.scalar_one_or_none():
|
|
||||||
slug = f"{base_slug}-{uuid_mod.uuid4().hex[:6]}"
|
|
||||||
|
|
||||||
template = ScriptTemplate(
|
# Check if the base slug is already taken; if not, use it clean (no suffix).
|
||||||
id=uuid_mod.uuid4(),
|
# If taken, or if the insert races with a concurrent request, retry with a
|
||||||
category_id=resolved_category_id,
|
# fresh UUID suffix. The unique constraint on script_templates.slug is the
|
||||||
created_by=user_id,
|
# authoritative guard — the application check just avoids unnecessary retries.
|
||||||
team_id=team_id if share_with_team else None,
|
existing = await db.execute(
|
||||||
name=name,
|
select(ScriptTemplate.id).where(ScriptTemplate.slug == base_slug)
|
||||||
slug=slug,
|
|
||||||
description=description,
|
|
||||||
script_body=script_body or session.latest_script,
|
|
||||||
parameters_schema=parameters_schema or {"parameters": []},
|
|
||||||
default_values={},
|
|
||||||
validation_rules={},
|
|
||||||
tags=[session.language, "ai-generated"],
|
|
||||||
complexity="intermediate",
|
|
||||||
is_verified=False,
|
|
||||||
is_active=True,
|
|
||||||
version=1,
|
|
||||||
usage_count=0,
|
|
||||||
)
|
)
|
||||||
db.add(template)
|
slug = base_slug if not existing.scalar_one_or_none() else f"{base_slug}-{uuid_mod.uuid4().hex[:6]}"
|
||||||
await db.flush()
|
|
||||||
return template
|
for attempt in range(3):
|
||||||
|
template = ScriptTemplate(
|
||||||
|
id=uuid_mod.uuid4(),
|
||||||
|
category_id=resolved_category_id,
|
||||||
|
created_by=user_id,
|
||||||
|
team_id=team_id if share_with_team else None,
|
||||||
|
name=name,
|
||||||
|
slug=slug,
|
||||||
|
description=description,
|
||||||
|
script_body=script_body or session.latest_script,
|
||||||
|
parameters_schema=parameters_schema or {"parameters": []},
|
||||||
|
default_values={},
|
||||||
|
validation_rules={},
|
||||||
|
tags=[session.language, "ai-generated"],
|
||||||
|
complexity="intermediate",
|
||||||
|
is_verified=False,
|
||||||
|
is_active=True,
|
||||||
|
version=1,
|
||||||
|
usage_count=0,
|
||||||
|
)
|
||||||
|
db.add(template)
|
||||||
|
try:
|
||||||
|
await db.flush()
|
||||||
|
return template
|
||||||
|
except IntegrityError as exc:
|
||||||
|
if "uq_script_templates_slug" not in str(exc.orig) or attempt == 2:
|
||||||
|
raise
|
||||||
|
await db.rollback()
|
||||||
|
slug = f"{base_slug}-{uuid_mod.uuid4().hex[:8]}"
|
||||||
|
|
||||||
|
raise RuntimeError("Failed to generate a unique slug after 3 attempts")
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useState, useEffect } from 'react'
|
import { useState, useEffect, useRef } from 'react'
|
||||||
import { useSearchParams } from 'react-router-dom'
|
import { useSearchParams } from 'react-router-dom'
|
||||||
import { Terminal } from 'lucide-react'
|
import { Terminal } from 'lucide-react'
|
||||||
import { cn } from '@/lib/utils'
|
import { cn } from '@/lib/utils'
|
||||||
@@ -21,6 +21,11 @@ export default function ScriptBuilderPage() {
|
|||||||
const [messages, setMessages] = useState<ScriptBuilderMessage[]>([])
|
const [messages, setMessages] = useState<ScriptBuilderMessage[]>([])
|
||||||
const [language, setLanguage] = useState('powershell')
|
const [language, setLanguage] = useState('powershell')
|
||||||
const [isLoading, setIsLoading] = useState(false)
|
const [isLoading, setIsLoading] = useState(false)
|
||||||
|
// Ref-based lock: prevents two concurrent handleSend calls (e.g. FlowPilot
|
||||||
|
// handoff useEffect + user keystroke) from each calling createSession() and
|
||||||
|
// creating two orphaned sessions. React state updates are async so isLoading
|
||||||
|
// alone can't guard across two calls in the same render cycle.
|
||||||
|
const creatingSessionRef = useRef(false)
|
||||||
const [previewScript, setPreviewScript] = useState<{ script: string; filename: string | null } | null>(null)
|
const [previewScript, setPreviewScript] = useState<{ script: string; filename: string | null } | null>(null)
|
||||||
const [showSaveDialog, setShowSaveDialog] = useState(false)
|
const [showSaveDialog, setShowSaveDialog] = useState(false)
|
||||||
const [handoffProcessed, setHandoffProcessed] = useState(false)
|
const [handoffProcessed, setHandoffProcessed] = useState(false)
|
||||||
@@ -75,8 +80,19 @@ export default function ScriptBuilderPage() {
|
|||||||
// Create session if needed
|
// Create session if needed
|
||||||
let currentSession = session
|
let currentSession = session
|
||||||
if (!currentSession) {
|
if (!currentSession) {
|
||||||
currentSession = await scriptBuilderApi.createSession(effectiveLanguage)
|
if (creatingSessionRef.current) {
|
||||||
setSession(currentSession)
|
// Another concurrent call is already creating the session; drop this send.
|
||||||
|
setIsLoading(false)
|
||||||
|
setMessages((prev) => prev.slice(0, -1))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
creatingSessionRef.current = true
|
||||||
|
try {
|
||||||
|
currentSession = await scriptBuilderApi.createSession(effectiveLanguage)
|
||||||
|
setSession(currentSession)
|
||||||
|
} finally {
|
||||||
|
creatingSessionRef.current = false
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Send message
|
// Send message
|
||||||
|
|||||||
Reference in New Issue
Block a user