State and credential contracts; chat_poll() stops returning $client - #10
Conversation
Nine verbs, and chat_poll() stops handing its client back. State (chat_channels, chat_history, chat_pending, chat_mark_read). chat_poll() answers "what changed since my cursor"; a process that just started has no useful cursor and needs "what is true now". chat_pending() is a separate verb rather than a mode of chat_poll() because overloading the cursor would make "start from nothing" and "tell me what is standing" the same call -- and a client that asked for pending invitations and thereby reset its read position would replay every channel it is in. chat_history() returns oldest-first whatever direction the platform paged in. Matrix and Slack both hand back newest-first and every consumer flips it; one flip in the adapter beats one per consumer, and a reversed transcript reads plausibly enough that nothing errors. Credentials (chat_matrix_config, chat_config_save, chat_matrix_configure, chat_relogin, chat_set_identity). chat_config keeps the application's own fields -- corteza stores its bots list and operator policy in the same file -- and carries app and path as attributes rather than fields, so neither is written back into the file as though it were a credential. print() shows field names and never values. chat_set_identity() is what unblocks 1f. The Matrix rename can rotate the access token underneath the caller, and a consumer that made that call itself had to notice and get the new token back into its client, which it did through the file they happened to share. Behind the contract the rotation lands in the client that performed it. So chat_poll() no longer returns $client. The cursor still reports the post-sync token; the refreshed credentials live on the client. Also fixes a hole the new verbs exposed. Every mx.api-backed method passed mx_client_session(client$env$mx) straight into its seam, and R made that a promise -- no seam double reads its session argument, so the session was never built and the test fixture had no token in it at all. The methods force it now, which is what production does the instant the real mx.api function touches it.
The contract has three kinds and no word for an image or a file, so anything unrecognized fell to m.text -- which posts a text message whose body is a filename. A caller that names a Matrix type explicitly now gets it, which is what stops corteza reaching around the contract to mx_send_text() for the msgtypes its exported matrix_send() documents.
The comment said unconditionally; the code returned early. A relogin inside the rename persists a refreshed token before retrying, so a retry that then fails has still rotated it -- the live token is on disk and the client is holding the rejected one. Nothing relogins on a send, so the next reply dies in a best-effort tryCatch with nothing logged. Found by mutation testing: cutting the reload out entirely left the suite green, which is what sent me looking for the case that would have covered it.
The blocker: `before` was documented as a message id, and Matrix's /messages does not take one. Its `from` is a pagination token out of a previous response, so handing it an event id does not page from that event. The contract also gave a consumer no way to continue at all -- one page and no token. There is no id that means the same thing on both reference transports: Slack pages by its own next_cursor, Matrix by `end`. So the contract does what it already does for chat_poll() -- the token is the adapter's, and a consumer only ever hands back what it was given. chat_history() now returns list(messages, cursor). NULL cursor means the channel has no more history behind this page, which on Matrix is `end` being omitted rather than the chunk being empty: a window can be all state events and still have conversation behind it. On Slack it is next_cursor coming back as "", which handed back to conversations.history is an error rather than a no-op. Loopback pages by a count rather than a message id, deliberately. It is what a new adapter gets read as an example, and one that paged by id would teach a contract the reference transport cannot honour.
The cursor-ignored mutation made it spin forever instead of failing. A hung suite is a worse report than a wrong one: no line number, no diff, just a runner that never finishes -- and it cost two five-minute timeouts to work out which mutation was responsible.
|
Fixed, and you were right about the shape of the fix. chat_history(client, channel, limit = 50L, cursor = NULL)
# -> list(messages = <chat_message, oldest first>, cursor = <opaque, NULL at the start of the channel>)Took option 1. There is no id that means the same thing on both reference transports — Matrix pages by Three things the fix turned up that the original would have got wrong even with translation bolted on: The stop signal is not an empty page. Matrix omits Slack sends Loopback pages by a count, deliberately. It is what a new adapter gets read as an example, and a reference implementation that paged by message id would teach a contract the reference transport cannot honour. corteza reads 846/846. 8 mutations on the pagination path, all caught. One of those mutations found a defect in my own test rather than the code: cutting the loopback cursor out made the page-walk loop spin forever instead of failing. A hung suite is a worse report than a wrong one — no line number, no diff, just a runner that never finishes. Bounded, with chat.api 0.0.1.17, corteza floor moved to match. Both branches pushed. |
Nine verbs, and phase 1f.
corteza's remaining 15 direct
mx.*calls were not stragglers of one job. They were two jobs the contract had no shape for, which is what the design inDESIGN.md(merged in #9) worked out. This implements both.State:
chat_channels,chat_history,chat_pending,chat_mark_readchat_poll()answers "what changed since my cursor". A process that just started has no useful cursor and needs "what is true now".chat_pending()is a separate verb rather than a mode ofchat_poll(). Overloading the cursor would make "start from nothing" and "tell me what is standing" the same call, and a client that asked for pending invitations and thereby reset its read position would replay every channel it is in. An invitation is standing state — which the contract already conceded whenchat_invitewas given no timestamp.chat_history()returns oldest-first whatever direction the platform paged in. Matrix and Slack both hand back newest-first and every consumer flips it. One flip in the adapter beats one per consumer, and a reversed transcript reads plausibly enough that nothing errors.Credentials:
chat_matrix_config,chat_config_save,chat_matrix_configure,chat_relogin,chat_set_identitychat_configkeeps the application's own fields — corteza stores its bots list and operator policy in the same file — and carriesapp/pathas attributes rather than fields, so neither is written back into the file as though it were a credential.print()shows field names and never values.1f
chat_set_identity()is what unblocked it. The Matrix rename is an authenticated call that can rotate the access token underneath the caller, so a consumer that made it had to notice the rotation and get the new token back into its client — which it did through the file they happened to share. Two rotation paths, two owners, one file. Behind the contract the rotation lands in the client that performed it.So
chat_poll()no longer returns$client. The cursor still reports the post-sync token; the refreshed credentials live on the client.A hole the new verbs exposed
Every mx.api-backed method passed
mx_client_session(client$env$mx)straight into its seam, and R made that a promise. No seam double reads its session argument, so the session was never actually built — the test fixture had notokenfield in it at all and nothing noticed. The methods force it now, which is what production does the instant the real mx.api function touches it.Verification
815/815. 13 mutations across the new surface, all caught.
One survived and was a real bug: cutting the config reload out of
chat_set_identity()left the suite green. Chasing that found the implementation contradicted its own comment — it claimed to reload unconditionally but returned early when the rename threw. A relogin persists a refreshed token before retrying, so a retry that then fails has still rotated it: the live token is on disk and the client holds the rejected one, and nothing relogins on a send.corteza's consumer is cornball-ai/corteza#173.