diff --git a/apps/api/openapi.json b/apps/api/openapi.json index d51c3e0c0a4..a89d21bfb4d 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -3688,6 +3688,23 @@ "title": "ClearanceTemplateSummaryDTO", "type": "object" }, + "CloseVisitPresenceRequest": { + "additionalProperties": false, + "description": "Body for `POST /visits/{visit_id}/close-presence`.\n\nUnlike check-in and check-out, this one DOES name an actor, because\nclosing somebody else's record is the whole point of the command. The\ncaller is still the envelope's `principal_id`, so the record shows both\nwho was present and who ended it.", + "properties": { + "actor_id": { + "description": "Actor whose open presence entry is closed.", + "format": "uuid", + "title": "Actor Id", + "type": "string" + } + }, + "required": [ + "actor_id" + ], + "title": "CloseVisitPresenceRequest", + "type": "object" + }, "CompleteSealRepublishingBody": { "additionalProperties": false, "description": "Optional complete-seal-republishing request body.", @@ -45714,6 +45731,86 @@ ] } }, + "/visits/{visit_id}/close-presence": { + "post": { + "operationId": "post_visits_close_presence_visits__visit_id__close_presence_post", + "parameters": [ + { + "description": "Target Visit's id.", + "in": "path", + "name": "visit_id", + "required": true, + "schema": { + "description": "Target Visit's id.", + "format": "uuid", + "title": "Visit Id", + "type": "string" + } + }, + { + "description": "Legacy principal-id header (trust-the-proxy shape). When IDENTITY_PROVIDERS is configured (bearer-auth mode), this header is IGNORED and the verified bearer token from `BearerAuthMiddleware` (Authorization: Bearer) sets the principal. When no IdPs are configured (legacy mode), the application TRUSTS this header (no cryptographic verification) -- production deployments in legacy mode MUST front the API with an auth proxy that strips any client-supplied X-Principal-Id and sets it to the verified principal UUID. Behavior when absent: see Settings.require_authenticated_principal.", + "in": "header", + "name": "X-Principal-Id", + "required": false, + "schema": { + "anyOf": [ + { + "format": "uuid", + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Legacy principal-id header (trust-the-proxy shape). When IDENTITY_PROVIDERS is configured (bearer-auth mode), this header is IGNORED and the verified bearer token from `BearerAuthMiddleware` (Authorization: Bearer) sets the principal. When no IdPs are configured (legacy mode), the application TRUSTS this header (no cryptographic verification) -- production deployments in legacy mode MUST front the API with an auth proxy that strips any client-supplied X-Principal-Id and sets it to the verified principal UUID. Behavior when absent: see Settings.require_authenticated_principal.", + "title": "X-Principal-Id" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CloseVisitPresenceRequest" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "Successful Response" + }, + "403": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + }, + "description": "Authorize port denied the command." + }, + "404": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + }, + "description": "No Visit exists with the given id, OR the named actor has no open presence entry." + }, + "422": { + "description": "Request body failed schema validation." + } + }, + "summary": "Close another actor's presence entry on a Visit", + "tags": [ + "trust" + ] + } + }, "/visits/{visit_id}/complete": { "post": { "operationId": "post_visits_complete_visits__visit_id__complete_post", diff --git a/apps/api/src/cora/trust/aggregates/visit/__init__.py b/apps/api/src/cora/trust/aggregates/visit/__init__.py index 431f2304ff1..d6e9fa3fba6 100644 --- a/apps/api/src/cora/trust/aggregates/visit/__init__.py +++ b/apps/api/src/cora/trust/aggregates/visit/__init__.py @@ -22,6 +22,7 @@ VisitCompleted, VisitEvent, VisitHeld, + VisitPresenceClosed, VisitRegistered, VisitResumed, VisitStarted, @@ -97,6 +98,7 @@ "VisitNotFoundError", "VisitParentMismatchedSurfaceError", "VisitParentNotFoundError", + "VisitPresenceClosed", "VisitRegistered", "VisitResumed", "VisitStarted", diff --git a/apps/api/src/cora/trust/aggregates/visit/events.py b/apps/api/src/cora/trust/aggregates/visit/events.py index 625cef8434e..1be2905fdac 100644 --- a/apps/api/src/cora/trust/aggregates/visit/events.py +++ b/apps/api/src/cora/trust/aggregates/visit/events.py @@ -13,7 +13,8 @@ - `VisitVoided` -- any non-terminal -> Voided (+ reason) FHIR `entered-in-error` analog. -Presence events: `VisitCheckedIn` / `VisitCheckedOut`. Surface-control +Presence events: `VisitCheckedIn` / `VisitCheckedOut` / +`VisitPresenceClosed`. Surface-control events: `VisitSurfaceControlTaken` / `VisitSurfaceControlReleased`. `VisitRegistered.permitted_*` lists become `frozenset` on state in the @@ -178,6 +179,31 @@ class VisitCheckedOut: occurred_at: datetime +@dataclass(frozen=True) +class VisitPresenceClosed: + """Somebody else ended this actor's presence on the Visit. + + Structurally identical to `VisitCheckedOut`: the open `PresenceEntry` + gains a `check_out_at` by the same frozen-replace, and the evolver + handles both in one arm. A distinct type because the two acts differ in + CAUSE, the way `VisitCancelled` / `VisitAborted` / `VisitVoided` all + reach a terminal status and stay separate events. + + That distinction has to survive in the event TYPE rather than only in + the envelope's `command_name`, because presence is read as evidence of + who was at a beamline: "did this person leave, or did somebody close + their record for them" should be one predicate over the stream, not a + join against envelope metadata. + + The actor who did the closing is the envelope's `principal_id` and is + deliberately not duplicated into the payload. + """ + + visit_id: UUID + actor_id: UUID + occurred_at: datetime + + @dataclass(frozen=True) class VisitSurfaceControlTaken: """The Visit took operational control of the Surface. @@ -228,6 +254,7 @@ class VisitSurfaceControlReleased: | VisitVoided | VisitCheckedIn | VisitCheckedOut + | VisitPresenceClosed | VisitSurfaceControlTaken | VisitSurfaceControlReleased ) @@ -329,7 +356,10 @@ def to_payload(event: VisitEvent) -> dict[str, Any]: "mode": mode, "occurred_at": occurred_at.isoformat(), } - case VisitCheckedOut(visit_id=visit_id, actor_id=actor_id, occurred_at=occurred_at): + case ( + VisitCheckedOut(visit_id=visit_id, actor_id=actor_id, occurred_at=occurred_at) + | VisitPresenceClosed(visit_id=visit_id, actor_id=actor_id, occurred_at=occurred_at) + ): return { "visit_id": str(visit_id), "actor_id": str(actor_id), @@ -463,6 +493,15 @@ def _build_visit_registered() -> VisitRegistered: occurred_at=datetime.fromisoformat(payload["occurred_at"]), ), ) + case "VisitPresenceClosed": + return deserialize_or_raise( + "VisitPresenceClosed", + lambda: VisitPresenceClosed( + visit_id=UUID(payload["visit_id"]), + actor_id=UUID(payload["actor_id"]), + occurred_at=datetime.fromisoformat(payload["occurred_at"]), + ), + ) case "VisitSurfaceControlTaken": return deserialize_or_raise( "VisitSurfaceControlTaken", @@ -495,6 +534,7 @@ def _build_visit_registered() -> VisitRegistered: "VisitCompleted", "VisitEvent", "VisitHeld", + "VisitPresenceClosed", "VisitRegistered", "VisitResumed", "VisitStarted", diff --git a/apps/api/src/cora/trust/aggregates/visit/evolver.py b/apps/api/src/cora/trust/aggregates/visit/evolver.py index 88666617fd3..b2620698467 100644 --- a/apps/api/src/cora/trust/aggregates/visit/evolver.py +++ b/apps/api/src/cora/trust/aggregates/visit/evolver.py @@ -13,6 +13,7 @@ from collections.abc import Sequence from dataclasses import replace +from datetime import datetime from typing import assert_never from cora.trust.aggregates.visit.events import ( @@ -24,6 +25,7 @@ VisitCompleted, VisitEvent, VisitHeld, + VisitPresenceClosed, VisitRegistered, VisitResumed, VisitStarted, @@ -40,6 +42,27 @@ ) +def _closed_at( + entries: frozenset[PresenceEntry], occurred_at: datetime +) -> frozenset[PresenceEntry]: + """Close every open presence entry at `occurred_at`. + + A Visit reaching a terminal state ends presence by implication: nobody is + at a beamline for a Visit that has completed, been cancelled, aborted, or + voided. Deriving that here rather than emitting per-actor close events + keeps the terminal deciders ignorant of presence, and means the closing + timestamp is exactly the transition's own, with no second clock reading. + + Same frozen-replace shape as the `VisitCheckedOut` arm. Returns `entries` + unchanged when nothing is open, so the common case allocates nothing. + """ + open_entries = {e for e in entries if e.check_out_at is None} + if not open_entries: + return entries + closed = {replace(e, check_out_at=occurred_at) for e in open_entries} + return (entries - open_entries) | closed + + def evolve(state: Visit | None, event: VisitEvent) -> Visit: """Apply one event to the current state.""" match event: @@ -79,18 +102,37 @@ def evolve(state: Visit | None, event: VisitEvent) -> Visit: assert state is not None, "VisitResumed requires prior state" # Preserve last_status_reason audit breadcrumb across resume. return replace(state, status=VisitStatus.IN_PROGRESS) - case VisitCompleted(): + case VisitCompleted(occurred_at=occurred_at): assert state is not None, "VisitCompleted requires prior state" - return replace(state, status=VisitStatus.COMPLETED) - case VisitCancelled(reason=reason): + return replace( + state, + status=VisitStatus.COMPLETED, + presence_entries=_closed_at(state.presence_entries, occurred_at), + ) + case VisitCancelled(reason=reason, occurred_at=occurred_at): assert state is not None, "VisitCancelled requires prior state" - return replace(state, status=VisitStatus.CANCELLED, last_status_reason=reason) - case VisitAborted(reason=reason): + return replace( + state, + status=VisitStatus.CANCELLED, + last_status_reason=reason, + presence_entries=_closed_at(state.presence_entries, occurred_at), + ) + case VisitAborted(reason=reason, occurred_at=occurred_at): assert state is not None, "VisitAborted requires prior state" - return replace(state, status=VisitStatus.ABORTED, last_status_reason=reason) - case VisitVoided(reason=reason): + return replace( + state, + status=VisitStatus.ABORTED, + last_status_reason=reason, + presence_entries=_closed_at(state.presence_entries, occurred_at), + ) + case VisitVoided(reason=reason, occurred_at=occurred_at): assert state is not None, "VisitVoided requires prior state" - return replace(state, status=VisitStatus.VOIDED, last_status_reason=reason) + return replace( + state, + status=VisitStatus.VOIDED, + last_status_reason=reason, + presence_entries=_closed_at(state.presence_entries, occurred_at), + ) case VisitCheckedIn(actor_id=actor_id, mode=mode, occurred_at=occurred_at): assert state is not None, "VisitCheckedIn requires prior state" # Set-union add. Decider has already guarded against open-entry duplicates; @@ -102,8 +144,13 @@ def evolve(state: Visit | None, event: VisitEvent) -> Visit: check_out_at=None, ) return replace(state, presence_entries=state.presence_entries | {new_entry}) - case VisitCheckedOut(actor_id=actor_id, occurred_at=occurred_at): - assert state is not None, "VisitCheckedOut requires prior state" + case ( + VisitCheckedOut(actor_id=actor_id, occurred_at=occurred_at) + | VisitPresenceClosed(actor_id=actor_id, occurred_at=occurred_at) + ): + # One arm for both: the state change is identical and only the + # cause differs, which the event TYPE already carries. + assert state is not None, "presence-closing event requires prior state" # Frozen-replace: find the actor's OPEN entry, remove it, insert a new # entry with check_out_at populated. Old + new are distinct frozenset # members because PresenceEntry's hash covers all 4 fields. Decider diff --git a/apps/api/src/cora/trust/features/close_visit_presence/__init__.py b/apps/api/src/cora/trust/features/close_visit_presence/__init__.py new file mode 100644 index 00000000000..4b92a42368e --- /dev/null +++ b/apps/api/src/cora/trust/features/close_visit_presence/__init__.py @@ -0,0 +1,9 @@ +"""Vertical slice for the `CloseVisitPresence` command.""" + +from cora.trust.features.close_visit_presence import tool +from cora.trust.features.close_visit_presence.command import CloseVisitPresence +from cora.trust.features.close_visit_presence.decider import decide +from cora.trust.features.close_visit_presence.handler import Handler, bind +from cora.trust.features.close_visit_presence.route import router + +__all__ = ["CloseVisitPresence", "Handler", "bind", "decide", "router", "tool"] diff --git a/apps/api/src/cora/trust/features/close_visit_presence/command.py b/apps/api/src/cora/trust/features/close_visit_presence/command.py new file mode 100644 index 00000000000..b04771a70ff --- /dev/null +++ b/apps/api/src/cora/trust/features/close_visit_presence/command.py @@ -0,0 +1,28 @@ +"""The `CloseVisitPresence` command -- intent dataclass. + +Closes ANOTHER actor's open presence entry. This is the deliberate +counterpart to `CheckOutVisit`, which closes only the caller's own. + +The two are separate commands, not one command with a nullable actor, so +that closing somebody else's record needs its own Policy grant. Somebody +who forgot to check out and went home leaves an entry that only this +command can close while the Visit is still running; once the Visit reaches +a terminal state the evolver closes every open entry anyway, so this exists +for the mid-Visit case. + +Attribution is the event envelope's, not a payload field: the envelope +already carries `principal_id` and `command_name`, so a `VisitCheckedOut` +recorded under `CloseVisitPresence` is distinguishable from a self-checkout +without duplicating the caller into the payload. +""" + +from dataclasses import dataclass +from uuid import UUID + + +@dataclass(frozen=True) +class CloseVisitPresence: + """Close `actor_id`'s currently-open presence entry on the Visit.""" + + visit_id: UUID + actor_id: UUID diff --git a/apps/api/src/cora/trust/features/close_visit_presence/decider.py b/apps/api/src/cora/trust/features/close_visit_presence/decider.py new file mode 100644 index 00000000000..9d3accc67d9 --- /dev/null +++ b/apps/api/src/cora/trust/features/close_visit_presence/decider.py @@ -0,0 +1,56 @@ +"""Pure decider for the `CloseVisitPresence` command. + +Requires an open presence entry for the NAMED actor. Does not require any +particular `Visit.status`: the mid-Visit case is the reason this exists, and +closing a lingering entry on an already-terminal Visit is harmless because the +evolver has closed it already, so the guard below simply refuses. + +Emits `VisitPresenceClosed`, NOT the `VisitCheckedOut` that `check_out_visit` +emits. The state change is identical and the evolver handles both in one arm, +but the two acts differ in cause, and presence is read as evidence of who was at +a beamline: "did this person leave, or did somebody close their record" has to be +one predicate over the event stream rather than a join against envelope metadata. + +Who did the closing is still the envelope's `principal_id` and is deliberately +not duplicated into the payload; under the fold-symmetry convention a `*_by` +field is attribution that would then demand a paired timestamp it does not need. +""" + +from datetime import datetime + +from cora.trust.aggregates.visit import ( + Visit, + VisitActorNotCheckedInError, + VisitNotFoundError, + VisitPresenceClosed, +) +from cora.trust.features.close_visit_presence.command import CloseVisitPresence + + +def decide( + state: Visit | None, + command: CloseVisitPresence, + *, + now: datetime, +) -> list[VisitPresenceClosed]: + """Decide events for closing another actor's presence entry. + + Invariants: + - State must not be None -> VisitNotFoundError + - Named actor must have an open presence entry + -> VisitActorNotCheckedInError + """ + if state is None: + raise VisitNotFoundError(command.visit_id) + open_entry_exists = any( + e.actor_id == command.actor_id and e.check_out_at is None for e in state.presence_entries + ) + if not open_entry_exists: + raise VisitActorNotCheckedInError(visit_id=state.id, actor_id=command.actor_id) + return [ + VisitPresenceClosed( + visit_id=state.id, + actor_id=command.actor_id, + occurred_at=now, + ) + ] diff --git a/apps/api/src/cora/trust/features/close_visit_presence/handler.py b/apps/api/src/cora/trust/features/close_visit_presence/handler.py new file mode 100644 index 00000000000..8b73bbf4da0 --- /dev/null +++ b/apps/api/src/cora/trust/features/close_visit_presence/handler.py @@ -0,0 +1,38 @@ +"""Application handler for the `close_visit_presence` slice. + +No `actor_kwarg`: the actor whose entry closes is named by the command, and +the caller reaches the record through the event envelope's `principal_id`. +""" + +from typing import Protocol +from uuid import UUID + +from cora.infrastructure.kernel import Kernel +from cora.infrastructure.routing import NIL_SENTINEL_ID +from cora.trust._visit_update_handler import make_visit_update_handler +from cora.trust.features.close_visit_presence.command import CloseVisitPresence +from cora.trust.features.close_visit_presence.decider import decide + + +class Handler(Protocol): + """Callable interface every close_visit_presence handler implements.""" + + async def __call__( + self, + command: CloseVisitPresence, + *, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None = None, + surface_id: UUID = NIL_SENTINEL_ID, + ) -> None: ... + + +def bind(deps: Kernel) -> Handler: + """Build a close_visit_presence handler closed over the shared deps.""" + return make_visit_update_handler( + deps, + command_name="CloseVisitPresence", + log_prefix="close_visit_presence", + decide_fn=decide, + ) diff --git a/apps/api/src/cora/trust/features/close_visit_presence/route.py b/apps/api/src/cora/trust/features/close_visit_presence/route.py new file mode 100644 index 00000000000..aab22e9861e --- /dev/null +++ b/apps/api/src/cora/trust/features/close_visit_presence/route.py @@ -0,0 +1,78 @@ +"""HTTP route for the `close_visit_presence` slice. + +Action endpoint at `POST /visits/{visit_id}/close-presence`. Body names the +actor whose entry is being closed. 204 on success. +""" + +from typing import Annotated +from uuid import UUID + +from fastapi import APIRouter, Depends, Path, Request, status +from pydantic import BaseModel, Field + +from cora.infrastructure.routing import ( + ErrorResponse, + get_correlation_id, + get_principal_id, + get_surface_id, +) +from cora.trust.features.close_visit_presence.command import CloseVisitPresence +from cora.trust.features.close_visit_presence.handler import Handler + + +class CloseVisitPresenceRequest(BaseModel): + """Body for `POST /visits/{visit_id}/close-presence`. + + Unlike check-in and check-out, this one DOES name an actor, because + closing somebody else's record is the whole point of the command. The + caller is still the envelope's `principal_id`, so the record shows both + who was present and who ended it. + """ + + model_config = {"extra": "forbid"} + + actor_id: UUID = Field(..., description="Actor whose open presence entry is closed.") + + +def _get_handler(request: Request) -> Handler: + handler: Handler = request.app.state.trust.close_visit_presence + return handler + + +router = APIRouter(tags=["trust"]) + + +@router.post( + "/visits/{visit_id}/close-presence", + status_code=status.HTTP_204_NO_CONTENT, + responses={ + status.HTTP_403_FORBIDDEN: { + "model": ErrorResponse, + "description": "Authorize port denied the command.", + }, + status.HTTP_404_NOT_FOUND: { + "model": ErrorResponse, + "description": ( + "No Visit exists with the given id, OR the named actor has no open presence entry." + ), + }, + status.HTTP_422_UNPROCESSABLE_CONTENT: { + "description": "Request body failed schema validation.", + }, + }, + summary="Close another actor's presence entry on a Visit", +) +async def post_visits_close_presence( + visit_id: Annotated[UUID, Path(description="Target Visit's id.")], + body: CloseVisitPresenceRequest, + handler: Annotated[Handler, Depends(_get_handler)], + cid: Annotated[UUID, Depends(get_correlation_id)], + principal_id: Annotated[UUID, Depends(get_principal_id)], + surface_id: Annotated[UUID, Depends(get_surface_id)], +) -> None: + await handler( + CloseVisitPresence(visit_id=visit_id, actor_id=body.actor_id), + principal_id=principal_id, + correlation_id=cid, + surface_id=surface_id, + ) diff --git a/apps/api/src/cora/trust/features/close_visit_presence/tool.py b/apps/api/src/cora/trust/features/close_visit_presence/tool.py new file mode 100644 index 00000000000..0bec8f3972e --- /dev/null +++ b/apps/api/src/cora/trust/features/close_visit_presence/tool.py @@ -0,0 +1,49 @@ +"""MCP tool for the `close_visit_presence` slice.""" + +from collections.abc import Callable +from typing import Annotated, Any +from uuid import UUID + +from mcp.server.fastmcp import Context, FastMCP +from pydantic import BaseModel, Field + +from cora.infrastructure.mcp_principal import get_mcp_principal_id +from cora.infrastructure.observability import current_correlation_id +from cora.infrastructure.routing import get_mcp_surface_id +from cora.trust.features.close_visit_presence.command import CloseVisitPresence +from cora.trust.features.close_visit_presence.handler import Handler + + +class CloseVisitPresenceOutput(BaseModel): + """Structured output of the `close_visit_presence` MCP tool.""" + + visit_id: UUID + actor_id: UUID + + +def register(mcp: FastMCP, *, get_handler: Callable[[], Handler]) -> None: + """Register the `close_visit_presence` tool on the given MCP server.""" + + @mcp.tool( + name="close_visit_presence", + description=( + "Close ANOTHER actor's open presence entry on a Visit, for somebody " + "who left without checking out. To close your own, use " + "check_out_visit. The named actor must have an open entry. A Visit " + "that reaches a terminal state closes every open entry on its own, " + "so this is for the mid-Visit case." + ), + ) + async def close_visit_presence_tool( # pyright: ignore[reportUnusedFunction] + ctx: Context[Any, Any, Any], + visit_id: Annotated[UUID, Field(description="Target Visit's id.")], + actor_id: Annotated[UUID, Field(description="Actor whose open presence entry is closed.")], + ) -> CloseVisitPresenceOutput: + handler = get_handler() + await handler( + CloseVisitPresence(visit_id=visit_id, actor_id=actor_id), + principal_id=get_mcp_principal_id(ctx), + correlation_id=current_correlation_id(), + surface_id=get_mcp_surface_id(), + ) + return CloseVisitPresenceOutput(visit_id=visit_id, actor_id=actor_id) diff --git a/apps/api/src/cora/trust/projections/visit_presence.py b/apps/api/src/cora/trust/projections/visit_presence.py index 4ccd9422194..bd1feb5c732 100644 --- a/apps/api/src/cora/trust/projections/visit_presence.py +++ b/apps/api/src/cora/trust/projections/visit_presence.py @@ -19,7 +19,11 @@ from cora.infrastructure.ports.event_store import StoredEvent from cora.infrastructure.projection.handler import ConnectionLike -_SUBSCRIBED: frozenset[str] = frozenset({"VisitCheckedIn", "VisitCheckedOut"}) +_TERMINAL: frozenset[str] = frozenset( + {"VisitCompleted", "VisitCancelled", "VisitAborted", "VisitVoided"} +) +_CLOSING: frozenset[str] = frozenset({"VisitCheckedOut", "VisitPresenceClosed"}) +_SUBSCRIBED: frozenset[str] = frozenset({"VisitCheckedIn"}) | _CLOSING | _TERMINAL _INSERT_PRESENCE_SQL = """ INSERT INTO proj_trust_visit_presence @@ -36,6 +40,18 @@ WHERE visit_id = $1 AND actor_id = $2 AND check_out_at IS NULL """ +# A Visit reaching a terminal state ends presence for EVERY actor still open on +# it, which is the same rule the aggregate evolver applies in `_closed_at`. The +# two must agree: the aggregate is what a decider reads and this table is what a +# query reads, and a Visit that folds to "nobody present" while the read model +# still shows open rows would make presence unusable as evidence. Idempotent for +# the same reason as the single-actor update: a replay matches zero open rows. +_CLOSE_ALL_PRESENCE_SQL = """ +UPDATE proj_trust_visit_presence +SET check_out_at = $2, updated_at = now() +WHERE visit_id = $1 AND check_out_at IS NULL +""" + class VisitPresenceProjection: """Maintains the `proj_trust_visit_presence` read model.""" @@ -53,9 +69,14 @@ async def apply( payload = event.payload visit_id = UUID(payload["visit_id"]) - actor_id = UUID(payload["actor_id"]) occurred_at = datetime.fromisoformat(payload["occurred_at"]) + if event.event_type in _TERMINAL: + await conn.execute(_CLOSE_ALL_PRESENCE_SQL, visit_id, occurred_at) + return + + actor_id = UUID(payload["actor_id"]) + match event.event_type: case "VisitCheckedIn": await conn.execute( @@ -65,7 +86,9 @@ async def apply( payload["mode"], occurred_at, ) - case "VisitCheckedOut": + case "VisitCheckedOut" | "VisitPresenceClosed": + # Same row update for both: they differ in cause, which the + # event type already records, not in what happens to the row. await conn.execute( _UPDATE_PRESENCE_SQL, visit_id, diff --git a/apps/api/src/cora/trust/routes.py b/apps/api/src/cora/trust/routes.py index 59361134d7f..2bbdb37a706 100644 --- a/apps/api/src/cora/trust/routes.py +++ b/apps/api/src/cora/trust/routes.py @@ -89,6 +89,7 @@ cancel_visit, check_in_visit, check_out_visit, + close_visit_presence, complete_visit, define_conduit, define_policy, @@ -223,6 +224,7 @@ def register_trust_routes(app: FastAPI) -> None: # Visit presence slices. app.include_router(check_in_visit.router) app.include_router(check_out_visit.router) + app.include_router(close_visit_presence.router) # Visit Surface-control slices. app.include_router(take_control_of_surface.router) app.include_router(release_control_of_surface.router) diff --git a/apps/api/src/cora/trust/tools.py b/apps/api/src/cora/trust/tools.py index 45d72859b52..ee4fd351655 100644 --- a/apps/api/src/cora/trust/tools.py +++ b/apps/api/src/cora/trust/tools.py @@ -14,6 +14,7 @@ from cora.trust.features.cancel_visit import tool as cancel_visit_tool from cora.trust.features.check_in_visit import tool as check_in_visit_tool from cora.trust.features.check_out_visit import tool as check_out_visit_tool +from cora.trust.features.close_visit_presence import tool as close_visit_presence_tool from cora.trust.features.complete_visit import tool as complete_visit_tool from cora.trust.features.define_conduit import tool as define_conduit_tool from cora.trust.features.define_policy import tool as define_policy_tool @@ -70,6 +71,7 @@ def register_trust_tools( # Visit presence tools. check_in_visit_tool.register(mcp, get_handler=lambda: get_handlers().check_in_visit) check_out_visit_tool.register(mcp, get_handler=lambda: get_handlers().check_out_visit) + close_visit_presence_tool.register(mcp, get_handler=lambda: get_handlers().close_visit_presence) # Visit Surface-control tools. take_control_of_surface_tool.register( mcp, get_handler=lambda: get_handlers().take_control_of_surface diff --git a/apps/api/src/cora/trust/wire.py b/apps/api/src/cora/trust/wire.py index 552d47023fe..01677c51092 100644 --- a/apps/api/src/cora/trust/wire.py +++ b/apps/api/src/cora/trust/wire.py @@ -28,6 +28,7 @@ cancel_visit, check_in_visit, check_out_visit, + close_visit_presence, complete_visit, define_conduit, define_policy, @@ -85,6 +86,7 @@ class TrustHandlers: void_visit: void_visit.Handler check_in_visit: check_in_visit.Handler check_out_visit: check_out_visit.Handler + close_visit_presence: close_visit_presence.Handler take_control_of_surface: take_control_of_surface.Handler release_control_of_surface: release_control_of_surface.Handler revoke_grant: revoke_grant.Handler @@ -212,6 +214,11 @@ def wire_trust(deps: Kernel) -> TrustHandlers: command_name="CheckOutVisit", bc=_BC, ), + close_visit_presence=with_tracing( + close_visit_presence.bind(deps), + command_name="CloseVisitPresence", + bc=_BC, + ), take_control_of_surface=with_tracing( take_control_of_surface.bind(deps), command_name="TakeControlOfSurface", diff --git a/apps/api/tests/contract/test_visit_endpoints.py b/apps/api/tests/contract/test_visit_endpoints.py index 3e229c52b0e..dfd31a3406a 100644 --- a/apps/api/tests/contract/test_visit_endpoints.py +++ b/apps/api/tests/contract/test_visit_endpoints.py @@ -1,9 +1,9 @@ -"""HTTP contract tests for the 13 Visit endpoints. +"""HTTP contract tests for the 14 Visit endpoints. Consolidated coverage file: covers `register_visit`, `record_visit_arrival`, `start_visit`, `hold_visit`, `resume_visit`, `complete_visit`, `cancel_visit`, `abort_visit`, `void_visit`, `check_in_visit`, -`check_out_visit`, `take_control_of_surface`, +`check_out_visit`, `close_visit_presence`, `take_control_of_surface`, `release_control_of_surface` per the arch-fitness substring-match rule. Pins the REST surface: status codes, body shapes, FSM-walk happy path, 404 / 409 / 400 error mappings. @@ -461,3 +461,40 @@ def test_check_out_rejects_a_body_naming_another_actor() -> None: json={"actor_id": str(uuid4())}, ) assert response.status_code == 422 + + +@pytest.mark.contract +def test_close_presence_returns_204_for_another_actor() -> None: + """The one thing check-out cannot do: end somebody else's presence.""" + with TestClient(create_app()) as client: + vid = _register_visit(client) + client.post(f"/visits/{vid}/record-arrival") + client.post(f"/visits/{vid}/check-in", json={"mode": "physical"}) + # The caller is the test principal, so its own entry is the open one. + response = client.post( + f"/visits/{vid}/close-presence", + json={"actor_id": "00000000-0000-0000-0000-000000000000"}, + ) + assert response.status_code == 204 + + +@pytest.mark.contract +def test_close_presence_returns_404_when_named_actor_has_no_open_entry() -> None: + with TestClient(create_app()) as client: + vid = _register_visit(client) + client.post(f"/visits/{vid}/record-arrival") + response = client.post( + f"/visits/{vid}/close-presence", + json={"actor_id": str(uuid4())}, + ) + assert response.status_code == 404 + + +@pytest.mark.contract +def test_close_presence_requires_an_actor_id() -> None: + """Unlike check-in and check-out, naming the actor IS the intent here.""" + with TestClient(create_app()) as client: + vid = _register_visit(client) + client.post(f"/visits/{vid}/record-arrival") + response = client.post(f"/visits/{vid}/close-presence", json={}) + assert response.status_code == 422 diff --git a/apps/api/tests/contract/test_visit_mcp_tools.py b/apps/api/tests/contract/test_visit_mcp_tools.py index 4de1be30923..a25f93ffbed 100644 --- a/apps/api/tests/contract/test_visit_mcp_tools.py +++ b/apps/api/tests/contract/test_visit_mcp_tools.py @@ -3,7 +3,7 @@ Consolidated coverage file: covers `register_visit`, `record_visit_arrival`, `start_visit`, `hold_visit`, `resume_visit`, `complete_visit`, `cancel_visit`, `abort_visit`, `void_visit`, `check_in_visit`, -`check_out_visit`, `take_control_of_surface`, +`check_out_visit`, `close_visit_presence`, `take_control_of_surface`, `release_control_of_surface` per the arch-fitness substring-match rule. Pins the MCP-tool surface: registration, structured output shape, isError on not-found. @@ -52,6 +52,7 @@ def _register_visit_via_rest(client: TestClient) -> str: # Presence tools. "check_in_visit", "check_out_visit", + "close_visit_presence", # Surface-control tools. "take_control_of_surface", "release_control_of_surface", @@ -114,9 +115,13 @@ def test_mcp_lifecycle_tool_returns_iserror_when_visit_not_found(tool_name: str) arguments: dict[str, str] = {"visit_id": str(uuid4())} if tool_name in {"hold_visit", "cancel_visit", "abort_visit", "void_visit"}: arguments["reason"] = "r" - # Presence tools name no actor: the caller checks itself in or out. + # Check-in and check-out name no actor: the caller checks itself in or + # out. close_visit_presence is the exception, and naming the target IS + # its intent. if tool_name == "check_in_visit": arguments["mode"] = "physical" + if tool_name == "close_visit_presence": + arguments["actor_id"] = str(uuid4()) # Surface-control tools carry surface_id. if tool_name in {"take_control_of_surface", "release_control_of_surface"}: arguments["surface_id"] = str(uuid4()) diff --git a/apps/api/tests/unit/trust/test_close_visit_presence_decider_properties.py b/apps/api/tests/unit/trust/test_close_visit_presence_decider_properties.py new file mode 100644 index 00000000000..10b1103ceca --- /dev/null +++ b/apps/api/tests/unit/trust/test_close_visit_presence_decider_properties.py @@ -0,0 +1,182 @@ +"""Property-based tests for `close_visit_presence.decide` (Trust BC, Visit). + +Complements the example-based `visit/test_close_visit_presence_decider.py`. +The decider closes ANOTHER actor's entry, so its shape is: + + (state, command, now) -> list[VisitCheckedOut] + +Load-bearing properties, all chosen for what distinguishes this slice from +`check_out_visit` rather than what it shares: + + - Targets the NAMED actor, never the caller. The command carries the + actor, and the emitted event must key on `command.actor_id` whatever + else is present on the Visit. + - Bystander isolation: closing one actor's entry never emits an event + naming a different actor, even when several are open at once. This is + the property that would catch a decider closing "the first open entry" + instead of the named one. + - Existence guard: a None state always raises `VisitNotFoundError`. + - Absence partition: an actor with no open entry always raises + `VisitActorNotCheckedInError` carrying the state's id and the command's + actor_id, whether the set is empty or holds only other actors. + - Lifecycle independence across non-terminal statuses: no status guard, + an open entry is the only precondition. Terminal statuses are excluded + because the evolver has already closed every entry by then, so that + combination is unreachable rather than merely untested. + - Pure: same inputs return equal results (no clock leakage). +""" + +from __future__ import annotations + +from dataclasses import replace +from typing import TYPE_CHECKING + +import pytest +from hypothesis import assume, given +from hypothesis import strategies as st + +from cora.trust.aggregates.visit import ( + PresenceEntry, + PresenceMode, + Visit, + VisitActorNotCheckedInError, + VisitNotFoundError, + VisitPresenceClosed, + VisitStatus, +) +from cora.trust.features.close_visit_presence import CloseVisitPresence +from cora.trust.features.close_visit_presence.decider import decide +from tests._strategies import aware_datetimes +from tests.unit.trust.visit._fixtures import VISIT_ID, make_visit + +if TYPE_CHECKING: + from datetime import datetime + from uuid import UUID + +# Terminal statuses are absent deliberately, and `make_visit` refuses to build +# them. A terminal Visit has already had every open entry closed by the +# evolver's `_closed_at`, so "terminal Visit WITH an open entry" is a state the +# fold cannot produce; asserting over it would test a fiction. +_NON_TERMINAL_STATUSES = ( + VisitStatus.PLANNED, + VisitStatus.ARRIVED, + VisitStatus.IN_PROGRESS, + VisitStatus.ON_HOLD, +) + + +def _state_with_open_entries( + *, + actor_ids: frozenset[UUID], + check_in_at: datetime, + status: VisitStatus = VisitStatus.IN_PROGRESS, +) -> Visit: + base = make_visit(status) + return replace( + base, + presence_entries=frozenset( + PresenceEntry( + actor_id=a, + mode=PresenceMode.PHYSICAL, + check_in_at=check_in_at, + check_out_at=None, + ) + for a in actor_ids + ), + ) + + +@pytest.mark.unit +@given(target=st.uuids(), bystanders=st.frozensets(st.uuids(), max_size=4), now=aware_datetimes()) +def test_close_presence_always_names_the_commanded_actor( + target: UUID, bystanders: frozenset[UUID], now: datetime +) -> None: + assume(target not in bystanders) + state = _state_with_open_entries(actor_ids=frozenset({target}) | bystanders, check_in_at=now) + events = decide( + state=state, + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=target), + now=now, + ) + [e] = events + assert isinstance(e, VisitPresenceClosed) + assert e.actor_id == target + assert e.visit_id == state.id + assert e.occurred_at == now + + +@pytest.mark.unit +@given( + target=st.uuids(), + bystanders=st.frozensets(st.uuids(), min_size=1, max_size=4), + now=aware_datetimes(), +) +def test_close_presence_never_touches_a_bystander( + target: UUID, bystanders: frozenset[UUID], now: datetime +) -> None: + """Catches a decider that closes the first open entry rather than the named one.""" + assume(target not in bystanders) + state = _state_with_open_entries(actor_ids=frozenset({target}) | bystanders, check_in_at=now) + events = decide( + state=state, + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=target), + now=now, + ) + assert all(e.actor_id not in bystanders for e in events) + + +@pytest.mark.unit +@given(actor_id=st.uuids(), now=aware_datetimes()) +def test_close_presence_on_absent_state_always_raises_not_found( + actor_id: UUID, now: datetime +) -> None: + with pytest.raises(VisitNotFoundError): + decide( + state=None, + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=actor_id), + now=now, + ) + + +@pytest.mark.unit +@given(target=st.uuids(), others=st.frozensets(st.uuids(), max_size=4), now=aware_datetimes()) +def test_close_presence_raises_when_target_has_no_open_entry( + target: UUID, others: frozenset[UUID], now: datetime +) -> None: + assume(target not in others) + state = _state_with_open_entries(actor_ids=others, check_in_at=now) + with pytest.raises(VisitActorNotCheckedInError) as exc: + decide( + state=state, + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=target), + now=now, + ) + assert exc.value.visit_id == state.id + assert exc.value.actor_id == target + + +@pytest.mark.unit +@given(actor_id=st.uuids(), status=st.sampled_from(_NON_TERMINAL_STATUSES), now=aware_datetimes()) +def test_close_presence_is_lifecycle_independent( + actor_id: UUID, status: VisitStatus, now: datetime +) -> None: + """No status guard: an open entry is the only precondition.""" + state = _state_with_open_entries( + actor_ids=frozenset({actor_id}), check_in_at=now, status=status + ) + events = decide( + state=state, + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=actor_id), + now=now, + ) + assert len(events) == 1 + + +@pytest.mark.unit +@given(actor_id=st.uuids(), now=aware_datetimes()) +def test_close_presence_is_pure_same_input_same_output(actor_id: UUID, now: datetime) -> None: + state = _state_with_open_entries(actor_ids=frozenset({actor_id}), check_in_at=now) + command = CloseVisitPresence(visit_id=VISIT_ID, actor_id=actor_id) + assert decide(state=state, command=command, now=now) == decide( + state=state, command=command, now=now + ) diff --git a/apps/api/tests/unit/trust/test_visit_handlers.py b/apps/api/tests/unit/trust/test_visit_handlers.py index 31135e1cf35..8984a591b28 100644 --- a/apps/api/tests/unit/trust/test_visit_handlers.py +++ b/apps/api/tests/unit/trust/test_visit_handlers.py @@ -1,9 +1,9 @@ -"""Application-handler unit tests for the 13 Visit slices. +"""Application-handler unit tests for the 14 Visit slices. Consolidated coverage file: covers `register_visit`, `record_visit_arrival`, `start_visit`, `hold_visit`, `resume_visit`, `complete_visit`, `cancel_visit`, `abort_visit`, `void_visit`, `check_in_visit`, -`check_out_visit`, `take_control_of_surface`, +`check_out_visit`, `close_visit_presence`, `take_control_of_surface`, `release_control_of_surface` per the arch-fitness substring-match rule. Pure-decider behavior is exercised in the per-slice files under `tests/unit/trust/visit/`; here we pin the handler-level concerns: @@ -38,6 +38,7 @@ cancel_visit, check_in_visit, check_out_visit, + close_visit_presence, complete_visit, hold_visit, record_visit_arrival, @@ -696,3 +697,103 @@ async def test_release_control_handler_rejects_when_pool_reports_other_holder() ) events, _ = await store.load("Visit", _VISIT_ID) assert not any(e.event_type == "VisitSurfaceControlReleased" for e in events) + + +@pytest.mark.unit +async def test_close_visit_presence_handler_closes_a_third_partys_entry() -> None: + """The one capability check-out cannot supply. + + Seeds a check-in for somebody who is NOT the caller, then closes it as the + caller. Under `check_out_visit` this is impossible by construction, which + is why the slice exists. + """ + store = InMemoryEventStore() + await _seed_to(store, VisitStatus.ARRIVED) + absent_actor = uuid4() + _, current_version = await store.load("Visit", _VISIT_ID) + seed_event = VisitCheckedIn( + visit_id=_VISIT_ID, + actor_id=absent_actor, + mode=PresenceMode.PHYSICAL.value, + occurred_at=_NOW, + ) + await store.append( + stream_type="Visit", + stream_id=_VISIT_ID, + expected_version=current_version, + events=[ + to_new_event( + event_type=event_type_name(seed_event), + payload=to_payload(seed_event), + occurred_at=seed_event.occurred_at, + event_id=uuid4(), + command_name="SeedCheckIn", + correlation_id=_CORRELATION_ID, + causation_id=None, + principal_id=absent_actor, + ) + ], + ) + deps = build_deps(ids=[_TRANSITION_EVENT_ID], now=_NOW, event_store=store) + handler = close_visit_presence.bind(deps) + await handler( + close_visit_presence.CloseVisitPresence(visit_id=_VISIT_ID, actor_id=absent_actor), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + events, _ = await store.load("Visit", _VISIT_ID) + folded = fold([from_stored(s) for s in events]) + assert folded is not None + [entry] = folded.presence_entries + assert entry.actor_id == absent_actor + assert entry.check_out_at is not None + + +@pytest.mark.unit +async def test_close_visit_presence_records_the_caller_on_the_envelope() -> None: + """The stream distinguishes this from a self-checkout twice over. + + By TYPE, because `VisitPresenceClosed` is not `VisitCheckedOut`, which is + what makes "was this person's record closed for them" one predicate. And by + ENVELOPE, which names the caller as principal while the payload names the + actor whose presence ended, so no `closed_by` payload field is needed.""" + store = InMemoryEventStore() + await _seed_to(store, VisitStatus.ARRIVED) + absent_actor = uuid4() + _, current_version = await store.load("Visit", _VISIT_ID) + seed_event = VisitCheckedIn( + visit_id=_VISIT_ID, + actor_id=absent_actor, + mode=PresenceMode.PHYSICAL.value, + occurred_at=_NOW, + ) + await store.append( + stream_type="Visit", + stream_id=_VISIT_ID, + expected_version=current_version, + events=[ + to_new_event( + event_type=event_type_name(seed_event), + payload=to_payload(seed_event), + occurred_at=seed_event.occurred_at, + event_id=uuid4(), + command_name="SeedCheckIn", + correlation_id=_CORRELATION_ID, + causation_id=None, + principal_id=absent_actor, + ) + ], + ) + deps = build_deps(ids=[_TRANSITION_EVENT_ID], now=_NOW, event_store=store) + handler = close_visit_presence.bind(deps) + await handler( + close_visit_presence.CloseVisitPresence(visit_id=_VISIT_ID, actor_id=absent_actor), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + events, _ = await store.load("Visit", _VISIT_ID) + closed = [e for e in events if e.event_type == "VisitPresenceClosed"] + assert len(closed) == 1 + assert closed[0].principal_id == _PRINCIPAL_ID + assert closed[0].metadata == {"command": "CloseVisitPresence"} + assert UUID(closed[0].payload["actor_id"]) == absent_actor diff --git a/apps/api/tests/unit/trust/visit/test_close_visit_presence_decider.py b/apps/api/tests/unit/trust/visit/test_close_visit_presence_decider.py new file mode 100644 index 00000000000..5061e22a181 --- /dev/null +++ b/apps/api/tests/unit/trust/visit/test_close_visit_presence_decider.py @@ -0,0 +1,90 @@ +"""Decider tests for `close_visit_presence` (closing another actor's entry).""" + +from dataclasses import replace +from uuid import uuid4 + +import pytest + +from cora.trust.aggregates.visit import ( + PresenceEntry, + PresenceMode, + VisitActorNotCheckedInError, + VisitNotFoundError, + VisitPresenceClosed, + VisitStatus, +) +from cora.trust.features.close_visit_presence import CloseVisitPresence +from cora.trust.features.close_visit_presence.decider import decide +from tests.unit.trust.visit._fixtures import NOW, VISIT_ID, make_visit + + +def _with_open_entry(actor_id: object) -> object: + base = make_visit(VisitStatus.IN_PROGRESS) + return replace( + base, + presence_entries=frozenset( + { + PresenceEntry( + actor_id=actor_id, # pyright: ignore[reportArgumentType] + mode=PresenceMode.PHYSICAL, + check_in_at=NOW, + check_out_at=None, + ) + } + ), + ) + + +@pytest.mark.unit +def test_close_presence_closes_the_named_actors_entry_not_the_callers() -> None: + """The command names its target; the caller is not the subject. + + This is the axis on which the slice differs from check_out_visit, so it is + the one worth pinning: a caller closing somebody else's entry must produce + an event naming THAT actor. + """ + absent_actor = uuid4() + events = decide( + state=_with_open_entry(absent_actor), # pyright: ignore[reportArgumentType] + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=absent_actor), + now=NOW, + ) + [e] = events + assert isinstance(e, VisitPresenceClosed) + assert e.actor_id == absent_actor + + +@pytest.mark.unit +def test_close_presence_raises_when_named_actor_has_no_open_entry() -> None: + someone_present = uuid4() + someone_else = uuid4() + with pytest.raises(VisitActorNotCheckedInError) as exc: + decide( + state=_with_open_entry(someone_present), # pyright: ignore[reportArgumentType] + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=someone_else), + now=NOW, + ) + assert exc.value.actor_id == someone_else + + +@pytest.mark.unit +def test_close_presence_raises_not_found_on_empty_state() -> None: + with pytest.raises(VisitNotFoundError): + decide( + state=None, + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=uuid4()), + now=NOW, + ) + + +@pytest.mark.unit +def test_close_presence_is_not_blocked_by_a_terminal_visit_status() -> None: + """No status guard: the evolver has already closed entries on a terminal + Visit, so an open entry here means the Visit is still live.""" + actor = uuid4() + events = decide( + state=_with_open_entry(actor), # pyright: ignore[reportArgumentType] + command=CloseVisitPresence(visit_id=VISIT_ID, actor_id=actor), + now=NOW, + ) + assert len(events) == 1 diff --git a/apps/api/tests/unit/trust/visit/test_visit_evolver.py b/apps/api/tests/unit/trust/visit/test_visit_evolver.py index fb949ec3026..302a416a406 100644 --- a/apps/api/tests/unit/trust/visit/test_visit_evolver.py +++ b/apps/api/tests/unit/trust/visit/test_visit_evolver.py @@ -1,7 +1,7 @@ """Evolver / fold tests: replay determinism + last_status_reason preservation.""" from datetime import UTC, datetime, timedelta -from uuid import UUID +from uuid import UUID, uuid4 import pytest @@ -10,6 +10,8 @@ VisitAborted, VisitArrived, VisitCancelled, + VisitCheckedIn, + VisitCheckedOut, VisitCompleted, VisitEvent, VisitHeld, @@ -162,3 +164,93 @@ def test_evolve_step_by_step_matches_fold() -> None: incremental = evolve(incremental, e) assert incremental == fold_state + + +_TERMINAL_AT = _NOW + timedelta(hours=3) + + +def _arrived_history() -> list[VisitEvent]: + return [_registered(), VisitArrived(visit_id=_VID, occurred_at=_NOW)] + + +# --- terminal transitions close open presence ------------------------------ +# +# A Visit that has completed, been cancelled, aborted or voided has nobody at +# the beamline by implication. The evolver derives that rather than requiring +# per-actor close events, so the fold and `proj_trust_visit_presence` agree +# without the terminal deciders knowing anything about presence. + + +def _visit_with_two_open_entries(status_event: VisitEvent) -> Visit: + """Arrived Visit, two actors checked in, then `status_event` applied.""" + a, b = uuid4(), uuid4() + history: list[VisitEvent] = [ + *_arrived_history(), + VisitCheckedIn(visit_id=_VID, actor_id=a, mode="physical", occurred_at=_NOW), + VisitCheckedIn(visit_id=_VID, actor_id=b, mode="remote", occurred_at=_NOW), + status_event, + ] + folded = fold(history) + assert folded is not None + return folded + + +@pytest.mark.parametrize( + ("status_event", "expected_status"), + [ + ( + VisitCompleted(visit_id=_VID, occurred_at=_TERMINAL_AT), + VisitStatus.COMPLETED, + ), + ( + VisitCancelled(visit_id=_VID, reason="r", occurred_at=_TERMINAL_AT), + VisitStatus.CANCELLED, + ), + ( + VisitAborted(visit_id=_VID, reason="r", occurred_at=_TERMINAL_AT), + VisitStatus.ABORTED, + ), + ( + VisitVoided(visit_id=_VID, reason="r", occurred_at=_TERMINAL_AT), + VisitStatus.VOIDED, + ), + ], +) +@pytest.mark.unit +def test_terminal_transition_closes_every_open_presence_entry( + status_event: VisitEvent, expected_status: VisitStatus +) -> None: + state = _visit_with_two_open_entries(status_event) + assert state.status is expected_status + assert len(state.presence_entries) == 2 + assert all(e.check_out_at == _TERMINAL_AT for e in state.presence_entries) + + +@pytest.mark.unit +def test_terminal_transition_leaves_an_already_closed_entry_at_its_own_time() -> None: + """A prior check-out keeps its own timestamp; the terminal one does not overwrite it.""" + actor = uuid4() + earlier = _NOW + timedelta(hours=1) + history: list[VisitEvent] = [ + *_arrived_history(), + VisitCheckedIn(visit_id=_VID, actor_id=actor, mode="physical", occurred_at=_NOW), + VisitCheckedOut(visit_id=_VID, actor_id=actor, occurred_at=earlier), + VisitCompleted(visit_id=_VID, occurred_at=_TERMINAL_AT), + ] + folded = fold(history) + assert folded is not None + [entry] = folded.presence_entries + assert entry.check_out_at == earlier + + +@pytest.mark.unit +def test_terminal_transition_with_no_presence_changes_nothing() -> None: + """The no-open-entries path returns the same frozenset, not a rebuilt one.""" + history: list[VisitEvent] = [ + *_arrived_history(), + VisitCompleted(visit_id=_VID, occurred_at=_TERMINAL_AT), + ] + folded = fold(history) + assert folded is not None + assert folded.presence_entries == frozenset() + assert folded.status is VisitStatus.COMPLETED diff --git a/apps/api/tests/unit/trust/visit/test_visit_presence_projection.py b/apps/api/tests/unit/trust/visit/test_visit_presence_projection.py index f811862fca0..f777f2d76f3 100644 --- a/apps/api/tests/unit/trust/visit/test_visit_presence_projection.py +++ b/apps/api/tests/unit/trust/visit/test_visit_presence_projection.py @@ -40,7 +40,17 @@ def _stored(event_type: str, payload: dict[str, Any]) -> StoredEvent: def test_projection_metadata() -> None: proj = VisitPresenceProjection() assert proj.name == "proj_trust_visit_presence" - assert proj.subscribed_event_types == frozenset({"VisitCheckedIn", "VisitCheckedOut"}) + assert proj.subscribed_event_types == frozenset( + { + "VisitCheckedIn", + "VisitCheckedOut", + "VisitPresenceClosed", + "VisitCompleted", + "VisitCancelled", + "VisitAborted", + "VisitVoided", + } + ) @pytest.mark.unit @@ -118,3 +128,68 @@ async def test_visit_checked_out_updates_open_entry_only() -> None: assert args.args[1] == _VID assert args.args[2] == _AID assert args.args[3] == _NOW + + +@pytest.mark.parametrize( + "terminal_type", + ["VisitCompleted", "VisitCancelled", "VisitAborted", "VisitVoided"], +) +@pytest.mark.unit +async def test_terminal_event_closes_every_open_row_for_the_visit( + terminal_type: str, +) -> None: + """The read model must agree with the aggregate on terminal closure. + + `_closed_at` in the evolver closes every open entry when a Visit reaches a + terminal state. If this projection did not do the same, a Visit would fold + to "nobody present" while the table still showed open rows, and presence + would be unusable as evidence for exactly the queries it exists to answer. + """ + proj = VisitPresenceProjection() + conn = AsyncMock() + event = _stored(terminal_type, {"visit_id": str(_VID), "occurred_at": _NOW.isoformat()}) + await proj.apply(event, conn) + conn.execute.assert_awaited_once() + sql, *args = conn.execute.await_args.args + assert "check_out_at IS NULL" in sql + assert args == [_VID, _NOW] + + +@pytest.mark.unit +async def test_terminal_event_needs_no_actor_id_in_its_payload() -> None: + """Terminal payloads carry no actor, so the handler must not read one.""" + proj = VisitPresenceProjection() + conn = AsyncMock() + event = _stored("VisitCompleted", {"visit_id": str(_VID), "occurred_at": _NOW.isoformat()}) + await proj.apply(event, conn) + conn.execute.assert_awaited_once() + + +@pytest.mark.unit +async def test_presence_closed_updates_the_same_row_as_a_check_out() -> None: + """The two closing events differ in cause, not in effect on the row. + + They are separate event types so the stream can be queried for one or the + other, but the read model must not diverge: both close the named actor's + open entry with the same UPDATE. + """ + proj = VisitPresenceProjection() + conn = AsyncMock() + event = _stored( + "VisitPresenceClosed", + { + "visit_id": str(_VID), + "actor_id": str(_AID), + "occurred_at": _NOW.isoformat(), + }, + ) + await proj.apply(event, conn) + conn.execute.assert_awaited_once() + args = conn.execute.await_args + assert args is not None + sql: str = args.args[0] + assert "UPDATE proj_trust_visit_presence" in sql + assert "check_out_at IS NULL" in sql + assert args.args[1] == _VID + assert args.args[2] == _AID + assert args.args[3] == _NOW