Repository navigation
Conversation
calebyhan
requested changes
Nov 4, 2026
calebyhan
left a comment
Collaborator
There was a problem hiding this comment.
Code looks good and covers everything in #93, but this needs a rebase before it can merge:
- Migration conflict. #110 added
4bfad84b2ce1_add_texter_token_revocation, which also revises38f046bc3fc3, so merging this as is leaves two Alembic heads and breaksalembic upgrade head/ CI. Per the issue: delete6d6bdb1ebc89_add_escalations.py, pullmain, and runmake migration m="add escalations"again (re-add the check constraint and theescalation_statusenum drop indowngrade()if autogenerate doesn't pick them up). - Conflict in
backend/app/main.py.mainnow registersadmin_resourcesandportal_ws. Keep all fourinclude_routerlines and all the imports.
Happy to approve once that's done.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some conversations will need to be escalated, following a protocol Peer2Peer is still working out with CAPS (TDD 4.3.2, open question 3). We can't build the final behavior yet, but we can build the frame around it: an escalation record, a separate encryption key for anything sensitive it holds, a responder action that creates one, and a coordinator view where every read is logged. Once the protocol is final, filling in the details should be a small change.
Changes:
Escalationmodel (app/models/escalation.py) per TDD 4.5.2, with anEscalationStatusenum (open,handed_off,closed), a nullableprotected_payload(BYTEA), andkey_versionnext to it, plus its migration. I added a check constraint so the payload and key version are always set together (or both null), and an index onconversation_id, since a conversation can have several escalations. - Addedapp/services/escalations.py, the only place escalation payloads are encrypdecrypted, the same wayservices/messages.pyhandles message bodies. Payloads useKeyPurpose.ESCALATION, never the message key. For now the payload is the{"source"portal"}placeholder. - AddedPOST /api/portal/conversations/{id}/escalate. It usesrequire_on_shift(merged) plus a check that the caller is the assigned responder. Anyone else gets 404, not 403, so responders can't learn which conversations exist. It creates anopenescalauditsescalation.create, and returns 201 with the escalation id. - AddedGET /api/admin/escalationsandGET /api/admin/escalations/{id}(require_coordinator). Both write anescalation.viewaudit entry on every read; the detail route includes the escalation id. The list is newest first. Only the detail route decrypts the payload, and it sendsCache-Control: no-store(the same as account creation) so the decrypted payload isn't cached.EscalationDTO(id,conversation_label,status,created_by_display_name,created_at) andEscalationDetailDTO, which addspayload, so the list response leavespayloadout entirely. I also addedCreatedEscalationDTOfor the create response.conversation_labelis "Texter N", or "Texter" if the conversation has no label number.tests/test_escalations.py: the assigned responder can escalate and anyone else gets 404, the stored payload isn't plaintext and can't be decrypted with the message key, coordinators can list and read escalations, every read adds an audit row, and responders get 403 on the admin routes. Extra cases: signed out (401), off shift (403not_on_shift), unknown ids (404), no audit row when access is refused, and no payload in the list.TODO(escalation-protocol)comments where the open decisions plug in: what the payload holds, who gets contacted, and how status changes. None of that is invented here.Notes for reviewers:
cd backend && uv run pytest tests/test_escalations.pyCloses #93