mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-02-17 18:21:46 -05:00
fix(copilot): Route workspace through db_accessors, fix transcript upload (#12148)
## Summary Fixes two bugs in the copilot executor: ### SECRT-2008: WorkspaceManager bypasses db_accessors `backend/util/workspace.py` imported 6 workspace functions directly from `backend/data/workspace.py`, which call `prisma()` directly. In the copilot executor (no Prisma connection), these fail. **Fix:** Replace direct imports with `workspace_db()` from `db_accessors`, routing through the database_manager HTTP client when Prisma is unavailable. Also: - Register all 6 workspace functions in `DatabaseManager` and `DatabaseManagerAsyncClient` - Add `UniqueViolationError` to the service `EXCEPTION_MAPPING` so it's properly re-raised over HTTP (needed for race-condition retry logic) ### SECRT-2009: Transcript upload asyncio.timeout error `asyncio.create_task()` at line 696 of `service.py` creates an orphaned background task in the executor's thread event loop. `gcloud-aio-storage`'s `asyncio.timeout()` fails in this context. **Fix:** Replace `create_task` with direct `await`. The upload runs after streaming completes (all chunks already yielded), so no user-facing latency impact. The function already has internal try/except error handling.
This commit is contained in:
@@ -693,11 +693,15 @@ async def stream_chat_completion_sdk(
|
||||
await asyncio.sleep(0.5)
|
||||
raw_transcript = read_transcript_file(captured_transcript.path)
|
||||
if raw_transcript:
|
||||
task = asyncio.create_task(
|
||||
_upload_transcript_bg(user_id, session_id, raw_transcript)
|
||||
)
|
||||
_background_tasks.add(task)
|
||||
task.add_done_callback(_background_tasks.discard)
|
||||
try:
|
||||
async with asyncio.timeout(30):
|
||||
await _upload_transcript_bg(
|
||||
user_id, session_id, raw_transcript
|
||||
)
|
||||
except asyncio.TimeoutError:
|
||||
logger.warning(
|
||||
f"[SDK] Transcript upload timed out for {session_id}"
|
||||
)
|
||||
else:
|
||||
logger.debug("[SDK] Stop hook fired but transcript not usable")
|
||||
|
||||
|
||||
@@ -93,7 +93,15 @@ from backend.data.user import (
|
||||
get_user_notification_preference,
|
||||
update_user_integrations,
|
||||
)
|
||||
from backend.data.workspace import get_or_create_workspace
|
||||
from backend.data.workspace import (
|
||||
count_workspace_files,
|
||||
create_workspace_file,
|
||||
get_or_create_workspace,
|
||||
get_workspace_file,
|
||||
get_workspace_file_by_path,
|
||||
list_workspace_files,
|
||||
soft_delete_workspace_file,
|
||||
)
|
||||
from backend.util.service import (
|
||||
AppService,
|
||||
AppServiceClient,
|
||||
@@ -274,7 +282,13 @@ class DatabaseManager(AppService):
|
||||
get_user_execution_summary_data = _(get_user_execution_summary_data)
|
||||
|
||||
# ============ Workspace ============ #
|
||||
count_workspace_files = _(count_workspace_files)
|
||||
create_workspace_file = _(create_workspace_file)
|
||||
get_or_create_workspace = _(get_or_create_workspace)
|
||||
get_workspace_file = _(get_workspace_file)
|
||||
get_workspace_file_by_path = _(get_workspace_file_by_path)
|
||||
list_workspace_files = _(list_workspace_files)
|
||||
soft_delete_workspace_file = _(soft_delete_workspace_file)
|
||||
|
||||
# ============ Understanding ============ #
|
||||
get_business_understanding = _(get_business_understanding)
|
||||
@@ -438,7 +452,13 @@ class DatabaseManagerAsyncClient(AppServiceClient):
|
||||
get_user_execution_summary_data = d.get_user_execution_summary_data
|
||||
|
||||
# ============ Workspace ============ #
|
||||
count_workspace_files = d.count_workspace_files
|
||||
create_workspace_file = d.create_workspace_file
|
||||
get_or_create_workspace = d.get_or_create_workspace
|
||||
get_workspace_file = d.get_workspace_file
|
||||
get_workspace_file_by_path = d.get_workspace_file_by_path
|
||||
list_workspace_files = d.list_workspace_files
|
||||
soft_delete_workspace_file = d.soft_delete_workspace_file
|
||||
|
||||
# ============ Understanding ============ #
|
||||
get_business_understanding = d.get_business_understanding
|
||||
|
||||
@@ -164,21 +164,23 @@ async def create_workspace_file(
|
||||
|
||||
async def get_workspace_file(
|
||||
file_id: str,
|
||||
workspace_id: Optional[str] = None,
|
||||
workspace_id: str,
|
||||
) -> Optional[WorkspaceFile]:
|
||||
"""
|
||||
Get a workspace file by ID.
|
||||
|
||||
Args:
|
||||
file_id: The file ID
|
||||
workspace_id: Optional workspace ID for validation
|
||||
workspace_id: Workspace ID for scoping (required)
|
||||
|
||||
Returns:
|
||||
WorkspaceFile instance or None
|
||||
"""
|
||||
where_clause: dict = {"id": file_id, "isDeleted": False}
|
||||
if workspace_id:
|
||||
where_clause["workspaceId"] = workspace_id
|
||||
where_clause: UserWorkspaceFileWhereInput = {
|
||||
"id": file_id,
|
||||
"isDeleted": False,
|
||||
"workspaceId": workspace_id,
|
||||
}
|
||||
|
||||
file = await UserWorkspaceFile.prisma().find_first(where=where_clause)
|
||||
return WorkspaceFile.from_db(file) if file else None
|
||||
@@ -268,7 +270,7 @@ async def count_workspace_files(
|
||||
Returns:
|
||||
Number of files
|
||||
"""
|
||||
where_clause: dict = {"workspaceId": workspace_id}
|
||||
where_clause: UserWorkspaceFileWhereInput = {"workspaceId": workspace_id}
|
||||
if not include_deleted:
|
||||
where_clause["isDeleted"] = False
|
||||
|
||||
@@ -283,7 +285,7 @@ async def count_workspace_files(
|
||||
|
||||
async def soft_delete_workspace_file(
|
||||
file_id: str,
|
||||
workspace_id: Optional[str] = None,
|
||||
workspace_id: str,
|
||||
) -> Optional[WorkspaceFile]:
|
||||
"""
|
||||
Soft-delete a workspace file.
|
||||
@@ -293,7 +295,7 @@ async def soft_delete_workspace_file(
|
||||
|
||||
Args:
|
||||
file_id: The file ID
|
||||
workspace_id: Optional workspace ID for validation
|
||||
workspace_id: Workspace ID for scoping (required)
|
||||
|
||||
Returns:
|
||||
Updated WorkspaceFile instance or None if not found
|
||||
|
||||
@@ -28,7 +28,7 @@ from typing import (
|
||||
import httpx
|
||||
import uvicorn
|
||||
from fastapi import FastAPI, Request, responses
|
||||
from prisma.errors import DataError
|
||||
from prisma.errors import DataError, UniqueViolationError
|
||||
from pydantic import BaseModel, TypeAdapter, create_model
|
||||
|
||||
import backend.util.exceptions as exceptions
|
||||
@@ -201,6 +201,7 @@ EXCEPTION_MAPPING = {
|
||||
UnhealthyServiceError,
|
||||
HTTPClientError,
|
||||
HTTPServerError,
|
||||
UniqueViolationError,
|
||||
*[
|
||||
ErrorType
|
||||
for _, ErrorType in inspect.getmembers(exceptions)
|
||||
@@ -416,6 +417,9 @@ class AppService(BaseAppService, ABC):
|
||||
self.fastapi_app.add_exception_handler(
|
||||
DataError, self._handle_internal_http_error(400)
|
||||
)
|
||||
self.fastapi_app.add_exception_handler(
|
||||
UniqueViolationError, self._handle_internal_http_error(400)
|
||||
)
|
||||
self.fastapi_app.add_exception_handler(
|
||||
Exception, self._handle_internal_http_error(500)
|
||||
)
|
||||
@@ -478,6 +482,7 @@ def get_service_client(
|
||||
# Don't retry these specific exceptions that won't be fixed by retrying
|
||||
ValueError, # Invalid input/parameters
|
||||
DataError, # Prisma data integrity errors (foreign key, unique constraints)
|
||||
UniqueViolationError, # Unique constraint violations
|
||||
KeyError, # Missing required data
|
||||
TypeError, # Wrong data types
|
||||
AttributeError, # Missing attributes
|
||||
|
||||
@@ -12,15 +12,8 @@ from typing import Optional
|
||||
|
||||
from prisma.errors import UniqueViolationError
|
||||
|
||||
from backend.data.workspace import (
|
||||
WorkspaceFile,
|
||||
count_workspace_files,
|
||||
create_workspace_file,
|
||||
get_workspace_file,
|
||||
get_workspace_file_by_path,
|
||||
list_workspace_files,
|
||||
soft_delete_workspace_file,
|
||||
)
|
||||
from backend.data.db_accessors import workspace_db
|
||||
from backend.data.workspace import WorkspaceFile
|
||||
from backend.util.settings import Config
|
||||
from backend.util.virus_scanner import scan_content_safe
|
||||
from backend.util.workspace_storage import compute_file_checksum, get_workspace_storage
|
||||
@@ -125,8 +118,9 @@ class WorkspaceManager:
|
||||
Raises:
|
||||
FileNotFoundError: If file doesn't exist
|
||||
"""
|
||||
db = workspace_db()
|
||||
resolved_path = self._resolve_path(path)
|
||||
file = await get_workspace_file_by_path(self.workspace_id, resolved_path)
|
||||
file = await db.get_workspace_file_by_path(self.workspace_id, resolved_path)
|
||||
if file is None:
|
||||
raise FileNotFoundError(f"File not found at path: {resolved_path}")
|
||||
|
||||
@@ -146,7 +140,8 @@ class WorkspaceManager:
|
||||
Raises:
|
||||
FileNotFoundError: If file doesn't exist
|
||||
"""
|
||||
file = await get_workspace_file(file_id, self.workspace_id)
|
||||
db = workspace_db()
|
||||
file = await db.get_workspace_file(file_id, self.workspace_id)
|
||||
if file is None:
|
||||
raise FileNotFoundError(f"File not found: {file_id}")
|
||||
|
||||
@@ -204,8 +199,10 @@ class WorkspaceManager:
|
||||
# For overwrite=True, we let the write proceed and handle via UniqueViolationError
|
||||
# This ensures the new file is written to storage BEFORE the old one is deleted,
|
||||
# preventing data loss if the new write fails
|
||||
db = workspace_db()
|
||||
|
||||
if not overwrite:
|
||||
existing = await get_workspace_file_by_path(self.workspace_id, path)
|
||||
existing = await db.get_workspace_file_by_path(self.workspace_id, path)
|
||||
if existing is not None:
|
||||
raise ValueError(f"File already exists at path: {path}")
|
||||
|
||||
@@ -232,7 +229,7 @@ class WorkspaceManager:
|
||||
# Create database record - handle race condition where another request
|
||||
# created a file at the same path between our check and create
|
||||
try:
|
||||
file = await create_workspace_file(
|
||||
file = await db.create_workspace_file(
|
||||
workspace_id=self.workspace_id,
|
||||
file_id=file_id,
|
||||
name=filename,
|
||||
@@ -246,12 +243,12 @@ class WorkspaceManager:
|
||||
# Race condition: another request created a file at this path
|
||||
if overwrite:
|
||||
# Re-fetch and delete the conflicting file, then retry
|
||||
existing = await get_workspace_file_by_path(self.workspace_id, path)
|
||||
existing = await db.get_workspace_file_by_path(self.workspace_id, path)
|
||||
if existing:
|
||||
await self.delete_file(existing.id)
|
||||
# Retry the create - if this also fails, clean up storage file
|
||||
try:
|
||||
file = await create_workspace_file(
|
||||
file = await db.create_workspace_file(
|
||||
workspace_id=self.workspace_id,
|
||||
file_id=file_id,
|
||||
name=filename,
|
||||
@@ -314,8 +311,9 @@ class WorkspaceManager:
|
||||
List of WorkspaceFile instances
|
||||
"""
|
||||
effective_path = self._get_effective_path(path, include_all_sessions)
|
||||
db = workspace_db()
|
||||
|
||||
return await list_workspace_files(
|
||||
return await db.list_workspace_files(
|
||||
workspace_id=self.workspace_id,
|
||||
path_prefix=effective_path,
|
||||
limit=limit,
|
||||
@@ -332,7 +330,8 @@ class WorkspaceManager:
|
||||
Returns:
|
||||
True if deleted, False if not found
|
||||
"""
|
||||
file = await get_workspace_file(file_id, self.workspace_id)
|
||||
db = workspace_db()
|
||||
file = await db.get_workspace_file(file_id, self.workspace_id)
|
||||
if file is None:
|
||||
return False
|
||||
|
||||
@@ -345,7 +344,7 @@ class WorkspaceManager:
|
||||
# Continue with database soft-delete even if storage delete fails
|
||||
|
||||
# Soft-delete database record
|
||||
result = await soft_delete_workspace_file(file_id, self.workspace_id)
|
||||
result = await db.soft_delete_workspace_file(file_id, self.workspace_id)
|
||||
return result is not None
|
||||
|
||||
async def get_download_url(self, file_id: str, expires_in: int = 3600) -> str:
|
||||
@@ -362,7 +361,8 @@ class WorkspaceManager:
|
||||
Raises:
|
||||
FileNotFoundError: If file doesn't exist
|
||||
"""
|
||||
file = await get_workspace_file(file_id, self.workspace_id)
|
||||
db = workspace_db()
|
||||
file = await db.get_workspace_file(file_id, self.workspace_id)
|
||||
if file is None:
|
||||
raise FileNotFoundError(f"File not found: {file_id}")
|
||||
|
||||
@@ -379,7 +379,8 @@ class WorkspaceManager:
|
||||
Returns:
|
||||
WorkspaceFile instance or None
|
||||
"""
|
||||
return await get_workspace_file(file_id, self.workspace_id)
|
||||
db = workspace_db()
|
||||
return await db.get_workspace_file(file_id, self.workspace_id)
|
||||
|
||||
async def get_file_info_by_path(self, path: str) -> Optional[WorkspaceFile]:
|
||||
"""
|
||||
@@ -394,8 +395,9 @@ class WorkspaceManager:
|
||||
Returns:
|
||||
WorkspaceFile instance or None
|
||||
"""
|
||||
db = workspace_db()
|
||||
resolved_path = self._resolve_path(path)
|
||||
return await get_workspace_file_by_path(self.workspace_id, resolved_path)
|
||||
return await db.get_workspace_file_by_path(self.workspace_id, resolved_path)
|
||||
|
||||
async def get_file_count(
|
||||
self,
|
||||
@@ -417,7 +419,8 @@ class WorkspaceManager:
|
||||
Number of files
|
||||
"""
|
||||
effective_path = self._get_effective_path(path, include_all_sessions)
|
||||
db = workspace_db()
|
||||
|
||||
return await count_workspace_files(
|
||||
return await db.count_workspace_files(
|
||||
self.workspace_id, path_prefix=effective_path
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user