agent: contact request rejection and service requests

[epoberezkin]

No description provided.

[simplex-chat-agent]

Adds two agent capabilities on top of the DR-enabled contact address feature: (1) contact-request rejection with an optional reason delivered over the double ratchet (RJCT), and (2) a service RPC round-trip — sendServiceRequest blocks for a single AgentServiceResponse/AgentRejection reply, with sync and async variants on both sides.

I read every touched file in full, followed the message flow through Agent.hs (serviceRequest_, prepareReply, sendReply{Sync,Async}, joinConnSrv', smpConfirmation, smpContactRequest, ICReplyDel, cleanupManager), the store/schema/migration changes, and the protocol encodings. The design is coherent and the tests exercise sync, async, rejection, and cross-outage resilience. Encodings don't collide (AgentMessage A/P/J, AgentMessageType A/P/J vs existing tags), smpConfirmation is always followed by ack, the in-memory serviceRequests TMVar map is keyed by a freshly-generated connId so concurrent requests can't collide, and the orElse (registerDelay) timeout logic is correct. Leftover requester connections and unanswered received requests are cleaned up by getExpiredServiceConns/deleteExpiredServiceRequests. No blocking issues found.

Non-blocking observations for the author:

1. Request vs response timeout asymmetry. The requester's end-to-end wait is serviceRequestTimeout (30s) while the responder may still reply within serviceResponseTimeout (180s) — prepareReply accepts requests up to 180s old and AM_SRV_RESP delivery is retried for 180s. A reply produced between 30s and 180s arrives after the requester has already timed out and torn down the reply connection, yielding ERR (A_SERVICE ASENoPendingRequest) and a wasted round-trip. If the 30s/180s split is intentional (interactive requester patience vs. service-side lifetime), fine; otherwise the requester wait should be >= the response window. Confirm this is deliberate.

2. Non-DR contact address silently degrades to a contact invitation. sendServiceRequest' → joinConnSrv' CRContactUri: when the target address publishes no ratchet keys (ratchet_ == Nothing), the mkInner/AgentServiceRequest payload is ignored and a plain AgentInvitation is sent. The recipient then sees a normal REQ (not SREQ) and the requester blocks until the 30s timeout. Service RPC only makes sense against a DR address, so consider failing fast with a clear error in that branch rather than emitting a contact invitation and timing out.

3. Minor duplication. The ICReplyDel handler body (secure snd queue → submitPendingMsg → deleteConnectionAsync' True) is identical to sendReplySync with nm = NRMBackground; could call sendReplySync directly.

4. Migration rename. M20260712_address_dr → M20260712_address_dr_rpc (name and column additions) is safe only because the address_dr migration is unreleased; renaming an applied migration would break the migrations table. Assuming this branch supersedes the still-unreleased address-dr work, this is fine.

[simplex-chat-agent]

Reviewed the full branch (d5f48de3b56b...ccc1d7f8eca5): all 18 files, following the new agent APIs, the store/migration changes, and the SMP transmission handling.

The change adds two capabilities over contact-address double-ratchet connections:

• Contact request rejection with a reason (rejectContact/rejectContactAsync), surfaced to the requester as the new RJCT event.
• Service (RPC) requests (sendServiceRequest/Async), with responses (sendServiceReply/Async) and rejections (rejectServiceRequest/Async), surfaced to the service as the new SREQ event.

The wire changes are consistent: AgentInvitationDR→AgentContactRequest (envelope tag 'J'→'A'), three new AgentMessage variants (A/P/J) with matching AgentMessageTypes, and the new AEvent/AEventTag constructors are threaded through aEventTag. Roundtrip encoding is covered by the new ConnectionRequestTests case, and the async/resilient flows have functional tests. The serviceRequests TMVar map, the timeout handling (serviceRequestTimeout/serviceResponseTimeout), the ICReplyDel deferred-reply command, and the cleanup path (getExpiredServiceConns/deleteExpiredServiceRequests) all hang together correctly, including the orphaned-connection GC after an agent restart.

Concurrency and lifecycle look sound: the TMVar is inserted before the send is enqueued, the blocking caller uses registerDelay+orElse for timeout, and stale/cancelled requests are handled by the Nothing branch in the async JOIN JRServiceReq worker. The serviceResponse connection flag and service_request invitation flag are persisted with sensible migration defaults (created_at defaulting to epoch for pre-existing rows, which never match the service_* GC predicates).

Only one nit found — a typo in a new record field name (conntactReq), suggested inline.

Two non-blocking observations:

• Migration rename. M20260712_address_dr is renamed to M20260712_address_dr_rpc (folding in the new columns) rather than adding a new migration. This is correct only if 20260712_address_dr has never shipped to any client DB; if it has, deployed installs recorded the old name and would re-run the CREATE TABLE address_ratchet_keys. Given both migrations share the same date and author and this is a stacked branch, it appears to be in-development — worth a quick confirm.
• Payload size. Service request/response payloads are encrypted into the fixed e2eEncConnInfoLength envelope (~11KB with PQ, ~14.8KB otherwise). Payloads beyond that fail encryption and surface as an error to the caller. Fine as a design bound for RPC, just noting it.

[simplex-chat-agent]

Adds two related capabilities on top of the address-DR feature: (1) contact-request rejection with a reason delivered over the double ratchet, and (2) service (RPC) requests — a client sends a signed request to a DR-enabled contact address and blocks for a single response/rejection. New agent APIs (sendServiceRequest[Async], sendServiceReply[Async], rejectServiceRequest[Async], rejectContact[Async]), new wire messages (AgentServiceRequest/Response/Rejection, RequestSignature), new events (SREQ/SSENT/RJCT, and a Bool added to REQ), a serviceRequests in-memory correlation map, DB columns for request expiry/kind, and an ICReplyDel internal command for durable async replies.

I read the full cumulative diff and traced the touched code paths (Agent.hs RPC/reply/delivery, Protocol encodings, Store/AgentStore, migrations, Client/Env, Crypto, tests). The implementation is coherent and well-tested (sync + async + resilience across server outages). No blocking issues found. Notes below.

Verified correct

• Signature binding. serviceReqBinding = sha3_256("SimpleXService" <> rcAD); a subagent confirmed rcAD is byte-identical on requester (Snd ratchet) and service (Rcv ratchet), so signatures verify. The signature also travels inside the DR-encrypted envelope, and an unsigned request yields key_ = Nothing for the service to reject at the app layer.
• Timeout/dispatch STM in serviceRequest_ (takeTMVar var \orElse` registerDelay) is correct; late replies after cleanup surface as ASENoPendingRequest` rather than corrupting state.
• deleteConnectionAsync' c True … uses waitDelivery=True, so the stored reply (AM_SRV_RESP/AM_RJCT, sent via sendConfirmation) is delivered before the reply connection is torn down.
• Async JOIN handler serialises with serviceRequest_ cleanup via withConnLock; a request whose waiter already vanished (Nothing in serviceRequests) is dropped rather than sent, and the persisted command is removed. Replay of a contact/service request is dropped via the existing ratchet-key-hash de-dup.
• Envelope tag 'J' → 'A' (rename AgentInvitationDR → AgentContactRequest) and the new inner AgentMessage tags don't collide; JoinRequest's 'S' prefix disambiguates from JRConnReq/JRInvitationDR.

Worth confirming (non-blocking)

• Timeout asymmetry. Default serviceRequestTimeout (client wait) is 30s but serviceResponseTimeout (service validity/expiry window) is 180s. A reply produced between 30–180s arrives after the client has already returned ASETimeout and is discarded (ASENoPendingRequest). This is fine given per-request override, but confirm the defaults are intended.
• Crypto.hs enables StrEncoding (PrivateKey Ed25519) globally (removing the "Do not enable, to avoid leaking key data" guard) so the signing key can be persisted inside the JRServiceReq async command. This mirrors the already-enabled X25519 instance and the key lives only in the (encrypted) agent DB, but it does broaden the surface for accidental serialization of a private key. Consider whether a scoped/newtype instance would be tighter.
• Migration M20260712_address_dr is replaced in place by M20260712_address_dr_rpc (same date, new id) rather than added as a follow-up migration. Safe only if 20260712_address_dr was never applied anywhere (unreleased); on any DB that already ran it, the new migration's CREATE TABLE address_ratchet_keys will fail. Confirm no such DBs exist.
• getExpiredServiceConns scans connections with no index on service_request_expires_at each cleanup cycle. Consistent with existing getDeletedConnIds, so not new, but keep in mind for large stores.
• Minor: the kindErr strings in prepareReply/rejectRequest_ are duplicated with slightly divergent wording, and sendReplySync repeats deleteConnectionAsync' c True connId in both branches — readable as-is, low priority.

One tiny inline suggestion (stray blank line in AgentConfig).

[simplex-chat-agent]

Adds contact-request rejection (with a DR-carried reason) and a request/response service-RPC layer over contact addresses. The design is sound and reuses existing structure well: the reply travels back as an SMP confirmation on the requester's reply queue, the signature is bound to the double-ratchet associated data so it cannot be replayed to another service or request, and the request/reply state machine keys off serviceRequestExpiresAt (client side) and the service_request flag on the invitation (service side). I verified the two points that could silently break this:

• rcAD (the signature binding source) is provably identical on both peers — it is pubKeyBytes(joiner_pub1) <> pubKeyBytes(creator_pub1) on both pqX3dhSnd and pqX3dhRcv, so verifyServiceReq will agree with signServiceReq.
• The service reply is acked (smpConfirmation ... >> ack), so dispatchServiceReply won't fire a spurious ASENoPendingRequest on redelivery.

Concurrency around serviceRequests is correctly serialized: both serviceRequest_ cleanup and the async JOIN handler take withConnLock c connId, and the timed-out waiter deletes the map entry before the retrying command re-checks it (finding Nothing and skipping the send). Encodings are collision-free (AgentMessage/AgentMessageType tags A/P/J don't clash) and round-trip tested. The migration rename M20260712_address_dr → _rpc is fine since the original is unreleased.

Two non-blocking observations (no code change requested):

• connections.created_at is added to the migration and schema and written in createConnRecord, but never SELECTed anywhere in simplexmq. If it isn't needed by a consumer, consider dropping it — it's dead weight on a WITHOUT ROWID, STRICT table, and the migration is unreleased so removal is cheap. Cleanup and expiry use conn_invitations.created_at and connections.service_request_expires_at, not this column.
• Delivery retry timeout for AM_RJCT falls through to messageTimeout (2 days), while AM_SRV_RESP uses serviceResponseTimeout (180s). A service rejection is only awaited by the requester for serviceRequestTimeout (30s), and the service-side reply connection carries no service marker, so it isn't reclaimed by getExpiredServiceConns; an offline requester leaves the service retrying a rejection for up to 2 days. AM_RJCT is shared with contact rejection (where 2 days is intended), so there's no clean per-msgType fix — worth a look if service-rejection connection buildup matters.

One trivial inline fix below (the previously-flagged blank line in AgentConfig, still present).