Skip to content

Close presence when a Visit ends, and let an operator close a stray entry - #609

Merged
xmap merged 1 commit into
mainfrom
feat/close-visit-presence
Aug 4, 2026
Merged

Close presence when a Visit ends, and let an operator close a stray entry#609
xmap merged 1 commit into
mainfrom
feat/close-visit-presence

Conversation

@xmap

@xmap xmap commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Stacked on #605. Base is fix/presence-names-the-caller, because this fills the gap that PR deliberately opened. Sibling of #608, not dependent on it.

Why

Dropping the caller-asserted actor from check-in and check-out closed a real hole and left a real gap: somebody who went home without checking out had an entry nobody could close. That capability existed only because of the hole. This restores it in two forms, neither of which reopens it.

Auto-close on terminal transition

A Visit reaching Completed / Cancelled / Aborted / Voided now closes every open presence entry at the transition's own timestamp.

No new event. "The Visit ended at T" already implies "nobody is present after T", so the evolver derives it and the terminal deciders stay ignorant of presence.

The projection subscribes to the same four events, and that is the part worth reviewing. The fold and the read model must agree: a Visit that folded to nobody-present while proj_trust_visit_presence still showed open rows would make presence useless as evidence for the one query it exists to answer. There is a test asserting they agree, not just that each works.

The explicit command

close_visit_presence covers the mid-Visit case, where somebody left but the beamtime is still running. Separate command rather than a nullable actor on check-out, so closing another person's record needs its own Policy grant.

Note the deliberate asymmetry at the wire, which looks inconsistent until you see why:

actor_id in body
check_in_visit forbidden (422)
check_out_visit forbidden (422)
close_visit_presence required

Naming the target is the entire intent of the third command.

An architecture test overturned my first design

It originally emitted VisitCheckedOut, reasoning that the envelope's principal_id and command_name already disambiguate. test_command_name_derives_event_name rejected that, and it was right.

Reading presence as evidence means "did this person leave, or did somebody close their record for them" should be one predicate over the event stream, not a join against envelope metadata. And CORA already models cause-distinguished events for a single structural transition: VisitCancelled / VisitAborted / VisitVoided.

So it emits VisitPresenceClosed. The feared duplication came to ~15 lines: the evolver takes both in one match arm and the projection routes both to the same UPDATE, because they differ in cause rather than effect. Who did the closing stays on the envelope; a closed_by payload field would duplicate principal_id and, being a *_by attribution field, would then demand a paired timestamp it does not need.

Tests

The slice-coverage fitness tests drove the surface: decider, handler, endpoint contract, MCP tool contract, and a paired PBT.

Two worth pointing at:

  • Bystander isolation (PBT). With several actors open at once, closing one never emits an event naming another. This catches a decider that closes the first open entry rather than the named one, which is the plausible way this slice would go wrong.
  • Prior check-out keeps its own timestamp. A terminal transition must not overwrite an already-closed entry.

The lifecycle-independence property is scoped to non-terminal statuses deliberately: a terminal Visit has already had its entries closed, so "terminal Visit with an open entry" is unreachable rather than untested. That reasoning is in the test, because the next reader will be tempted to widen it back.

Verification

ruff + pyright     clean
unit + contract    16,146 passed, 1 skipped
architecture       30,186 passed, 614 skipped
openapi snapshot   regenerated
pre-commit hooks   all passed

🤖 Generated with Claude Code

@xmap
xmap force-pushed the feat/close-visit-presence branch from 9fd4057 to bb0ec4a Compare August 3, 2026 22:45
xmap added a commit that referenced this pull request Aug 3, 2026
`CheckInVisit` and `CheckOutVisit` each carried an `actor_id` command field,
so any principal holding the command grant could record another actor as
present, or close another actor's open entry. Presence was something a caller
asserted rather than something the facility knew, which left
`proj_trust_visit_presence` unusable as evidence of who was at a beamline.

Both commands now take the actor from the authenticated principal, injected
into the decider through the `actor_kwarg` knob that Federation, Agent,
Subject and Budget already use. Request bodies set `extra="forbid"`, so a
client still sending `actor_id` gets a 422 rather than a 204 that silently
recorded the caller instead of the actor it named.

Both halves ship together: fixing only check-in would leave presence forgeable
in the other direction, since an entry could no longer be faked but any
principal could still erase one.

Gate-reviewed across four lenses, all LOCK_WITH_FIXES, all findings addressed.
Mutation testing showed the rewritten duplicate-guard line had no coverage at
all, and the diff-coverage gate caught the MCP success path reporting the
recorded actor being unexercised. Both closed.

Known regression, deliberate: nobody can close another actor's forgotten entry.
That capability existed only because of the hole. #609 restores it properly,
with auto-close on terminal Visit transitions plus an explicit command carrying
its own grant.
Base automatically changed from fix/presence-names-the-caller to main August 3, 2026 22:51
…ntry

Dropping the caller-asserted actor from check-in and check-out left a real
gap: somebody who went home without checking out had an entry nobody could
close. This fills it from both ends.

A Visit reaching a terminal state now closes every open presence entry, at
the transition's own timestamp. No new event: "the Visit ended at T"
already implies "nobody is present after T", so the evolver derives it and
the terminal deciders stay ignorant of presence. The projection subscribes
to the same four events, because the fold and the read model have to agree.
A Visit that folded to nobody-present while the table still showed open
rows would make presence useless as evidence for the one query it exists
to answer.

`close_visit_presence` covers the mid-Visit case, where somebody left but
the beamtime is still running. It is a separate command rather than a
nullable actor on check-out so that closing another person's record needs
its own Policy grant. Note the deliberate asymmetry at the wire: check-in
and check-out FORBID an actor_id, this one REQUIRES it, because naming the
target is the whole intent.

It emits `VisitPresenceClosed`, not the `VisitCheckedOut` it started out
emitting. `test_command_name_derives_event_name` rejected the reuse and was
right to. Reading presence as evidence means "did this person leave, or did
somebody close their record for them" should be one predicate over the
stream rather than a join against envelope metadata, and CORA already
models cause-distinguished events for one structural transition:
VisitCancelled / VisitAborted / VisitVoided. The state change is identical,
so the evolver takes both in one match arm and the projection routes both
to the same UPDATE. Who did the closing stays on the envelope; a closed_by
payload field would duplicate principal_id and, being a *_by attribution
field, would then demand a paired timestamp it does not need.

The slice-coverage fitness tests drove the test surface: decider, handler,
endpoint contract, MCP tool contract, and a paired PBT. The property worth
keeping is bystander isolation, which catches a decider closing the FIRST
open entry rather than the NAMED one. The lifecycle-independence property
is scoped to non-terminal statuses because a terminal Visit has already had
its entries closed, so "terminal Visit with an open entry" is unreachable
rather than untested.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@xmap
xmap force-pushed the feat/close-visit-presence branch from bb0ec4a to 9688a96 Compare August 3, 2026 23:15
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  apps/api/src/cora/trust
  routes.py
  tools.py
  apps/api/src/cora/trust/aggregates/visit
  events.py
  evolver.py
  apps/api/src/cora/trust/features/close_visit_presence
  __init__.py
  command.py
  decider.py
  handler.py
  route.py
  tool.py 49
  apps/api/src/cora/trust/projections
  visit_presence.py
Project Total  

This report was generated by python-coverage-comment-action

@xmap
xmap merged commit ba94a0a into main Aug 4, 2026
16 checks passed
@xmap
xmap deleted the feat/close-visit-presence branch August 4, 2026 02:18
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.

1 participant