Skip to content

Commit 47afcdc

Browse files
jrosskopfclaude
andauthored
feat(msteams): out-of-band OutboundCourier for proactive Teams delivery (#327)
* feat(msteams): out-of-band OutboundCourier — proactive delivery to a stored conversationReference Async-ops Phase 3 (Teams push), the write half: deliver a terminal operation result back to the Teams conversation that started it, out-of-band (no inbound turn in flight). Today msteams can only reply as the tail of handling an inbound webhook; there is no seam to send to a *persisted* conversationReference. - triton-core: `OutboundRequest` gains an optional `reference` (a channel-specific routing value). Adapters keyed solely on `to` (WhatsApp, Twilio, email) ignore it; the Teams courier requires it — the `serviceUrl` is per-tenant and separate from any flat recipient id. - triton-adapters-http: `/v1/outbound` accepts + forwards `reference`. - triton-chat-msteams: `impl OutboundCourier for MsTeamsAdapter`. `authorize` re-checks the stored `serviceUrl` host against the allow-list (defence in depth) and binds the conversation's tenant to the caller's (#113); `deliver` renders via the existing surface mapper and POSTs through `post_activity_to` — a small refactor of the private `post_activity` that takes an explicit serviceUrl, so the inbound reply and the proactive send share one poster + the federated Bot Connector token path. Audit rides `record_post` (ADR-6). A missing/blank reference fails closed. - triton-bin: register the msteams courier in the outbound registry (mirrors the WhatsApp arm), so `/v1/outbound {"adapter":"msteams",...}` resolves. No-mock DoD (triton-tests `msteams_outbound`, real binary + real OIDC issuer + FakeBotFramework connector): a proactive send delivers the activity to the stored conversation and audits `posted`; a cross-tenant conversation is 403 with nothing couriered; a send with no reference is refused. Existing msteams + outbound suites stay green (82 passed); fmt + clippy clean. Part of the async-operations → chat delivery chain (escurel operation → runner courier → agent /v1/outbound → this seam). Follow-ups: surface the inbound conversationReference to the host to persist (T2), and the agent-side receiver + start_operation call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(msteams): T2 — surface the inbound conversationReference to the host router The out-of-band courier (T1) needs a persisted Teams conversationReference to deliver back to. Triton parsed it inside handle_webhook and discarded it — the host (agent) had no hook to capture it. This surfaces it on the existing host seam. - triton-chat-routing: `RouteCtx` gains an optional `conversation_ref: Option<Value>` — the channel's reference for a LATER out-of-band send. The host persists it against a long-running operation so a terminal result can be delivered back to this exact conversation. `None` for channels/turns with no capturable reference. - triton-chat-msteams: both router call sites (inbound message + chooser-pick callback) populate it via `conversation_reference_json`, flattening the Activity into the shape the T1 courier parses — inbound `recipient` (bot) → `bot_id` (outbound `from`), inbound `from` (user) → `user_id` (outbound `recipient`), JWT-derived serviceUrl + tenant. - triton-chat-googlechat: sets `conversation_ref: None` (replies synchronously on the webhook; proactive delivery not wired there yet). Unit test (`conversation_reference_is_flattened_for_the_courier`): the flattened reference carries the right ids and round-trips straight back into the courier's `TeamsConversationRef`. Routing units + the msteams/google_chat integration suites stay green (94 passed); fmt + clippy clean. The full inbound-turn → router-sees-conversation_ref → persist E2E lands with the agent-side receiver (the host's AgentRouter impl persists it against the operation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(msteams): T2 — self-describing channel in the conversationReference The delivery side (the agent's outbound-callback receiver) routes a terminal operation result to the right triton adapter by this channel discriminator, so the persisted reference is self-describing rather than assumed to be Teams. * sec(msteams): the outbound tenant binding was optional, so callers opted out Crew review of this PR (security · codex, correctness · claude, testing · muse) returned REQUEST_CHANGES, converging on one root cause: the persisted `conversationReference` is unauthenticated caller-supplied JSON, and it decided both where a real Bot Connector bearer went and which tenant the send was bound to. **The binding was `Option<String>`, absent meaning "no binding asserted".** The reference comes from the caller, so the caller chose whether to be bound: omit one field and an `outbound:send` holder could deliver into any conversation id it knew. `cross_tenant_conversation_is_ forbidden` passed throughout, because it supplied the field. Required now — which costs a genuine reference nothing, since `conversation_reference_json` always writes the real tenant. Red first: `a_reference_without_a_tenant_is_forbidden` drives the real binary and the real connector fake. It asserts **400**, not 403, and the difference is deliberate — a reference with no tenant is MALFORMED, one naming another tenant is FORBIDDEN, and keeping the two distinguishable is what stops a future regression passing as "some 4xx". Also closed from the same review: * **`deliver` re-runs `authorize`.** `OutboundCourier` is a public trait, so `deliver` is reachable without the endpoint that gated it. A gate one call site away from the thing it guards is a convention. * **`conversation_id` is validated before it becomes a path segment.** It is interpolated into the connector URL, so `..`, `/`, `?` or `#` aimed a POST carrying a real bot token at a different endpoint. `an_attacker_controlled_service_url_is_forbidden` is added as a fence and passes ALREADY — the exfiltration half was closed by #329, which narrowed `SERVICE_URL_HOST_SUFFIXES` off the whole `trafficmanager.net` namespace. It covers suffix confusion, userinfo smuggling and a plain-http downgrade so the property cannot regress quietly. cargo test --workspace --no-fail-fast: 923 passed, 0 failed. * fix(msteams): a stale conversation is dropped, not retried forever Crew finding F9. The courier marked EVERY non-2xx `PostOutcome::Retry`, while the inbound reply path in the same file already classified correctly — `>= 500 || 429` retry, else drop. Same connector, same failures, two answers. It matters more here than inbound: Teams answers 403/404 when the bot has been removed from a conversation, and for PROACTIVE delivery a stale `conversationReference` is the expected steady state rather than an exception. The whole point of the feature is sending to a conversation that ended long ago. Marking those `Retry` meant retrying the one failure that cannot succeed. Red first: `a_stale_conversation_is_dropped_not_retried` drives the real binary and asserts the audit line's `status_label`. It needed a seam the fixture did not have — `FakeBotFramework::set_activity_status` — because without a way to produce a 403 the classification was pinned by nothing. Before the fix the line read `"status":403,"status_label":"retry"`. cargo test --workspace --no-fail-fast: 924 passed, 0 failed. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c1658c4 commit 47afcdc

12 files changed

Lines changed: 795 additions & 5 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/triton-adapters-http/src/outbound.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ struct OutboundBody {
8282
/// `category`.
8383
#[serde(default)]
8484
variables: Vec<String>,
85+
/// Optional channel-specific routing reference (e.g. a Teams
86+
/// `conversationReference`) for adapters whose recipient is more than a
87+
/// flat `to`. Ignored by adapters keyed solely on `to`.
88+
#[serde(default)]
89+
reference: Option<serde_json::Value>,
8590
}
8691

8792
async fn outbound_send(
@@ -165,6 +170,7 @@ async fn outbound_send(
165170
result: body.result,
166171
category: body.category,
167172
variables: body.variables,
173+
reference: body.reference,
168174
};
169175

170176
// 5. Authz layer 2 (#113): recipient/tenant binding, behind the

‎crates/triton-bin/src/main.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,15 @@ async fn main() -> std::io::Result<()> {
12221222
canonical_claimed = true;
12231223
}
12241224
tracing::info!(adapter = %name, "msteams webhook adapter wired");
1225-
let r = Arc::new(built).router();
1225+
// Keep an Arc for the outbound courier registry
1226+
// (proactive async-operation delivery) before
1227+
// `router()` consumes one — mirrors the WhatsApp arm.
1228+
let built = Arc::new(built);
1229+
outbound_couriers.insert(
1230+
name.to_string(),
1231+
built.clone() as Arc<dyn triton_core::OutboundCourier>,
1232+
);
1233+
let r = built.router();
12261234
chat_router = Some(match chat_router.take() {
12271235
Some(acc) => acc.merge(r),
12281236
None => r,

‎crates/triton-chat-googlechat/src/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,6 +1411,9 @@ async fn handle_webhook(
14111411
tenant: &tenant,
14121412
caller_sub: &sub,
14131413
pick,
1414+
// Google Chat replies synchronously on the webhook; proactive
1415+
// out-of-band delivery is not wired here yet.
1416+
conversation_ref: None,
14141417
};
14151418
match router.route(ctx).await {
14161419
triton_chat_routing::RouteOutcome::Dispatch { agent_id, text } => {

‎crates/triton-chat-msteams/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ license.workspace = true
77
publish = false
88

99
[dependencies]
10+
async-trait = "0.1"
1011
tokio-util = { workspace = true, features = ["rt"] }
1112
# Decode an upstream-rendered chart PNG (peacock render_report `png_base64`)
1213
# and content-address it for the signed `/{name}/img/{token}` route.

0 commit comments

Comments
 (0)