Skip to content

Commit 441483c

Browse files
committed
fix: address PR review — add PATCH to CORS methods, fix semaphore leak
- CORS: add PATCH to allow_methods (assignment rules + agent registrations use PATCH routes) - Evaluate semaphore: use asyncio.shield + done callback so the semaphore slot stays acquired until the worker thread actually exits, not when the HTTP timeout fires
1 parent 995748c commit 441483c

File tree

2 files changed

+29
-19
lines changed

2 files changed

+29
-19
lines changed

src/edictum_server/main.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,7 @@ async def _worker_monitor(app: FastAPI) -> None:
122122
workers = app.state.background_workers
123123
if workers["approval_timeout"].done():
124124
logger.warning("Restarting crashed approval_timeout worker")
125-
workers["approval_timeout"] = asyncio.create_task(
126-
_approval_timeout_worker(app)
127-
)
125+
workers["approval_timeout"] = asyncio.create_task(_approval_timeout_worker(app))
128126
if workers["partition"].done():
129127
logger.warning("Restarting crashed partition worker")
130128
workers["partition"] = asyncio.create_task(_partition_worker())
@@ -405,7 +403,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
405403
CORSMiddleware,
406404
allow_origins=[o.strip() for o in _settings.cors_origins.split(",") if o.strip()],
407405
allow_credentials=True,
408-
allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"],
406+
allow_methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"],
409407
allow_headers=["Content-Type", "X-Requested-With", "Authorization", "X-Edictum-Agent-Id"],
410408
)
411409

@@ -456,17 +454,14 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
456454

457455
# --- Validation error handler: strip Pydantic internals ----------------------
458456
@app.exception_handler(RequestValidationError)
459-
async def validation_error_handler(
460-
_request: Request, exc: RequestValidationError
461-
) -> JSONResponse:
457+
async def validation_error_handler(_request: Request, exc: RequestValidationError) -> JSONResponse:
462458
"""Return 422 with sanitized error details.
463459
464460
Strips ``ctx`` and ``type`` fields from Pydantic errors to avoid
465461
leaking framework internals (L4 finding).
466462
"""
467463
sanitized = [
468-
{"loc": e.get("loc", []), "msg": e.get("msg", "Validation error")}
469-
for e in exc.errors()
464+
{"loc": e.get("loc", []), "msg": e.get("msg", "Validation error")} for e in exc.errors()
470465
]
471466
return JSONResponse(status_code=422, content={"detail": sanitized})
472467

src/edictum_server/routes/evaluate.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,39 @@ async def evaluate(
4242
status_code=429,
4343
detail="Too many concurrent evaluations. Try again shortly.",
4444
) from exc
45+
46+
# Create thread task independently so we can track it after timeout.
47+
# shield() prevents wait_for from cancelling the inner task, letting
48+
# the done-callback fire and release the semaphore when the thread
49+
# actually finishes — not when the HTTP response is sent.
50+
thread_task = asyncio.ensure_future(
51+
asyncio.to_thread(
52+
evaluate_contracts,
53+
yaml_content=body.yaml_content,
54+
tool_name=body.tool_name,
55+
tool_args=body.tool_args,
56+
environment=body.environment,
57+
principal_input=body.principal,
58+
)
59+
)
4560
try:
46-
return await asyncio.wait_for(
47-
asyncio.to_thread(
48-
evaluate_contracts,
49-
yaml_content=body.yaml_content,
50-
tool_name=body.tool_name,
51-
tool_args=body.tool_args,
52-
environment=body.environment,
53-
principal_input=body.principal,
54-
),
61+
result = await asyncio.wait_for(
62+
asyncio.shield(thread_task),
5563
timeout=_EVALUATE_TIMEOUT_SECONDS,
5664
)
5765
except TimeoutError as exc:
66+
# Thread still running — release semaphore only when it finishes.
67+
thread_task.add_done_callback(lambda _: _EVALUATE_SEMAPHORE.release())
5868
raise HTTPException(
5969
status_code=422,
6070
detail="Evaluation timed out — contract bundle may be too complex.",
6171
) from exc
6272
except ValueError as exc:
73+
_EVALUATE_SEMAPHORE.release()
6374
raise HTTPException(status_code=422, detail=str(exc)) from exc
64-
finally:
75+
except BaseException:
76+
_EVALUATE_SEMAPHORE.release()
77+
raise
78+
else:
6579
_EVALUATE_SEMAPHORE.release()
80+
return result

0 commit comments

Comments
 (0)