Skill archive download failures return 200 JSON instead of an HTTP error

[VectorPeak]

Problem

The skill archive download endpoints return a ZIP file on success, but failed archive preparation currently returns a JSON error envelope with HTTP 200.

Affected endpoints:

• GET /api/v1/skills/archive?skill_name=...
• GET /api/v1/skills/{skill_name}/archive
• legacy GET /api/skills/download?name=...

This is confusing for callers that treat these endpoints as binary downloads. The dashboard calls the v1 query endpoint with responseType: 'blob', wraps the response as application/zip, and saves it as <skill>.zip. If the backend returns a 200 JSON error body, the browser can save an error payload as a fake ZIP archive.

Reproduction sketch

1. Request a skill archive for a local skill that cannot be exported, such as a missing skill or a plugin-provided/sandbox-only skill.
2. Backend SkillsService.prepare_skill_archive() raises SkillsServiceError.
3. _download_skill() catches the exception and returns error(str(exc)).
4. FastAPI serializes that dict as HTTP 200 JSON.
5. Blob/ZIP callers treat the response as a successful archive download.

Expected behavior

Failed skill archive downloads should return a non-2xx HTTP error while preserving the existing JSON error shape.

Actual behavior

Failed skill archive downloads can return HTTP 200 with a JSON error body.

[dosubot]

Confirmed. The root cause is in _download_skill() [1] — it catches SkillsServiceError and returns the error() dict directly, which FastAPI serializes as HTTP 200:

async def _download_skill(service: SkillsService, name: str):
    try:
        return _archive_response(service.prepare_skill_archive(name))
    except SkillsServiceError as exc:
        return error(str(exc))
    except Exception as exc:
        logger.error(str(exc), exc_info=True)
        return error(str(exc))

The error() helper [2] only builds a {"status": "error", "message": ...} dict — it doesn't set any HTTP status code.

A minimal fix would be to raise an HTTPException (or return a JSONResponse with an explicit status code) instead of returning the dict:

from fastapi.responses import JSONResponse

async def _download_skill(service: SkillsService, name: str):
    try:
        return _archive_response(service.prepare_skill_archive(name))
    except SkillsServiceError as exc:
        return JSONResponse(status_code=400, content=error(str(exc)))
    except Exception as exc:
        logger.error(str(exc), exc_info=True)
        return JSONResponse(status_code=500, content=error(str(exc)))

This preserves the existing JSON error shape while returning proper HTTP error codes. Worth noting that the same pattern exists in the _run() helper [3], so other skills API endpoints have the same issue — but those return JSON to JSON-expecting callers, making it less harmful than the archive case where the caller expects a binary blob.

Also, SkillsServiceError is a plain exception [4] without a status_code field, unlike the existing ApiError class [5]. An alternative approach would be to leverage ApiError (which already carries a status code) or add a status code to SkillsServiceError.

To reply, just mention @dosu.

---

Docs are dead. Just use Dosu.