fix: code review fixes — date calc, input validation, rate limits, shared components

- Fix monthly_reset_at crash when billing anchor day exceeds next month's length
- Add environment_tags sanitization (max 20 tags, 100 chars each) to prevent prompt injection
- Add @limiter.limit("10/minute") rate limiting to all AI endpoints
- Use getTreeNavigatePath() routing helper instead of hardcoded paths
- Extract shared CreateFlowDropdown component from QuickStartPage and TreeLibraryPage
- Clear useCachedQuota on logout to prevent stale data across user sessions
- Add useRef guard to scaffold useEffect to prevent potential double-fire
- Use node.id as React key instead of array index in BranchDetailView
- Remove redundant dead logic in ai_tree_validator

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
chihlasm
2026-02-21 01:32:38 -05:00
parent 4b9863f22d
commit 37e1202f46
11 changed files with 163 additions and 182 deletions

View File

@@ -11,9 +11,11 @@ import logging
from typing import Annotated
import anthropic
from fastapi import APIRouter, Depends, HTTPException, status
from fastapi import APIRouter, Depends, HTTPException, Request, status
from sqlalchemy.ext.asyncio import AsyncSession
from app.core.rate_limit import limiter
from app.api.deps import get_current_active_user, get_db, require_engineer_or_admin
from app.core.config import settings
from app.core.ai_conversation_store import (
@@ -86,7 +88,9 @@ async def get_quota(
@router.post("/start", response_model=AIStartResponse, status_code=201)
@limiter.limit("10/minute")
async def start_conversation(
request: Request,
data: AIStartRequest,
current_user: Annotated[User, Depends(get_current_active_user)],
db: Annotated[AsyncSession, Depends(get_db)],
@@ -140,7 +144,9 @@ async def start_conversation(
@router.post("/scaffold", response_model=AIScaffoldResponse)
@limiter.limit("10/minute")
async def scaffold(
request: Request,
data: AIScaffoldRequest,
current_user: Annotated[User, Depends(get_current_active_user)],
db: Annotated[AsyncSession, Depends(get_db)],
@@ -249,7 +255,9 @@ async def scaffold(
@router.post("/branch-detail", response_model=AIBranchDetailResponse)
@limiter.limit("10/minute")
async def branch_detail(
request: Request,
data: AIBranchDetailRequest,
current_user: Annotated[User, Depends(get_current_active_user)],
db: Annotated[AsyncSession, Depends(get_db)],
@@ -364,7 +372,9 @@ async def branch_detail(
@router.post("/assemble", response_model=AIAssembleResponse)
@limiter.limit("10/minute")
async def assemble(
request: Request,
data: AIAssembleRequest,
current_user: Annotated[User, Depends(get_current_active_user)],
db: Annotated[AsyncSession, Depends(get_db)],

View File

@@ -4,6 +4,7 @@ Enforces monthly and daily limits on AI flow builder usage.
Monthly quota consumed only on successful tree assembly (counts_toward_quota=True).
Daily limit is an anti-abuse guard consumed on conversation start.
"""
import calendar
from datetime import datetime, timezone, timedelta
from typing import Optional
from uuid import UUID
@@ -127,9 +128,13 @@ async def check_ai_quota(
deny_reason = "daily"
# Calculate reset timestamps
next_month = month_start.month % 12 + 1
next_year = month_start.year + (1 if month_start.month == 12 else 0)
max_day = calendar.monthrange(next_year, next_month)[1]
monthly_reset_at = month_start.replace(
month=month_start.month % 12 + 1,
year=month_start.year + (1 if month_start.month == 12 else 0),
month=next_month,
year=next_year,
day=min(month_start.day, max_day),
)
daily_reset_at = day_start + timedelta(hours=24)

View File

@@ -159,7 +159,7 @@ def _check_branch_termination(node: dict[str, Any], errors: list[str]) -> None:
if node_type == "solution":
return # Solution is a valid terminus
if not children and node_type != "solution":
if not children:
errors.append(
f"Node '{node_id}' (type={node_type}) is a dead end — "
"it has no children and is not a solution node"

View File

@@ -2,7 +2,7 @@
from typing import Any, Literal, Optional
from uuid import UUID
from pydantic import BaseModel, Field
from pydantic import BaseModel, Field, field_validator
# ── Requests ──
@@ -17,7 +17,17 @@ class AIStartRequest(BaseModel):
category_id: Optional[UUID] = None
name: str = Field(..., min_length=1, max_length=255)
description: str = Field("", max_length=2000)
environment_tags: list[str] = Field(default_factory=list)
environment_tags: list[str] = Field(default_factory=list, max_length=20)
@field_validator("environment_tags")
@classmethod
def validate_tags(cls, v: list[str]) -> list[str]:
for tag in v:
if len(tag) > 100:
raise ValueError("Each environment tag must be 100 characters or fewer")
if not tag.strip():
raise ValueError("Environment tags must not be empty")
return v
class AIScaffoldRequest(BaseModel):

View File

@@ -1,4 +1,4 @@
import { useEffect } from 'react'
import { useEffect, useRef } from 'react'
import { useNavigate } from 'react-router-dom'
import { Modal } from '@/components/common/Modal'
import { useAIFlowBuilderStore } from '@/store/aiFlowBuilderStore'
@@ -34,9 +34,11 @@ export function AIFlowBuilderModal({ isOpen, onClose }: AIFlowBuilderModalProps)
}
}, [isOpen, loadQuota])
// Auto-trigger scaffold after conversation starts
// Auto-trigger scaffold after conversation starts (ref prevents double-fire)
const hasTriggeredScaffold = useRef(false)
useEffect(() => {
if (phase === 'scaffolding' && !useAIFlowBuilderStore.getState().suggestedBranches.length) {
if (phase === 'scaffolding' && !hasTriggeredScaffold.current && !useAIFlowBuilderStore.getState().suggestedBranches.length) {
hasTriggeredScaffold.current = true
scaffold()
}
}, [phase, scaffold])

View File

@@ -200,8 +200,8 @@ function NodePreview({ node, depth }: { node: Record<string, unknown>; depth: nu
<span className="text-xs text-foreground truncate">{label}</span>
<span className="text-[10px] font-label text-muted-foreground">{type}</span>
</div>
{children.map((child, i) => (
<NodePreview key={i} node={child} depth={depth + 1} />
{children.map((child) => (
<NodePreview key={child.id as string ?? crypto.randomUUID()} node={child} depth={depth + 1} />
))}
</div>
)

View File

@@ -0,0 +1,93 @@
import { useState } from 'react'
import { Link } from 'react-router-dom'
import { Plus, ChevronDown, Sparkles, FolderTree, ListOrdered, Wrench } from 'lucide-react'
import { cn } from '@/lib/utils'
interface CreateFlowDropdownProps {
aiEnabled: boolean
onOpenAIBuilder: () => void
className?: string
/** Button label — defaults to "Create Flow" */
label?: string
}
export function CreateFlowDropdown({
aiEnabled,
onOpenAIBuilder,
className,
label = 'Create Flow',
}: CreateFlowDropdownProps) {
const [showMenu, setShowMenu] = useState(false)
return (
<div className={cn('relative', className)}>
<button
onClick={() => setShowMenu(!showMenu)}
className="flex items-center gap-2 rounded-lg bg-gradient-brand px-4 py-2 text-sm font-semibold text-white shadow-lg shadow-primary/20 hover:opacity-90 transition-opacity"
>
<Plus size={16} />
{label}
<ChevronDown size={14} />
</button>
{showMenu && (
<>
<div className="fixed inset-0 z-10" onClick={() => setShowMenu(false)} />
<div className="absolute right-0 z-20 mt-1 w-56 rounded-lg border border-border bg-card p-1 shadow-xl backdrop-blur-sm">
<Link
to="/trees/new"
onClick={() => setShowMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<FolderTree className="h-4 w-4 text-muted-foreground" />
<div>
<div className="font-medium">Troubleshooting Tree</div>
<div className="text-xs text-muted-foreground">Branching decision flow</div>
</div>
</Link>
<Link
to="/flows/new"
onClick={() => setShowMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<ListOrdered className="h-4 w-4 text-muted-foreground" />
<div>
<div className="font-medium">Procedural Flow</div>
<div className="text-xs text-muted-foreground">Step-by-step procedure</div>
</div>
</Link>
<Link
to="/flows/new?type=maintenance"
onClick={() => setShowMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<Wrench className="h-4 w-4 text-amber-400" />
<div>
<div className="font-medium">Maintenance Flow</div>
<div className="text-xs text-muted-foreground">Scheduled multi-target tasks</div>
</div>
</Link>
{aiEnabled && (
<>
<div className="my-1 border-t border-border" />
<button
type="button"
onClick={() => {
setShowMenu(false)
onOpenAIBuilder()
}}
className="flex w-full items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<Sparkles className="h-4 w-4 text-primary" />
<div className="text-left">
<div className="font-medium">Build with AI</div>
<div className="text-xs text-muted-foreground">AI-assisted flow creation</div>
</div>
</button>
</>
)}
</div>
</>
)}
</div>
)
}

View File

@@ -5,6 +5,11 @@ const CACHE_TTL_MS = 5 * 60 * 1000
let cachedResult: { aiEnabled: boolean; timestamp: number } | null = null
/** Clear the cached quota (call on logout to prevent stale data across users). */
export function clearCachedQuota() {
cachedResult = null
}
export function useCachedQuota() {
const [aiEnabled, setAiEnabled] = useState(cachedResult?.aiEnabled ?? false)
const [isLoading, setIsLoading] = useState(!cachedResult)

View File

@@ -1,6 +1,6 @@
import { useState, useEffect, useRef, useMemo } from 'react'
import { useNavigate, Link } from 'react-router-dom'
import { Search, Plus, Loader2, Star, ChevronDown, ChevronLeft, ChevronRight, Sparkles, FolderTree, ListOrdered, Wrench } from 'lucide-react'
import { useNavigate } from 'react-router-dom'
import { Search, Loader2, Star, ChevronLeft, ChevronRight } from 'lucide-react'
import { treesApi } from '@/api/trees'
import { sessionsApi } from '@/api/sessions'
import type { TreeListItem } from '@/types'
@@ -19,6 +19,7 @@ import { TreeListView } from '@/components/library/TreeListView'
import { TreeTableView } from '@/components/library/TreeTableView'
import { ViewToggle } from '@/components/library/ViewToggle'
import { AIFlowBuilderModal } from '@/components/ai-builder/AIFlowBuilderModal'
import { CreateFlowDropdown } from '@/components/common/CreateFlowDropdown'
import { cn } from '@/lib/utils'
function timeAgo(dateStr: string): string {
@@ -61,8 +62,7 @@ export function QuickStartPage() {
// Favorites state
const [showAllFavorites, setShowAllFavorites] = useState(false)
// Create menu + AI Builder
const [showCreateMenu, setShowCreateMenu] = useState(false)
// AI Builder
const [showAIBuilder, setShowAIBuilder] = useState(false)
const { aiEnabled } = useCachedQuota()
@@ -212,13 +212,7 @@ export function QuickStartPage() {
// Handlers
const handleStartSession = (treeId: string, treeType?: string) => {
if (treeType === 'maintenance') {
navigate(`/flows/${treeId}/maintenance`)
} else if (treeType === 'procedural') {
navigate(`/flows/${treeId}/navigate`)
} else {
navigate(`/trees/${treeId}/navigate`)
}
navigate(getTreeNavigatePath(treeId, treeType))
}
const handleDeleteTree = () => {} // Not used on dashboard
@@ -242,75 +236,10 @@ export function QuickStartPage() {
</div>
<div className="flex items-center gap-2">
{canCreateTrees && (
<div className="relative">
<button
onClick={() => setShowCreateMenu(!showCreateMenu)}
className="flex items-center gap-2 rounded-lg bg-gradient-brand px-4 py-2 text-sm font-semibold text-white shadow-lg shadow-primary/20 hover:opacity-90 transition-opacity"
>
<Plus size={16} />
Create Flow
<ChevronDown size={14} />
</button>
{showCreateMenu && (
<>
<div className="fixed inset-0 z-10" onClick={() => setShowCreateMenu(false)} />
<div className="absolute right-0 z-20 mt-1 w-56 rounded-lg border border-border bg-card p-1 shadow-xl backdrop-blur-sm">
<Link
to="/trees/new"
onClick={() => setShowCreateMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<FolderTree className="h-4 w-4 text-muted-foreground" />
<div>
<div className="font-medium">Troubleshooting Tree</div>
<div className="text-xs text-muted-foreground">Branching decision flow</div>
</div>
</Link>
<Link
to="/flows/new"
onClick={() => setShowCreateMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<ListOrdered className="h-4 w-4 text-muted-foreground" />
<div>
<div className="font-medium">Procedural Flow</div>
<div className="text-xs text-muted-foreground">Step-by-step procedure</div>
</div>
</Link>
<Link
to="/flows/new?type=maintenance"
onClick={() => setShowCreateMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<Wrench className="h-4 w-4 text-amber-400" />
<div>
<div className="font-medium">Maintenance Flow</div>
<div className="text-xs text-muted-foreground">Scheduled multi-target tasks</div>
</div>
</Link>
{aiEnabled && (
<>
<div className="my-1 border-t border-border" />
<button
type="button"
onClick={() => {
setShowCreateMenu(false)
setShowAIBuilder(true)
}}
className="flex w-full items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<Sparkles className="h-4 w-4 text-primary" />
<div className="text-left">
<div className="font-medium">Build with AI</div>
<div className="text-xs text-muted-foreground">AI-assisted flow creation</div>
</div>
</button>
</>
)}
</div>
</>
)}
</div>
<CreateFlowDropdown
aiEnabled={aiEnabled}
onOpenAIBuilder={() => setShowAIBuilder(true)}
/>
)}
</div>
</div>
@@ -444,13 +373,11 @@ export function QuickStartPage() {
<div className="py-12 text-center">
<p className="text-muted-foreground mb-4">You haven&apos;t created any flows yet.</p>
{canCreateTrees && (
<button
onClick={() => setShowCreateMenu(true)}
className="inline-flex items-center gap-2 rounded-lg bg-gradient-brand px-4 py-2 text-sm font-semibold text-white shadow-lg shadow-primary/20 hover:opacity-90 transition-opacity"
>
<Plus size={16} />
Create your first flow
</button>
<CreateFlowDropdown
aiEnabled={aiEnabled}
onOpenAIBuilder={() => setShowAIBuilder(true)}
label="Create your first flow"
/>
)}
</div>
) : (

View File

@@ -1,6 +1,6 @@
import { useEffect, useState, useCallback, useMemo } from 'react'
import { useNavigate, Link, useSearchParams } from 'react-router-dom'
import { Plus, X, RotateCcw, Play, ChevronDown, Sparkles, FolderTree, ListOrdered, Wrench } from 'lucide-react'
import { useNavigate, useSearchParams } from 'react-router-dom'
import { X, RotateCcw, Play } from 'lucide-react'
import { treesApi } from '@/api/trees'
import { categoriesApi } from '@/api/categories'
import { foldersApi } from '@/api/folders'
@@ -14,12 +14,13 @@ import { TreeTableView } from '@/components/library/TreeTableView'
import { ViewToggle } from '@/components/library/ViewToggle'
import { SortDropdown } from '@/components/library/SortDropdown'
import { cn, safeGetItem } from '@/lib/utils'
import { getSessionResumePath } from '@/lib/routing'
import { getSessionResumePath, getTreeNavigatePath } from '@/lib/routing'
import { usePermissions } from '@/hooks/usePermissions'
import { useUserPreferencesStore } from '@/store/userPreferencesStore'
import { usePinnedFlowsStore } from '@/store/pinnedFlowsStore'
import { useCachedQuota } from '@/hooks/useCachedQuota'
import { AIFlowBuilderModal } from '@/components/ai-builder/AIFlowBuilderModal'
import { CreateFlowDropdown } from '@/components/common/CreateFlowDropdown'
import { toast } from '@/lib/toast'
export function TreeLibraryPage() {
@@ -72,8 +73,7 @@ export function TreeLibraryPage() {
// Fork state
const [isForkingTree, setIsForkingTree] = useState(false)
// Create menu & AI builder state
const [showCreateMenu, setShowCreateMenu] = useState(false)
// AI builder state
const [showAIBuilder, setShowAIBuilder] = useState(false)
const { aiEnabled } = useCachedQuota()
@@ -215,13 +215,7 @@ export function TreeLibraryPage() {
}
const handleStartSession = (treeId: string, treeType?: string) => {
if (treeType === 'maintenance') {
navigate(`/flows/${treeId}/maintenance`)
} else if (treeType === 'procedural') {
navigate(`/flows/${treeId}/navigate`)
} else {
navigate(`/trees/${treeId}/navigate`)
}
navigate(getTreeNavigatePath(treeId, treeType))
}
const handleCreateFolder = (parentId?: string | null) => {
@@ -285,78 +279,11 @@ export function TreeLibraryPage() {
</p>
</div>
{canCreateTrees && (
<div className="relative">
<button
onClick={() => setShowCreateMenu(!showCreateMenu)}
className={cn(
'flex items-center gap-2 rounded-md bg-gradient-brand px-4 py-2 text-sm font-medium text-white shadow-lg shadow-primary/20',
'hover:opacity-90'
)}
>
<Plus className="h-4 w-4" />
Create New
<ChevronDown className="h-3.5 w-3.5" />
</button>
{showCreateMenu && (
<>
<div className="fixed inset-0 z-10" onClick={() => setShowCreateMenu(false)} />
<div className="absolute right-0 z-20 mt-1 w-56 rounded-lg border border-border bg-card p-1 shadow-xl backdrop-blur-sm">
<Link
to="/trees/new"
onClick={() => setShowCreateMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<FolderTree className="h-4 w-4 text-muted-foreground" />
<div>
<div className="font-medium">Troubleshooting Tree</div>
<div className="text-xs text-muted-foreground">Branching decision flow</div>
</div>
</Link>
<Link
to="/flows/new"
onClick={() => setShowCreateMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<ListOrdered className="h-4 w-4 text-muted-foreground" />
<div>
<div className="font-medium">Procedural Flow</div>
<div className="text-xs text-muted-foreground">Step-by-step procedure</div>
</div>
</Link>
<Link
to="/flows/new?type=maintenance"
onClick={() => setShowCreateMenu(false)}
className="flex items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<Wrench className="h-4 w-4 text-amber-400" />
<div>
<div className="font-medium">Maintenance Flow</div>
<div className="text-xs text-muted-foreground">Scheduled multi-target tasks</div>
</div>
</Link>
{aiEnabled && (
<>
<div className="my-1 border-t border-border" />
<button
type="button"
onClick={() => {
setShowCreateMenu(false)
setShowAIBuilder(true)
}}
className="flex w-full items-center gap-3 rounded-md px-3 py-2.5 text-sm text-foreground hover:bg-accent"
>
<Sparkles className="h-4 w-4 text-primary" />
<div className="text-left">
<div className="font-medium">Build with AI</div>
<div className="text-xs text-muted-foreground">AI-assisted flow creation</div>
</div>
</button>
</>
)}
</div>
</>
)}
</div>
<CreateFlowDropdown
aiEnabled={aiEnabled}
onOpenAIBuilder={() => setShowAIBuilder(true)}
label="Create New"
/>
)}
</div>

View File

@@ -3,6 +3,7 @@ import { persist } from 'zustand/middleware'
import type { User, Token, UserCreate, UserLogin, Account, SubscriptionDetails } from '@/types'
import { authApi } from '@/api/auth'
import { apiClient } from '@/api/client'
import { clearCachedQuota } from '@/hooks/useCachedQuota'
interface AuthState {
user: User | null
@@ -79,6 +80,7 @@ export const useAuthStore = create<AuthState>()(
} finally {
localStorage.removeItem('access_token')
localStorage.removeItem('refresh_token')
clearCachedQuota()
set({ user: null, token: null, account: null, subscription: null, isAuthenticated: false, error: null })
}
},