Skip to content

feat: artifact upload endpoint with pluggable storage backend (BGSTM #298) - #310

Closed
bg-playground with Copilot wants to merge 7 commits into
mainfrom
copilot/add-artifact-upload-endpoint
Closed

feat: artifact upload endpoint with pluggable storage backend (BGSTM #298)#310
bg-playground with Copilot wants to merge 7 commits into
mainfrom
copilot/add-artifact-upload-endpoint

Conversation

Copilot AI commented May 7, 2026

Copy link
Copy Markdown
Contributor

Adds POST /api/v1/external-results/artifact — the multipart artifact upload slice of the External Results API. Also merges the case-result endpoints (BGSTM #303, PR #307) as a prerequisite since the artifact FK depends on external_case_results.

Storage abstraction

  • StorageBackend ABC (put / url_for / delete) in backend/app/storage/base.py
  • LocalFsBackend — streams to disk via asyncio.to_thread, no third-party deps
  • S3Backend stub — importable but raises NotImplementedError on construction with a clear message
  • get_storage_backend() FastAPI Depends factory; singleton selected by settings.STORAGE_BACKEND

Endpoint

POST /api/v1/external-results/artifact accepts multipart/form-data:

case_result_id  UUID    (required) — must exist in external_case_results
kind            enum    screenshot | trace | video | log | other
filename        str     (optional, defaults to upload filename)
file            binary  the artifact body
  • Streams upload in 64 KB chunks; enforces STORAGE_MAX_UPLOAD_BYTES mid-stream → 413 on overflow
  • Content-type validated against allowlist → 415 on miss
  • case_result_id existence checked → 404 on miss
  • Storage key: external-artifacts/{case_result_id}/{uuid4()}-{safe_filename}
  • Returns 201 with full ArtifactResponse including permanent URL

Database

New external_case_artifacts table (migration j9k0l1m2n3o4, chained from i8j9k0l1m2n3):

  • FK case_result_id → external_case_results(id) ON DELETE CASCADE
  • FK runner_token_id → runner_tokens(id) for provenance
  • storage_key unique constraint

Settings additions

STORAGE_BACKEND: Literal["local", "s3"] = "local"
STORAGE_LOCAL_ROOT: Path = Path("./var/artifacts")
STORAGE_LOCAL_PUBLIC_BASE_URL: str = "http://localhost:8000/artifacts"
STORAGE_MAX_UPLOAD_BYTES: int = 52_428_800   # 50 MB
STORAGE_ALLOWED_CONTENT_TYPES: list[str] = [...]

Dev serving

StaticFiles mounted at /artifacts pointing to STORAGE_LOCAL_ROOT, gated by settings.STORAGE_BACKEND == "local" so production with S3 doesn't expose a local directory.

Other

  • ArtifactResponse.created_at renamed to uploaded_at to match the DB column name
  • python-multipart==0.0.27 added to requirements.txt (required for Form/File endpoints; 0.0.27 is the current patched version with no known CVEs)
  • 13 integration tests covering all 7 acceptance scenarios (happy path, 413, 415, 404, 422, 401/403, cascade delete)
Original prompt

Implement the artifact upload endpoint for the External Results API with a pluggable storage backend. Closes #298.

This is the next slice of the External Results API contract (parent: #291), following #299 (spec), #296 (auth), #300 (router/session), and #303 (case results) which are all merged.

Scope

Backend only. Add:

  1. A StorageBackend ABC with two concrete implementations
  2. A new POST /api/v1/external-results/artifact endpoint
  3. A new external_case_artifacts table + Alembic migration
  4. Settings additions for storage configuration
  5. Tests covering happy path + error cases

Do NOT touch: audit_log wiring (that's #297, comes next), the spec doc (already merged), case_results.py, auth.py, or any frontend code.

Files to add/modify

New files

  • backend/app/storage/__init__.py
  • backend/app/storage/base.pyStorageBackend ABC
  • backend/app/storage/local.pyLocalFsBackend (default for dev/tests)
  • backend/app/storage/s3.pyS3Backend stub that raises NotImplementedError clearly
  • backend/app/models/external_artifact.pyExternalCaseArtifact SQLAlchemy model
  • backend/app/crud/external_artifact.py — CRUD helpers
  • backend/alembic/versions/<new>_add_external_case_artifacts_table.py — migration
  • backend/tests/api/test_external_artifacts.py — integration tests

Modified files

  • backend/app/api/external_results.py — add the artifact endpoint and a Depends(get_storage_backend) dependency
  • backend/app/core/config.py (or wherever Settings lives) — add storage settings
  • backend/app/main.py — only if a static-files mount is needed for serving local artifacts in dev (see "Static serving" below)

Storage abstraction

# backend/app/storage/base.py
from abc import ABC, abstractmethod
from typing import BinaryIO

class StorageBackend(ABC):
    @abstractmethod
    async def put(self, key: str, body: BinaryIO, content_type: str) -> str:
        """Store the object and return a URL (signed if applicable)."""

    @abstractmethod
    async def url_for(self, key: str, expires_in: int = 3600) -> str:
        """Return a fetchable URL for the stored key."""

    @abstractmethod
    async def delete(self, key: str) -> None:
        """Remove the stored object."""
# backend/app/storage/local.py
class LocalFsBackend(StorageBackend):
    """Default backend for dev and tests. Writes files under settings.STORAGE_LOCAL_ROOT."""
    def __init__(self, root: Path, public_base_url: str):
        self.root = root
        self.public_base_url = public_base_url.rstrip("/")
        self.root.mkdir(parents=True, exist_ok=True)
    # implement put/url_for/delete
# backend/app/storage/s3.py
class S3Backend(StorageBackend):
    """Placeholder. Importable so settings.STORAGE_BACKEND='s3' fails fast with a clear error."""
    def __init__(self, *args, **kwargs):
        raise NotImplementedError(
            "S3Backend is not yet implemented. Use STORAGE_BACKEND='local' for now."
        )

Provide a get_storage_backend() dependency in backend/app/storage/__init__.py (or a new dependencies.py) that selects the backend based on settings.STORAGE_BACKEND and is injectable in FastAPI routes.

Settings additions

Add to the existing Settings class:

  • STORAGE_BACKEND: Literal["local", "s3"] = "local"
  • STORAGE_LOCAL_ROOT: Path = Path("./var/artifacts")
  • STORAGE_LOCAL_PUBLIC_BASE_URL: str = "http://localhost:8000/artifacts"
  • STORAGE_MAX_UPLOAD_BYTES: int = 52_428_800 # 50 MB
  • STORAGE_ALLOWED_CONTENT_TYPES: list[str] = ["image/png", "image/jpeg", "video/mp4", "video/webm", "application/zip", "text/plain", "application/json"]

Match the project's existing settings convention (likely pydantic-settings/BaseSettings).

Endpoint

POST /api/v1/external-results/artifactmultipart/form-data

Form fields:

  • case_result_id: UUID (required) — must exist in external_case_results
  • kind: str — enum screenshot | trace | video | log | other
  • filename: str (optional, defaults to upload's filename)

File field:

  • The uploaded file (use FastAPI UploadFile)

Behavior:

  • Validate kind against the enum (422 on miss)
  • Validate case_result_id exists (404 on miss)
  • Stream the upload — DO NOT read the full body into memory. Enforce STORAGE_MAX_UPLOAD_BYTES while streaming. On overflow, return 413.
  • Validate Content-Type against STORAGE_ALLOWED_CONTENT_TYPES (415 on miss)
  • Generate storage key as external-artifacts/{case_result_id}/{uuid4()}-{safe_filename}
  • Call backend.put(key, stream, content_type)
  • Persist a row in external_case_artifacts with: id, case_result_id (FK), kind, filename, content_type, size_bytes, storage_key, url, uploaded_at, runner_token_id (FK to the auth token, mirrors how case results capture provenance — check external_results.py and case_results.py for the existing pattern)
  • Return 201 with the new artifact row + permanent URL (fro...

This pull request was created from Copilot chat.

Copilot AI and others added 3 commits May 7, 2026 02:49
…t-upload-endpoint

Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…nd (BGSTM#298)

Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/e8dd1b96-4437-41c6-be62-2a9b050201cc

Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/e8dd1b96-4437-41c6-be62-2a9b050201cc

Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Copilot AI changed the title [WIP] Add artifact upload endpoint for External Results API feat: artifact upload endpoint with pluggable storage backend (BGSTM #298) May 7, 2026
Copilot AI requested a review from bg-playground May 7, 2026 03:04
@bg-playground
bg-playground marked this pull request as ready for review May 7, 2026 13:08
@bg-playground

Copy link
Copy Markdown
Owner

#311 is the known root cause of the E2E Playwright failures here. Merging this one and will address in that PR.

@bg-playground
bg-playground deleted the copilot/add-artifact-upload-endpoint branch May 7, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Endpoints] Artifact upload with pluggable storage backend

2 participants