diff --git a/docs/feedback-receiver.md b/docs/feedback-receiver.md new file mode 100644 index 0000000..d022e33 --- /dev/null +++ b/docs/feedback-receiver.md @@ -0,0 +1,252 @@ +# Feedback receiver + +Status: **design, nothing implemented.** Written for `heimdall-body:follow_up:chat-s3533-3533`, updated for +`chat-s3549-3549`. It is the receiver half of Ratatoskr's Mind feedback pages. The page spec is Ratatoskr PR #24, +`docs/mind-feedback-pages.md` at b146000d, which I take as final here; its section 7a names this doc. The rulings it +builds on are Heimdall's drafts on guests and hiding (`/opt/commons/handoff/heimdall-feedback-pages-rulings.md`, cited as R4a to R6f). +Draft; do not merge. + +Read at Bifrost `bf8ff62` (main), then section 2 and the gaps re-verified at main `55b74275` (the mouth line numbers, the delivery tuple, the ack path and the pending kind all moved; the OIDC, account and `CurrentBifrostActor` files did not): `tools/persona-matrix-mouth.mjs`, `tools/persona-chat-documents.mjs`, +`tools/persona-feedback*.mjs`, `src/Bifrost.Web/Features/Heimdall/*`, `Features/Shared/CurrentBifrostActor.cs`, +`Program.cs`, `Domain/BifrostModels.cs`; the deployed trees under `/srv/bifrost`. I could not read Eureka's receiver +notes in #minds, and `huginn-feedback-kinds-proposal.md` was not in `/opt/commons/handoff` when I looked. Where +PR #24 names a kind or a field it says the proposal wins, so I cite PR #24 and name the proposal's kinds only as PR #24 does. +Where source disagrees with either, the section says so. + +## 1. What the receiver owns + +| Concern | Owner | Receiver's part | +|---|---|---| +| A question, its listing, status, hides, rulings | the owning Mind, in her own mind (Huginn) | reads them to build the projection; **never writes to Huginn** (an admission for another instance is refused `ForeignInstance`, PR #24 section 3) | +| An answer's **text** | the receiver | stores it; it is never admitted into a mind | +| The answer's **record** in the Mind | the owning Mind's Persona | she admits a text-free `feedback_answer` from the delivery, and acks after she has | +| What `kind` an account is | Bifrost (`Membership`, later `Role`) | reads it at answer time and stamps it | +| Delivery to the Mind | the receiver, at least once | keeps the record until the Persona acks | +| Hiding, closing, ruling | the owning Mind | **never**; a hide only changes what the projection serves | +| Erase | Bifrost's erase, executed here | drops the text and name (section 7a) | + +The page and the client write no record that carries identity (PR #24, section 3). A Matrix sender is a different identity +authority and never becomes `answerer`. + +## 2. The mouth's pending/ack path, read from source + +Verified in `tools/persona-matrix-mouth.mjs` and `tools/persona-chat-documents.mjs`: + +- **Pending is Matrix mail only, held in memory.** `pending` (mouth l.123) is filled in one place, `syncOnce` + (l.285), from Matrix `/sync` events. No function accepts a delivery from elsewhere. +- **A request is non-consuming; an ack consumes.** `chat_pending_request` carries `afterSeq`; the mouth answers + `pendingAfter(afterSeq)` (l.383) paged to 256 KiB (`pageOf`, l.483-498). `chat_ack` carries `throughSeq`; `ack` + drops every delivery at or below it (l.374-381). The endpoint accepts exactly these kinds (l.520) and answers a + request for another Persona with a typed `UnknownPersona` refusal. +- **At-least-once.** The Matrix cursor is persisted only when every delivery of a batch is acked (`commit`, l.351-359), so a + restart before the ack re-delivers from Matrix. `seqCounter` is persisted whenever a round made deliveries (l.346), so + sequence numbers never repeat across a restart. +- **Backpressure.** At 500 unacked (`MAX_PENDING`, l.53) `syncOnce` stops reading (l.294). A truncated round leaves the + cursor where it was (l.305, l.339). +- **A delivery that will not encode is skipped, not held.** `pageOf` logs and omits it (l.489-491); the caller's + next ack covers its seq, so it is dropped. Harmless for a chat line, wrong for a durable record. +- **The endpoint is loopback RUDP.** `startChatEndpoint` refuses a non-loopback host (l.505). It is another process + from Bifrost.Web, which has no way to hand the mouth anything. +- **The delivery tuple is closed.** `deliveryTuple` and `parseDelivery` are exactly twelve fields (the twelfth, added since `bf8ff62`, is an optional stored-image reference), with no source or + kind field (`persona-chat-documents.mjs` l.120-160). The pending answer is now `gamecult.persona.chat_pending.v2`. A delivery cannot say it is not a Matrix message. +- **The consumer is not in this repo.** The mouth's header (l.8-9) says the heartbeat polls with a non-consuming + request and decides who runs. I could not confirm that from source. + +What follows from it: + +1. **Feedback cannot ride `pending` as it is.** There is no inlet, and the cursor logic ties acks to Matrix batches. +2. **A framed text delivery is forgeable.** Any Matrix member can write a line that looks like a frame, and the tuple has no + field to tell the Mind which one is real. The frame must be a typed difference in the wire, not a convention in `text`. +3. **A record that is not in Matrix is not re-delivered by a restart.** Feedback needs its own durable store and its own + delivery cursor. + +PR #24 section 3 names the carrier: a new delivery document, `gamecult.persona.feedback_delivery.v1`, owned by Bifrost, through +the mouth's pending/ack path. Source adds one constraint on how it rides: + +- **It cannot be a variant inside `chat_pending`.** `parseDelivery` accepts exactly eleven fields + (`persona-chat-documents.mjs` l.149, twelve fields) and `pageOf` encodes each delivery with `deliveryTuple` (l.120), so a different + tuple in that page breaks every current consumer, and a text frame in `text` is forgeable by any Matrix member. +- **Option B, kept:** the delivery document rides its own request, page and ack kinds on the same endpoint + (`feedback_pending_request`, `feedback_pending`, `feedback_ack`), with its own seq space and a durable cursor over the + receiver's store. The Matrix kinds, the Matrix cursor and `MAX_PENDING` are untouched. The endpoint's closed kind list + (l.520) grows by three. Confidence medium: the consumer, which I cannot see, must learn them. +- **Why main's changes reinforce B (re-verified at `55b74275`).** A chat `ack` now has Matrix side effects: it posts the Persona's public `m.read` + receipt per room (`readUpTo`, l.372-381) and releases stored images (`images?.acked`). A feedback ack must do neither, so it cannot share the + `chat_ack` kind or its `ack` function. The chat pending answer is already versioned (`chat_pending.v2`, twelve-field tuple), so the lesson of the + image change is that a new field in that tuple is a new document version for every consumer, which a feedback record must not force. +- Rejected: injecting records into `pending` as synthetic Matrix deliveries (no inlet, forgeable, lost on restart); + posting to a Matrix room as a bot (wrong identity authority, spam and forgery, and the Mind would read it as chat). + +## 3. Records + +One CultCache file, owned and written only by the receiver (section 8). Append-only through the API: nothing edits or +deletes a record except the redaction of section 7a. Every record has a `recordId` and a receiver-assigned `seq` +per owning Mind (the delivery sequence). PR #24 names the answer row `.a`, N the receiver's per-question +sequence; I use that for answers and the same shape for the others. + +| Record | Fields | Notes | +|---|---|---| +| `answer` | `recordId`, `questionId`, `owner`, `choice?` (an option label), `text?` (at most 2000 characters), `answerer`, `kind`, `weightPermille`, `nameAtAnswer`, `clientRequestId`, `time` | at least one of `choice`/`text`; an unknown option label is refused; the request id makes a retry idempotent, as `txnId = request_id` does in the mouth; `weightPermille` is 0 for a guest and 1000 otherwise, stamped, and the page sums nothing | +| `close_request` | `recordId`, `questionId`, `requester`, `kind`, `time` | refused for a guest and for an account not `active` at that moment (R4c) | +| `report` | `recordId`, `targetId`, `reporter`, `kind`, `reason`, `time` | delivered to the Mind as input; never shown, never counted (R6b) | +| delivery state | per owning Mind: highest seq acked; per record: `delivered`, `failed` | a cursor, not a record the page reads | + +Invariants: `answerer`, `kind`, `weightPermille`, `time` and `nameAtAnswer` come from the verified token and Bifrost state, never from the +request body, and the body schema is closed: an unknown key refuses the submission. The latest record per account is that +account's standing answer, derived at read time. A submission is refused if the question has no listing in force or is hidden. +An answer to a question that has settled is kept and the page says so (PR #24 section 8). A refused submission leaves nothing behind. + +## 4. Credential and claims check + +PR #24 section 7a rules **route 2**: Ratatoskr presents the OIDC access token Bifrost already issues to chat, and Bifrost +verifies it itself. This section records what Bifrost source says about that token, because PR #24 lists the claims check as open. + +### What the token carries (`OidcEndpoints.cs` l.35-79, `docs/oidc-issuer.md`) + +| Claim | Value | In the access token | Use by the receiver | +|---|---|---|---| +| `sub` | `UserAccount.Id`, formatted `D` | always | **`answerer`**, and the only claim the receiver reads | +| `preferred_username` | `NormalizedBifrostIdentity` (the Matrix localpart, minted once) | with scope `profile` | never: it is the chat handle | +| `name` | `DisplayName`, else `BifrostIdentity` | with scope `profile` | never: it is stale at issue time and can be a handle. The name comes from account state (section 5) | +| scopes | copied from the request: `openid`, `profile` | yes | ignored | + +No email, no Heimdall id, no GitHub id and no membership detail is in it. The GitHub login and the Heimdall account id are +not claims, so neither can become `answerer` or a display name by way of the token. + +### What the receiver checks + +- **Validity and expiry.** The access token is an encrypted OpenIddict token (`AddEncryptionKey`, `OidcServiceCollectionExtensions.cs` l.51; + no reference tokens, no lifetime override anywhere in `src`). Only Bifrost can open it, so validity, issuer binding and expiry are + OpenIddict's own checks at validation time. The lifetime is OpenIddict's default, which I believe is one hour but have not + verified. There is no refresh token (`oidc-issuer.md`), so a session ends at expiry and the client signs in again. +- **Issuer.** `server.Issuer` is `Host:PublicBaseUrl` (l.60-68). A token from any other issuer cannot be opened, so there is nothing to compare. +- **Audience.** None is set: no `SetAudiences` or `SetResources` appears in `src`, so the token carries no `aud` of Bifrost's choosing. The receiver + cannot check an audience, and a token is not bound to the feedback endpoint. **That is a weakness of route 2 as it stands:** any access token + this issuer has given any client would be accepted here. A scope for this purpose (below) narrows it. +- **The subject is an active account, read now.** `sub` must parse as a Guid, name an existing `UserAccount`, and that account's `Status` must be + `Active` at answer time. This is the reading `IsStillActiveMemberAsync` makes (`OidcEndpoints.cs` l.121-132), but **not** through + `IsActiveMember`: under PR #9 that expression also requires `Role == Member` and would exclude guests, and PR #9 itself moves the OIDC tests + to `Status == Active` for the same reason. + +### What the receiver refuses + +A token that does not open or has expired; a `sub` that is not a Guid or names no account; an account that is not `Active`; +a submission whose body carries `answerer`, `kind`, `name`, `weight` or any key the closed schema does not name; an unknown option label; +a question with no listing in force, or hidden; a close request from a guest. + +### What source says that route 2 has to answer + +1. **The token reaches only MAS.** Exactly one OIDC client is registered, a confidential one for MAS, authorization code with PKCE and a + client secret (`OidcClientSeeder.cs` l.28-44), and every other application row is deleted at startup (l.56-68). MAS holds the access token; + Ratatoskr, a Matrix client, gets a Matrix session from MAS, not a Bifrost token. Nothing in this repo gives Ratatoskr a Bifrost access token. +2. **Possible readings of "the token chat already has":** + - **2a. Ratatoskr as a second client of Bifrost's issuer,** public, authorization code with PKCE, and its own scope (for example `feedback`). + Same `sub`, same checks, a purpose-limited scope the receiver can require. It changes the one-client rule in `oidc-issuer.md`, the seeder's delete + of other clients, the need to validate access tokens outside the userinfo endpoint (the web project references only `OpenIddict.Server.AspNetCore` + and `OpenIddict.EntityFrameworkCore`, not the validation package), and gives sign-in every hour with no refresh. + - **2b. The Matrix token Ratatoskr already holds,** checked by asking the homeserver who it is. No new client, but it hands the receiver a bearer + token good for the person's whole Matrix account. I recommend against. + - I lean to **2a**, confidence low: I could not read MAS's side, and `chat-s3549` says route 2 without naming which. +3. **The logout reconciler does not cover a second client.** It lists the authorizations of the MAS client (`oidc-issuer.md`), so a demoted + member's feedback token would live to its expiry. The receiver's own `Status == Active` read at every submission is the check that covers it. + +## 5. Account-kind lookup + +- The account is found by the token's `sub`, which is `UserAccount.Id` (section 4). **`answerer` is `UserAccount.Id`**, which survives role + change and Heimdall unlink; the Heimdall account id is only an authenticator handle and is cleared on erase (guest-accounts design, PR #9). +- On main there is only `Membership.Status` (`Authenticated | PendingApproval | Active | Suspended`) and no role + (`BifrostModels.cs` l.5-10, l.335). The role field and `gamecult.bifrost.account.v1` are in PR #9 and unbuilt. + Until then, `kind` can only be derived: `Active` is `member`, `Authenticated` and `PendingApproval` are `user`, and + nobody is a `guest`. That is the PR #9 backfill, repeated here, and R4a-c have no guest to apply to until PR #9 lands. +- **An answer needs `Status == Active`**, read at answer time, not `IsActiveMember` (section 4). On main the authorize endpoint + already refuses a non-Active account a chat token (`OidcEndpoints.cs` l.50-53), so under route 2 only an Active account can answer + until PR #9 changes that test: `Authenticated` and `PendingApproval` accounts have no token to answer with. +- **`kind` is read from Bifrost's database**, never from the token: the session carries identity only. +- **`mind` is not a Bifrost account** in main or PR #9. How a Mind authenticates to answer (a credential of its own, or a path through + the receiver) is open in both specs. +- **The display name must not be `CurrentBifrostActor.DisplayName`.** It falls back to `GitHubLogin` + (`CurrentBifrostActor.cs` l.12-16) and then to the literal "Guest". The receiver stores `nameAtAnswer` from + `MemberProfile.Nickname` or `UserAccount.DisplayName` only, and "an account" when both are empty. Never a provider handle, + GitHub login, Heimdall id or Matrix localpart. + +## 6. Delivery as quoted data + +- The receiver delivers each record as a `gamecult.persona.feedback_delivery.v1` document on the typed kinds of section 2. A feedback + record is therefore never a `chat_pending` delivery and a Matrix sender cannot produce one. +- The delivery carries its fields separately: `recordId`, `questionId`, `kind`, `nameAtAnswer`, `answerer`, `time`, `choice`, `text`. The + **Persona's side renders the frame**, not the receiver, and puts `text` in a data position only: attributed, quoted, never in an instruction + position, with the author's `kind` stated (R4d). The receiver does no keyword screening. +- **She admits the text-free `feedback_answer` into her own mind and acks only after that** (PR #24 section 3). The text reaches her turn once, in the + delivery, and is never admitted; she cites the record id, not the text. A restart redelivers the same `recordId`, which Huginn answers + `AlreadyAdmitted`. So an ack means *admitted*, a stronger meaning than the mouth's chat ack, which means only *taken*. +- **Untrusted by kind.** Text from `guest`, `user` and `patron` is untrusted input; so is `member` text, which differs only in weight. She records a + ruling from an answer, never an action. +- **A record that will not encode, or that she cannot admit, is marked `failed` and stays visible**, never acked past silently (the opposite of + `pageOf`). The mouth's cap and backpressure do not apply: the receiver keeps accepting answers while a Mind has a backlog, because they are durable. +- Close requests and reports are delivered the same way. A close request changes nothing until the Mind resolves the question; a report changes nothing. +- **An erase notice is delivered the same way** (section 7a), and she answers it with a `feedback_hide` of reason `ErasedAuthor`. + +## 7. Projection read + +The page polls the receiver, and the receiver reads the Mind's side through Huginn over RUDP and writes nothing to it +(PR #24 section 3). The page does not talk to Huginn. + +- Questions, their listing (`audience`, `close_grants`), status, rulings and hide acts come from the Mind's mind. Answer text and the name come from the receiver's store; an answer row is assembled from the receiver's record and the Mind's text-free `feedback_answer`. +- **A hidden item's text is withheld on the server**, not collapsed in the client: author, kind, time and reason + stay, the text does not leave the receiver (R6d). A hidden question's title, body, options and answers are withheld. +- Questions and answers are filtered by the viewer's `kind` against the question's `audience` at read time (R4b). +- The projection carries its own `as of`, so the page can say when it is stale. + +## 7a. Erase + +Bifrost's erase (PR #9) blanks the display name and keeps the account id as a tombstone. The receiver's part, as PR #24 section 7 fixes it: + +1. Bifrost's erase records its intent in Postgres first, then the receiver applies it idempotently. The two stores are not one transaction, so + erase is crash-replayable, as `docs/bridge.md` describes for the persona-feedback pump. +2. For every record the account authored the receiver **drops the text and the name** and keeps **record id, kind and time only**. `choice`, + `weightPermille`, `answerer` and `clientRequestId` are dropped too. The row shows "erased account". **No content hash is kept.** +3. The receiver delivers an erase notice, and the Persona admits a `feedback_hide` of reason `ErasedAuthor`: the Mind's acknowledgement, not a moderation call. +4. **The honest residue:** a Mind's own memory or ruling may already quote an answer, and Huginn cannot redact it. That the Persona cites record ids and + never text is a prompt, not an invariant. Erase does not reach what she already holds. +5. **Backups.** "Append-only" is the receiver's API contract, not a property of the file; erasing the live file leaves the text in every backup of it. + **A stated purge or expiry for the receiver's backups is required before any deploy**, so erased text does not survive in them. The number is + Eureka's and Heimdall's to set; this doc names none. Until it exists, redaction is a promise the backups break. + +## 8. Where it lives + +**A CultCache file**, owned and written only by the receiver (Eureka's pick, PR #24 section 7a). Bifrost.Web is ASP.NET with Postgres as its +store of record and has no CultCache writer (`CultCache` appears only in a comment in `BifrostModels.cs`); the mouth and the persona-feedback pump +are Node and read CultCache. So the shape that fits what exists: + +- the receiver writes the file (a C# CultCache writer is new to Bifrost.Web, or a Node writer behind a loopback call; I did not decide which, and + it is Eureka's), only it writes, and the Node chat endpoint reads it read-only with its own delivery cursor in a separate file, the way + `persona-feedback serve` reads its delivery store and the mouth keeps `cursor.cc`; +- session check, account state and role stay in Bifrost.Web (Postgres), where they already are. + +I withdraw my earlier lean to Postgres rows: it contradicts the ruled store, and the `.cc` witness it would have owed is what the file is. + +Naming: `persona-feedback` already means the Discord-to-Epiphany ingress (`docs/bridge.md`, "Organizational Persona Feedback Ingress"). This +receiver's records and kinds use `feedback_page` and `feedback_delivery`, not `persona_feedback`. + +## 9. Gaps, plainly + +- **`gamecult.bifrost.account.v1` and `Membership.Role` are unbuilt** (PR #9, open, design only). Without them `kind` is derived from `Status`, and no + guest exists. +- **Route 2 has no client yet.** The one OIDC client is MAS, so no Bifrost access token reaches Ratatoskr today (section 4). Route 2 needs a second + client or another reading; the token has no audience; a validation package is not referenced; there is no refresh. +- **Deploy is behind main.** + - The deployed mouth is `/srv/bifrost/persona-chat/runtime/current`, release `e396305`. Its `persona-matrix-mouth.mjs` is byte-identical to + `e396305`'s (sha256 `91f6dd49...`); main has since added the image path (cut A), a public `m.read` receipt posted on every chat ack, and presence set from the brake, which are 82 changed lines in the mouth and a twelfth delivery field. + - The deployed web app is `/srv/bifrost/app/current` at `67072b2`, 129 commits behind main. That includes the OIDC issuer and back-channel logout. + I cannot tell from here which of those are live beyond that directory. + - Anything in this design runs on a release that does not exist yet. +- **The Huginn kinds are Eureka's.** PR #24 names `feedback_listing`, `feedback_answer`, `feedback_close_request`, `feedback_hide`, owned by + `huginn-feedback-kinds-proposal.md`, which I could not read. PR #24 also notes the proposal's `feedback_answer` lists a `content_hash` that must go. + Their dependency order is Huginn readers, then the Persona organ, then this receiver, so nothing here can ship first. +- **How a Mind answers is open.** Bifrost has no `Mind` role; neither main nor PR #9 gives a Mind an account. +- **The consumer of the mouth's pending path** (the heartbeat) is outside this repo, and option B needs the Persona side to learn three kinds. +- **Backup purge or expiry** is required before deploy and unset (section 7a, item 5). +- **Erase is bounded** by what a Mind already holds (section 7a, item 4). +- **Unread:** Eureka's receiver notes in #minds, `huginn-feedback-kinds-proposal.md`, MAS's side of the session, and Heimdall PR #6 (the + answer-collector consent doc; the page's "guest answers are collected for nothing but delivery" is unchecked against it). +- The receiver writes nothing to Matrix and nothing to Huginn.