Repository navigation
Allow authenticated message sending over REST - #106
Merged
Merged
Conversation
csp-bot already serves its channels over csp-gateway's REST API, but messages_out was read-only, so a job had no way to ask the bot to deliver a message. allow_send_messages, off by default, registers it as a send channel so POST /api/v1/send/messages_out works. Two things had to change for a posted message to arrive: The publish loop filtered the edge built locally from command responses, not the channel, so anything else feeding messages_out never reached a backend. It now reads the channel, which the gateway merges with the send adapter after every module has connected. This is not a cycle: subscribe() is a source and publish() a sink, so nothing flows back into what composes the channel. Verified by running the graph. _filter_messages_for_backend only looked at metadata["backend"], which is written by inbound tagging. A caller posting a message sets chatom's backend field instead, so the message was accepted and then silently dropped. The filter now falls back to that field, with metadata still taking precedence. This is the same class of bug as the one TestMetadataPropagation covers, fixed at the filter rather than at each producer. Off by default because it turns the bot into a relay for anything that can reach its API. The documentation pairs it with configuring an API key, since MountAPIKeyMiddleware only rejects requests once it has one. Verified against a Symphony pod: an authenticated POST carrying MessageML built by chatom arrived in the room with its headings, table, and bold intact. Signed-off-by: Tim Paine <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #106 +/- ##
==========================================
+ Coverage 84.56% 84.68% +0.11%
==========================================
Files 44 44
Lines 4569 4604 +35
Branches 337 338 +1
==========================================
+ Hits 3864 3899 +35
+ Misses 600 598 -2
- Partials 105 107 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
What this adds
allow_send_messages, off by default, makesmessages_outa send channel so an authenticated caller can ask the bot to deliver a message:csp-bot already serves its channels over csp-gateway's REST API, but
messages_outwas read-only, so a job had no way in.Two fixes were needed for a posted message to arrive
The publish loop read the wrong edge. It filtered the edge built locally from command responses rather than the channel, so anything else feeding
messages_outnever reached a backend. It now reads the channel, which the gateway merges with the send adapter after every module has connected.This is not a cycle, which was the main thing I wanted to be sure of:
subscribe()is a source andpublish()a sink, so nothing here flows back into what composes the channel. Confirmed by building and running the graph rather than reasoning about it.The backend filter ignored the field a caller would set.
_filter_messages_for_backendonly looked atmetadata["backend"], which inbound tagging writes. A message posted over REST sets chatom's ownbackendfield, so it was accepted by the send endpoint with a 200 and then silently dropped — the worst failure mode. The filter now falls back to that field, with metadata still taking precedence.That is the same class of bug
TestMetadataPropagationalready documents, where a command returned aMessagewithbackendset but no metadata. That was fixed at the producer; this fixes it at the filter, so any future producer is covered. I updated that class's docstring, which claimed the filter looks at metadata and notmsg.backend.Why it is off by default
Turning it on makes the bot relay anything that can reach its API.
MountAPIKeyMiddlewareis mounted by default but rejects nothing until it has a key, so the documentation pairs the two settings.Verification
Against a live Symphony pod:
POST /api/v1/send/messages_outwith no token →403200, and the message appears in the roomMessageBuilderand rendered to MessageML arrives with its<h3>,<table>,<b>, and<td>intactSix new tests cover metadata routing, field routing, metadata winning over the field, a message naming no backend going nowhere, and the opt-in defaulting off. I checked the field-routing test fails without the filter change.
Suite: 288 passed, up from 282. The twelve failures are
ModuleNotFoundError: No module named 'claude_agent_sdk'in my environment and are identical on an unmodified checkout.make lintpasses.Note
The REST surface for
messages_inandmessages_outreturns 500 on csp-gateway as released, because its response path callstype_adapter()and chatom'sMessageis a plain pydantic model. Point72/csp-gateway#378 fixes that, and this feature needs it to be useful. Worth merging that first.