mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[F089] honest WorkSession agent_id nullability across the read path
The work_sessions.agent_id column is nullable=True with ondelete=SET
NULL — deleting an agent nulls the FK on every session it ever held. The
ORM annotation lied (Mapped[UUID] non-optional), the converter papered
over the lie (typing_cast to a non-optional UUID), and the response
model rejected None outright (WorkSessionResponse.agent_id: UUID). A
session whose agent had been deleted crashed the GET endpoint with a
pydantic ValidationError instead of serializing agent_id: null.
Make the read path honest end-to-end:
- WorkSessionTable.agent_id: Mapped[UUID | None] (matches the column).
- WorkSessionResponse.agent_id: UUID | None (serializes null, no crash).
- session_to_response passes agent_id via typing_cast('UUID | None', ...)
to bridge SQLAlchemy's UUID[Any] to stdlib uuid.UUID while preserving
None-ness (the cast stays for the same mypy-plugin reason every other
field uses one; it no longer narrows away None).
WorkSessionCreate.agent_id stays UUID — at create time the claiming
agent is always known. The unused WorkSession pydantic read model is
left as-is (never materialized from a DB row). task.py:_needs_revision_dev
already None-guards ws.agent_id via to_python_uuid (returns None -> skip).
This commit is contained in:
@@ -28,7 +28,9 @@ class WorkSessionResponse(BaseModel):
|
|||||||
id: UUID
|
id: UUID
|
||||||
project_id: UUID
|
project_id: UUID
|
||||||
task_id: UUID
|
task_id: UUID
|
||||||
agent_id: UUID
|
# Nullable in the DB (ondelete SET NULL when an agent is deleted); a session
|
||||||
|
# whose agent was deleted serializes agent_id as null, not a crash.
|
||||||
|
agent_id: UUID | None
|
||||||
|
|
||||||
# Branch management
|
# Branch management
|
||||||
branch_name: str
|
branch_name: str
|
||||||
@@ -129,7 +131,7 @@ def session_to_response(session: "WorkSessionTable") -> WorkSessionResponse:
|
|||||||
id=typing_cast("UUID", session.id),
|
id=typing_cast("UUID", session.id),
|
||||||
project_id=typing_cast("UUID", session.project_id),
|
project_id=typing_cast("UUID", session.project_id),
|
||||||
task_id=typing_cast("UUID", session.task_id),
|
task_id=typing_cast("UUID", session.task_id),
|
||||||
agent_id=typing_cast("UUID", session.agent_id),
|
agent_id=typing_cast("UUID | None", session.agent_id),
|
||||||
branch_name=str(session.branch_name),
|
branch_name=str(session.branch_name),
|
||||||
base_branch=str(session.base_branch),
|
base_branch=str(session.base_branch),
|
||||||
target_branch=str(session.target_branch),
|
target_branch=str(session.target_branch),
|
||||||
|
|||||||
+1
-1
@@ -816,7 +816,7 @@ class WorkSessionTable(Base):
|
|||||||
nullable=False,
|
nullable=False,
|
||||||
index=True,
|
index=True,
|
||||||
)
|
)
|
||||||
agent_id: Mapped[UUID] = mapped_column(
|
agent_id: Mapped[UUID | None] = mapped_column(
|
||||||
UUID(as_uuid=True),
|
UUID(as_uuid=True),
|
||||||
ForeignKey("agents.id", ondelete="SET NULL"),
|
ForeignKey("agents.id", ondelete="SET NULL"),
|
||||||
nullable=True,
|
nullable=True,
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
"""session_to_response / session_to_summary honesty for a null agent_id.
|
||||||
|
|
||||||
|
The ``work_sessions.agent_id`` column is ``nullable=True`` with
|
||||||
|
``ondelete="SET NULL"`` — if an agent row is deleted, the FK nulls out on every
|
||||||
|
session that agent ever held. The read path (``session_to_response`` ->
|
||||||
|
``WorkSessionResponse``) used to assume ``agent_id`` was always present: the
|
||||||
|
ORM ``Mapped[UUID]`` lied, the converter ``typing_cast("UUID", ...)``
|
||||||
|
papered over the lie, and ``WorkSessionResponse.agent_id: UUID`` rejected
|
||||||
|
``None`` outright. A session whose agent had been deleted therefore crashed
|
||||||
|
the GET endpoint with a pydantic ``ValidationError`` instead of serializing
|
||||||
|
``agent_id: null``. These tests pin the honest end-to-end read path.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from datetime import UTC, datetime
|
||||||
|
from uuid import UUID, uuid4
|
||||||
|
|
||||||
|
from roboco.api.schemas.work_session import (
|
||||||
|
WorkSessionResponse,
|
||||||
|
session_to_response,
|
||||||
|
session_to_summary,
|
||||||
|
)
|
||||||
|
from roboco.db.tables import WorkSessionTable
|
||||||
|
from roboco.models.work_session import WorkSessionStatus
|
||||||
|
|
||||||
|
|
||||||
|
def _make_session(agent_id: UUID | None) -> WorkSessionTable:
|
||||||
|
"""Build a detached WorkSessionTable row with an explicit agent_id."""
|
||||||
|
now = datetime.now(UTC)
|
||||||
|
return WorkSessionTable(
|
||||||
|
id=uuid4(),
|
||||||
|
project_id=uuid4(),
|
||||||
|
task_id=uuid4(),
|
||||||
|
agent_id=agent_id,
|
||||||
|
branch_name="feature/x",
|
||||||
|
base_branch="main",
|
||||||
|
target_branch="main",
|
||||||
|
started_at=now,
|
||||||
|
status=WorkSessionStatus.ACTIVE,
|
||||||
|
commits=[],
|
||||||
|
files_modified=[],
|
||||||
|
created_at=now,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_session_to_response_serializes_null_agent_id() -> None:
|
||||||
|
"""A SET-NULL'd session (agent deleted) must serialize agent_id as None."""
|
||||||
|
session = _make_session(agent_id=None)
|
||||||
|
result = session_to_response(session)
|
||||||
|
assert isinstance(result, WorkSessionResponse)
|
||||||
|
assert result.agent_id is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_session_to_response_preserves_present_agent_id() -> None:
|
||||||
|
"""A normal session still carries its agent_id through unchanged."""
|
||||||
|
agent_id = uuid4()
|
||||||
|
result = session_to_response(_make_session(agent_id=agent_id))
|
||||||
|
assert result.agent_id == agent_id
|
||||||
|
|
||||||
|
|
||||||
|
def test_session_to_summary_does_not_read_agent_id() -> None:
|
||||||
|
"""The summary view omits agent_id entirely, so a null agent must not raise."""
|
||||||
|
result = session_to_summary(_make_session(agent_id=None))
|
||||||
|
assert result.status == WorkSessionStatus.ACTIVE
|
||||||
Reference in New Issue
Block a user