fix(slack): name channels in approval summaries, log resolver misses

STATUS.md

@@ -45,7 +45,7 @@
 - 10 service templates shipped as OpenAPI 3.1: Eventbrite, GitHub, Gmail, Google Calendar, Google Drive, Google Tasks, Resend, Slack, Stripe, X (plus the `overslash` platform namespace)
 - `outlook` service — Microsoft Graph mail (Gmail-equivalent): profile, messages (search/read/send/move/delete), drafts (create/send), and mail folders, over `provider: microsoft` (already seeded). Structured `sendMail` JSON gives clean approval disclosure without base64 decoding
 - OAuth for external MCP-runtime services — `x-overslash-mcp.auth: {kind: oauth, provider: …}` resolves the caller's owner connection (D22) into the outbound MCP bearer via the standard `oauth_providers`/`connections` machinery (shipped for HubSpot). `template_oauth_provider` + `McpDetail.provider` extended so oauth MCP services auto-connect, validate pinned connections, and surface a dashboard connect affordance.
-- `slack` service — Slack's Web API (`slack.com/api/*`) with a user token from the `slack` provider: `send_message`, `update_message`, `add_reaction`, `list_channels`, `read_channel_history`, `get_thread_replies`, `get_user`, `list_users`, `search_messages`. Writes disclose the channel by name through a `conversations.info` resolver. Replaces the D24 MCP wrapper, since Slack's MCP server only admits Marketplace-listed apps. Shared-app caveat: Slack caps `conversations.history`/`.replies` at 1 req/min and 15 messages for non-Marketplace apps, and an org's own BYOC Slack app is exempt.
+- `slack` service — Slack's Web API (`slack.com/api/*`) with a user token from the `slack` provider: `send_message`, `update_message`, `add_reaction`, `list_channels`, `read_channel_history`, `get_thread_replies`, `get_user`, `list_users`, `search_messages`. Summaries and disclosures name the channel (`'#general'`) through a `conversations.info` resolver on every channel-taking action, and `get_user` names the person (`users.info`); a channel Slack won't name (a DM, a scope the token lacks) falls back to the raw ID. HTTP resolver misses are now logged (`display-param resolver …`, with the upstream's in-band `error` code), so the reason is greppable. Replaces the D24 MCP wrapper, since Slack's MCP server only admits Marketplace-listed apps. Shared-app caveat: Slack caps `conversations.history`/`.replies` at 1 req/min and 15 messages for non-Marketplace apps, and an org's own BYOC Slack app is exempt.
 - `x-overslash-success` (alias `success`) — a jq predicate over a below-400 JSON body, set under `info` or per operation, HTTP only. A body that fails it is an upstream error for `is_error`, audit, response capture and paging, with the real status kept. It is stored on replay payloads and syntax-checked at template register (`success_invalid_jq`). Slack uses `.ok == true`.
 - Template/service instance split — templates (OpenAPI 3.1 blueprints with `x-overslash-*` extensions) + service instances (named, with credentials and lifecycle)
 - Three-tier template registry — global (read-only, shipped OpenAPI) + org (CRUD by org admins) + user (CRUD, gated by `user_template_policy`) (PR #100)

crates/overslash-api/src/services/param_resolver.rs

@@ -87,6 +87,19 @@ fn project(
     (display, canonical)
 }
 
+/// A top-level `error` code from a resolver response, when it looks like one:
+/// a short identifier, not a sentence that could echo the request back.
+fn upstream_error_code(body: &serde_json::Value) -> Option<&str> {
+    body.get("error")
+        .and_then(serde_json::Value::as_str)
+        .filter(|code| {
+            code.len() <= 64
+                && code
+                    .bytes()
+                    .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'_' | b'.' | b'-'))
+        })
+}
+
 /// Resolve display names for params that declare `source:` — answered from the
 /// gateway's own records rather than from the upstream.
 ///
@@ -225,12 +238,49 @@ pub async fn resolve_display_params(
                     for (key, value) in &headers {
                         req = req.header(key, value);
                     }
-                    let resp = req.send().await.ok()?;
-                    if !resp.status().is_success() {
+                    // Each miss is logged, as on the MCP path: an approval
+                    // quoting a raw id is otherwise undiagnosable. `get` is the
+                    // unsubstituted template path, never the URL, so the
+                    // caller's argument stays out of the log.
+                    let resp = match req.send().await {
+                        Ok(resp) => resp,
+                        Err(e) => {
+                            // reqwest's Display quotes the URL, which carries
+                            // the substituted argument.
+                            tracing::warn!(
+                                get = %get,
+                                param = %name,
+                                error = %e.without_url(),
+                                "display-param resolver request failed; using the raw argument"
+                            );
+                            return None;
+                        }
+                    };
+                    let status = resp.status();
+                    if !status.is_success() {
+                        tracing::warn!(
+                            get = %get,
+                            param = %name,
+                            status = status.as_u16(),
+                            "display-param resolver got an error status; using the raw argument"
+                        );
                         return None;
                     }
                     let json: serde_json::Value = resp.json().await.ok()?;
-                    Some(project(&resolver, &json))
+                    let projected = project(&resolver, &json);
+                    if projected.0.is_none() {
+                        // Providers that answer failures in-band (Slack's
+                        // `200 {"ok": false, "error": "missing_scope"}`) land
+                        // here. Their error code is a short enum, safe to log
+                        // and the one thing that says *why*.
+                        tracing::info!(
+                            get = %get,
+                            param = %name,
+                            upstream_error = upstream_error_code(&json).unwrap_or_default(),
+                            "display-param resolver projected nothing; using the raw argument"
+                        );
+                    }
+                    Some(projected)
                 }
                 .await;
                 (name, fetched)
@@ -473,7 +523,7 @@ fn resolver_body(
 
 #[cfg(test)]
 mod tests {
-    use super::{project, resolver_body};
+    use super::{project, resolver_body, upstream_error_code};
     use crate::services::mcp_client::ToolCallResult;
     use overslash_core::types::ParamResolver;
     use serde_json::json;
@@ -547,4 +597,20 @@ mod tests {
             (Some("Peluquería canina".to_string()), None)
         );
     }
+
+    #[test]
+    fn upstream_error_code_takes_only_a_short_identifier() {
+        assert_eq!(
+            upstream_error_code(&json!({"ok": false, "error": "missing_scope"})),
+            Some("missing_scope")
+        );
+        // A sentence could quote the request back, so it is not logged.
+        assert_eq!(
+            upstream_error_code(&json!({"error": "no channel named C1 for alice"})),
+            None
+        );
+        // `error` as an object (OAuth-style APIs) carries no flat code.
+        assert_eq!(upstream_error_code(&json!({"error": {"code": 404}})), None);
+        assert_eq!(upstream_error_code(&json!({"ok": true})), None);
+    }
 }

crates/overslash-api/tests/slack.rs

@@ -10,7 +10,10 @@
 //!   * Slack's in-band failure (`200 {"ok": false}`) comes back as `is_error`
 //!     and is audited as one, with the status left at 200;
 //!   * a paged read offers the next cursor, and stops on an in-band failure;
-//!   * a gated write discloses the channel by name, through the resolver;
+//!   * a gated write names the channel (`#general`) in its title and
+//!     disclosure, through the resolver, and falls back to the raw id when
+//!     Slack can't name it;
+//!   * read summaries name the thread's channel and the looked-up user;
 //!   * no connection → a clean error before any upstream call.
 //!
 //! Nothing reaches the network.
@@ -152,7 +155,17 @@ async fn start_fake_slack() -> (String, SeenLog) {
             "/api/chat.postMessage" => {
                 json!({ "ok": true, "channel": body["channel"], "ts": "1700000000.000100" })
             }
-            "/api/conversations.info" => json!({ "ok": true, "channel": { "name": "general" } }),
+            // Only `C1` exists. Anything else is Slack's in-band miss, which a
+            // resolver must survive by falling back to the raw id.
+            "/api/conversations.info" if query == "channel=C1" => {
+                json!({ "ok": true, "channel": { "name": "general" } })
+            }
+            "/api/conversations.info" => json!({ "ok": false, "error": "channel_not_found" }),
+            "/api/conversations.replies" => json!({ "ok": true, "messages": [] }),
+            "/api/users.info" => json!({
+                "ok": true,
+                "user": { "name": "alice", "real_name": "Alice Liddell" }
+            }),
             // Page one offers a cursor. Following it hits an in-band failure,
             // which must end the walk rather than offer the cursor again.
             "/api/conversations.list" if query.contains("cursor=") => {
@@ -385,6 +398,11 @@ async fn a_gated_send_discloses_the_channel_by_name() {
     )
     .await;
     assert_eq!(body["status"], "pending_approval", "{body}");
+    let title = body["action_description"].as_str().unwrap_or_default();
+    assert!(
+        title.contains("to '#general'"),
+        "the approval title must name the channel: {title}"
+    );
     let disclosed: std::collections::HashMap<String, String> = body["disclosed_fields"]
         .as_array()
         .expect("disclosed_fields present")
@@ -398,7 +416,7 @@ async fn a_gated_send_discloses_the_channel_by_name() {
         .collect();
     assert_eq!(
         disclosed.get("Channel").map(String::as_str),
-        Some("general")
+        Some("#general")
     );
     assert_eq!(
         disclosed.get("Message").map(String::as_str),
@@ -410,6 +428,63 @@ async fn a_gated_send_discloses_the_channel_by_name() {
     );
 }
 
+/// Slack can't name the channel (a DM, a private channel, a token without
+/// `channels:read`): the approval still goes up, quoting the raw id.
+#[tokio::test]
+async fn an_unresolvable_channel_falls_back_to_its_id() {
+    let h = setup(common::test_pool().await, false).await;
+    seed_slack_connection(&h).await;
+
+    let (status, body) = call(
+        &h,
+        "send_message",
+        json!({ "channel": "D0PRIVATE", "text": "psst" }),
+    )
+    .await;
+    assert_eq!(status, 202, "{body}");
+    assert_eq!(body["status"], "pending_approval", "{body}");
+    let title = body["action_description"].as_str().unwrap_or_default();
+    assert!(title.contains("to D0PRIVATE"), "{title}");
+    assert!(!title.contains('#'), "{title}");
+}
+
+/// Reads aren't gated, but their audit rows are what a human scans later.
+#[tokio::test]
+async fn read_summaries_name_the_channel_and_the_user() {
+    let h = setup(common::test_pool().await, true).await;
+    seed_slack_connection(&h).await;
+
+    for (action, params, expected) in [
+        (
+            "get_thread_replies",
+            json!({ "channel": "C1", "ts": "1700000000.000100" }),
+            "in '#general'",
+        ),
+        (
+            "get_user",
+            json!({ "user": "U1" }),
+            "'Alice Liddell (@alice)'",
+        ),
+    ] {
+        let (status, body) = call(&h, action, params).await;
+        assert_eq!(status, 200, "{action}: {body}");
+        let description: Option<String> = sqlx::query_scalar(
+            "SELECT description FROM audit_log
+              WHERE org_id = $1 AND action = 'action.executed'
+              ORDER BY created_at DESC LIMIT 1",
+        )
+        .bind(h.org_id)
+        .fetch_one(&h.pool)
+        .await
+        .expect("the call must write an audit row");
+        let description = description.unwrap_or_default();
+        assert!(
+            description.contains(expected),
+            "{action}: expected {expected:?} in {description:?}"
+        );
+    }
+}
+
 // ============================================================================
 // No connection
 // ============================================================================

services/slack.yaml

@@ -71,7 +71,7 @@ paths:
                   description: Channel ID (e.g. C0123456789), or a user ID for a DM.
                   resolve:
                     get: /api/conversations.info?channel={channel}
-                    pick: channel.name
+                    display: '#{channel.name}'
                 text:
                   type: string
                   minLength: 1
@@ -110,7 +110,7 @@ paths:
                   description: Channel ID the message is in.
                   resolve:
                     get: /api/conversations.info?channel={channel}
-                    pick: channel.name
+                    display: '#{channel.name}'
                 ts:
                   type: string
                   description: The message's `ts`.
@@ -148,7 +148,7 @@ paths:
                   description: Channel ID the message is in.
                   resolve:
                     get: /api/conversations.info?channel={channel}
-                    pick: channel.name
+                    display: '#{channel.name}'
                 timestamp:
                   type: string
                   description: The message's `ts`.
@@ -233,7 +233,7 @@ paths:
             minLength: 1
           resolve:
             get: /api/conversations.info?channel={channel}
-            pick: channel.name
+            display: '#{channel.name}'
         - name: limit
           in: query
           schema:
@@ -282,6 +282,9 @@ paths:
           schema:
             type: string
             minLength: 1
+          resolve:
+            get: /api/conversations.info?channel={channel}
+            display: '#{channel.name}'
         - name: ts
           in: query
           required: true
@@ -318,6 +321,9 @@ paths:
           schema:
             type: string
             minLength: 1
+          resolve:
+            get: /api/users.info?user={user}
+            display: '{user.real_name}[ (@{user.name})]'
   /api/users.list:
     get:
       operationId: list_users