mirror of
https://github.com/leonvanzyl/autocoder.git
synced 2026-01-30 06:12:06 +00:00
fix: address critical issues in PR #75 agent scheduling feature
This commit fixes several issues identified in the agent scheduling feature from PR #75: Frontend Fixes: - Add day boundary handling in timeUtils.ts for timezone conversions - Add utcToLocalWithDayShift/localToUTCWithDayShift functions - Add shiftDaysForward/shiftDaysBackward helpers for bitfield adjustment - Update ScheduleModal to correctly adjust days_of_week when crossing day boundaries during UTC conversion (fixes schedules running on wrong days for users in extreme timezones like UTC+9) Backend Fixes: - Add MAX_SCHEDULES_PER_PROJECT (50) limit to prevent resource exhaustion - Wire up crash recovery callback in scheduler_service._start_agent() - Convert schedules.py endpoints to use context manager for DB sessions - Fix race condition in override creation with atomic delete-then-create - Replace deprecated datetime.utcnow with datetime.now(timezone.utc) - Add DB-level CHECK constraints for Schedule model fields Files Modified: - api/database.py: Add _utc_now helper, CheckConstraint imports, constraints - progress.py: Replace deprecated datetime.utcnow - server/routers/schedules.py: Add context manager, schedule limits - server/services/assistant_database.py: Replace deprecated datetime.utcnow - server/services/scheduler_service.py: Wire crash recovery, fix race condition - ui/src/components/ScheduleModal.tsx: Use day shift functions - ui/src/lib/timeUtils.ts: Add day boundary handling functions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -8,10 +8,16 @@ Provides CRUD operations for time-based schedule configuration.
|
||||
|
||||
import re
|
||||
import sys
|
||||
from contextlib import contextmanager
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from pathlib import Path
|
||||
from typing import Generator, Tuple
|
||||
|
||||
from fastapi import APIRouter, HTTPException
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
# Schedule limits to prevent resource exhaustion
|
||||
MAX_SCHEDULES_PER_PROJECT = 50
|
||||
|
||||
from ..schemas import (
|
||||
NextRunResponse,
|
||||
@@ -48,8 +54,15 @@ def validate_project_name(name: str) -> str:
|
||||
return name
|
||||
|
||||
|
||||
def _get_db_session(project_name: str):
|
||||
"""Get database session for a project."""
|
||||
@contextmanager
|
||||
def _get_db_session(project_name: str) -> Generator[Tuple[Session, Path], None, None]:
|
||||
"""Get database session for a project as a context manager.
|
||||
|
||||
Usage:
|
||||
with _get_db_session(project_name) as (db, project_path):
|
||||
# ... use db ...
|
||||
# db is automatically closed
|
||||
"""
|
||||
from api.database import create_database
|
||||
|
||||
project_name = validate_project_name(project_name)
|
||||
@@ -68,7 +81,11 @@ def _get_db_session(project_name: str):
|
||||
)
|
||||
|
||||
_, SessionLocal = create_database(project_path)
|
||||
return SessionLocal(), project_path
|
||||
db = SessionLocal()
|
||||
try:
|
||||
yield db, project_path
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@router.get("", response_model=ScheduleListResponse)
|
||||
@@ -76,9 +93,7 @@ async def list_schedules(project_name: str):
|
||||
"""Get all schedules for a project."""
|
||||
from api.database import Schedule
|
||||
|
||||
db, _ = _get_db_session(project_name)
|
||||
|
||||
try:
|
||||
with _get_db_session(project_name) as (db, _):
|
||||
schedules = db.query(Schedule).filter(
|
||||
Schedule.project_name == project_name
|
||||
).order_by(Schedule.start_time).all()
|
||||
@@ -100,8 +115,6 @@ async def list_schedules(project_name: str):
|
||||
for s in schedules
|
||||
]
|
||||
)
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@router.post("", response_model=ScheduleResponse, status_code=201)
|
||||
@@ -111,9 +124,18 @@ async def create_schedule(project_name: str, data: ScheduleCreate):
|
||||
|
||||
from ..services.scheduler_service import get_scheduler
|
||||
|
||||
db, project_path = _get_db_session(project_name)
|
||||
with _get_db_session(project_name) as (db, project_path):
|
||||
# Check schedule limit to prevent resource exhaustion
|
||||
existing_count = db.query(Schedule).filter(
|
||||
Schedule.project_name == project_name
|
||||
).count()
|
||||
|
||||
if existing_count >= MAX_SCHEDULES_PER_PROJECT:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Maximum schedules per project ({MAX_SCHEDULES_PER_PROJECT}) exceeded"
|
||||
)
|
||||
|
||||
try:
|
||||
# Create schedule record
|
||||
schedule = Schedule(
|
||||
project_name=project_name,
|
||||
@@ -178,9 +200,6 @@ async def create_schedule(project_name: str, data: ScheduleCreate):
|
||||
created_at=schedule.created_at,
|
||||
)
|
||||
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@router.get("/next", response_model=NextRunResponse)
|
||||
async def get_next_scheduled_run(project_name: str):
|
||||
@@ -189,9 +208,7 @@ async def get_next_scheduled_run(project_name: str):
|
||||
|
||||
from ..services.scheduler_service import get_scheduler
|
||||
|
||||
db, _ = _get_db_session(project_name)
|
||||
|
||||
try:
|
||||
with _get_db_session(project_name) as (db, _):
|
||||
schedules = db.query(Schedule).filter(
|
||||
Schedule.project_name == project_name,
|
||||
Schedule.enabled == True, # noqa: E712
|
||||
@@ -245,18 +262,13 @@ async def get_next_scheduled_run(project_name: str):
|
||||
active_schedule_count=active_count,
|
||||
)
|
||||
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@router.get("/{schedule_id}", response_model=ScheduleResponse)
|
||||
async def get_schedule(project_name: str, schedule_id: int):
|
||||
"""Get a single schedule by ID."""
|
||||
from api.database import Schedule
|
||||
|
||||
db, _ = _get_db_session(project_name)
|
||||
|
||||
try:
|
||||
with _get_db_session(project_name) as (db, _):
|
||||
schedule = db.query(Schedule).filter(
|
||||
Schedule.id == schedule_id,
|
||||
Schedule.project_name == project_name,
|
||||
@@ -278,9 +290,6 @@ async def get_schedule(project_name: str, schedule_id: int):
|
||||
created_at=schedule.created_at,
|
||||
)
|
||||
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@router.patch("/{schedule_id}", response_model=ScheduleResponse)
|
||||
async def update_schedule(
|
||||
@@ -293,9 +302,7 @@ async def update_schedule(
|
||||
|
||||
from ..services.scheduler_service import get_scheduler
|
||||
|
||||
db, project_path = _get_db_session(project_name)
|
||||
|
||||
try:
|
||||
with _get_db_session(project_name) as (db, project_path):
|
||||
schedule = db.query(Schedule).filter(
|
||||
Schedule.id == schedule_id,
|
||||
Schedule.project_name == project_name,
|
||||
@@ -337,9 +344,6 @@ async def update_schedule(
|
||||
created_at=schedule.created_at,
|
||||
)
|
||||
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@router.delete("/{schedule_id}", status_code=204)
|
||||
async def delete_schedule(project_name: str, schedule_id: int):
|
||||
@@ -348,9 +352,7 @@ async def delete_schedule(project_name: str, schedule_id: int):
|
||||
|
||||
from ..services.scheduler_service import get_scheduler
|
||||
|
||||
db, _ = _get_db_session(project_name)
|
||||
|
||||
try:
|
||||
with _get_db_session(project_name) as (db, _):
|
||||
schedule = db.query(Schedule).filter(
|
||||
Schedule.id == schedule_id,
|
||||
Schedule.project_name == project_name,
|
||||
@@ -367,9 +369,6 @@ async def delete_schedule(project_name: str, schedule_id: int):
|
||||
db.delete(schedule)
|
||||
db.commit()
|
||||
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
def _calculate_window_end(schedule, now: datetime) -> datetime:
|
||||
"""Calculate when the current window ends."""
|
||||
|
||||
@@ -7,7 +7,7 @@ Each project has its own assistant.db file in the project directory.
|
||||
"""
|
||||
|
||||
import logging
|
||||
from datetime import datetime
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
@@ -19,6 +19,11 @@ logger = logging.getLogger(__name__)
|
||||
Base = declarative_base()
|
||||
|
||||
|
||||
def _utc_now() -> datetime:
|
||||
"""Return current UTC time. Replacement for deprecated datetime.utcnow()."""
|
||||
return datetime.now(timezone.utc)
|
||||
|
||||
|
||||
class Conversation(Base):
|
||||
"""A conversation with the assistant for a project."""
|
||||
__tablename__ = "conversations"
|
||||
@@ -26,8 +31,8 @@ class Conversation(Base):
|
||||
id = Column(Integer, primary_key=True, index=True)
|
||||
project_name = Column(String(100), nullable=False, index=True)
|
||||
title = Column(String(200), nullable=True) # Optional title, derived from first message
|
||||
created_at = Column(DateTime, default=datetime.utcnow)
|
||||
updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow)
|
||||
created_at = Column(DateTime, default=_utc_now)
|
||||
updated_at = Column(DateTime, default=_utc_now, onupdate=_utc_now)
|
||||
|
||||
messages = relationship("ConversationMessage", back_populates="conversation", cascade="all, delete-orphan")
|
||||
|
||||
@@ -40,7 +45,7 @@ class ConversationMessage(Base):
|
||||
conversation_id = Column(Integer, ForeignKey("conversations.id"), nullable=False, index=True)
|
||||
role = Column(String(20), nullable=False) # "user" | "assistant" | "system"
|
||||
content = Column(Text, nullable=False)
|
||||
timestamp = Column(DateTime, default=datetime.utcnow)
|
||||
timestamp = Column(DateTime, default=_utc_now)
|
||||
|
||||
conversation = relationship("Conversation", back_populates="messages")
|
||||
|
||||
@@ -175,7 +180,7 @@ def add_message(project_dir: Path, conversation_id: int, role: str, content: str
|
||||
session.add(message)
|
||||
|
||||
# Update conversation's updated_at timestamp
|
||||
conversation.updated_at = datetime.utcnow()
|
||||
conversation.updated_at = _utc_now()
|
||||
|
||||
# Auto-generate title from first user message if not set
|
||||
if not conversation.title and role == "user":
|
||||
|
||||
@@ -368,6 +368,14 @@ class SchedulerService:
|
||||
logger.info(f"Agent already running for {project_name}, skipping scheduled start")
|
||||
return
|
||||
|
||||
# Register crash callback to enable auto-restart during scheduled windows
|
||||
async def on_status_change(status: str):
|
||||
if status == "crashed":
|
||||
logger.info(f"Crash detected for {project_name}, attempting recovery")
|
||||
await self.handle_crash_during_window(project_name, project_dir)
|
||||
|
||||
manager.add_status_callback(on_status_change)
|
||||
|
||||
logger.info(
|
||||
f"Starting agent for {project_name} "
|
||||
f"(schedule {schedule.id}, yolo={schedule.yolo_mode}, concurrency={schedule.max_concurrency})"
|
||||
@@ -382,6 +390,8 @@ class SchedulerService:
|
||||
logger.info(f"✓ Agent started successfully for {project_name}")
|
||||
else:
|
||||
logger.error(f"✗ Failed to start agent for {project_name}: {msg}")
|
||||
# Remove callback if start failed
|
||||
manager.remove_status_callback(on_status_change)
|
||||
|
||||
async def _stop_agent(self, project_name: str, project_dir: Path):
|
||||
"""Stop the agent for a project."""
|
||||
@@ -457,7 +467,10 @@ class SchedulerService:
|
||||
def _create_override_for_active_schedules(
|
||||
self, project_name: str, project_dir: Path, override_type: str
|
||||
):
|
||||
"""Create overrides for all active schedule windows."""
|
||||
"""Create overrides for all active schedule windows.
|
||||
|
||||
Uses atomic delete-then-create pattern to prevent race conditions.
|
||||
"""
|
||||
from api.database import Schedule, ScheduleOverride, create_database
|
||||
|
||||
try:
|
||||
@@ -479,17 +492,20 @@ class SchedulerService:
|
||||
# Calculate window end time
|
||||
window_end = self._calculate_window_end(schedule, now)
|
||||
|
||||
# Check if override already exists
|
||||
existing = db.query(ScheduleOverride).filter(
|
||||
# Atomic operation: delete any existing overrides of this type
|
||||
# and create a new one in the same transaction
|
||||
deleted = db.query(ScheduleOverride).filter(
|
||||
ScheduleOverride.schedule_id == schedule.id,
|
||||
ScheduleOverride.override_type == override_type,
|
||||
ScheduleOverride.expires_at > now,
|
||||
).first()
|
||||
).delete()
|
||||
|
||||
if existing:
|
||||
continue
|
||||
if deleted:
|
||||
logger.debug(
|
||||
f"Removed {deleted} existing '{override_type}' override(s) "
|
||||
f"for schedule {schedule.id}"
|
||||
)
|
||||
|
||||
# Create override
|
||||
# Create new override
|
||||
override = ScheduleOverride(
|
||||
schedule_id=schedule.id,
|
||||
override_type=override_type,
|
||||
|
||||
Reference in New Issue
Block a user