diff --git a/cli/src/slack-bridge.test.ts b/cli/src/slack-bridge.test.ts index 609c135b..e9742b44 100644 --- a/cli/src/slack-bridge.test.ts +++ b/cli/src/slack-bridge.test.ts @@ -1,7 +1,7 @@ import { test, describe } from "node:test"; import { strict as assert } from "node:assert"; import { parse as parseYaml } from "yaml"; -import { routeSlackMessage, slackToPlain } from "./slack/inbound.js"; +import { routeSlackMessage, slackToPlain, prefixAuthor } from "./slack/inbound.js"; import { slackAppManifest } from "./slack/manifest.js"; import { formatReceivedMessage, RECEIVED_MESSAGE_HEADER } from "./slack/announce.js"; @@ -11,19 +11,24 @@ const reasonOf = (d: ReturnType) => (d.action === "ign describe("routeSlackMessage", () => { test("delivers a human message in a mapped channel to the right agent", () => { const d = routeSlackMessage({ type: "message", channel: "C123", user: "U1", text: "focus on the lodash PR" }, MAPPING, "UBOT"); - assert.deepEqual(d, { action: "deliver", slug: "dependabot-scout", text: "focus on the lodash PR", files: [] }); + assert.deepEqual(d, { action: "deliver", slug: "dependabot-scout", text: "focus on the lodash PR", files: [], user: "U1" }); }); test("delivers a file_share with attachments even when the text is empty", () => { const files = [{ id: "F1", name: "screenshot.png", mimetype: "image/png", url_private: "https://files.slack.com/F1" }]; const d = routeSlackMessage({ type: "message", subtype: "file_share", channel: "C123", user: "U1", text: "", files }, MAPPING); - assert.deepEqual(d, { action: "deliver", slug: "dependabot-scout", text: "", files }); + assert.deepEqual(d, { action: "deliver", slug: "dependabot-scout", text: "", files, user: "U1" }); }); test("delivers a file_share with text and attachments together", () => { const files = [{ id: "F2", name: "ticket.pdf", mimetype: "application/pdf", url_private: "https://files.slack.com/F2" }]; const d = routeSlackMessage({ type: "message", subtype: "file_share", channel: "C123", user: "U1", text: "read this", files }, MAPPING); - assert.deepEqual(d, { action: "deliver", slug: "dependabot-scout", text: "read this", files }); + assert.deepEqual(d, { action: "deliver", slug: "dependabot-scout", text: "read this", files, user: "U1" }); + }); + + test("carries the Slack sender id so the bridge can attribute the message", () => { + const d = routeSlackMessage({ type: "message", channel: "C123", user: "UDREW", text: "who am I" }, MAPPING); + assert.equal(d.action === "deliver" ? d.user : undefined, "UDREW"); }); test("ignores the bot's own messages (no loops)", () => { @@ -46,6 +51,17 @@ describe("routeSlackMessage", () => { }); }); +describe("prefixAuthor", () => { + test("labels a message with its resolved Slack author", () => { + assert.equal(prefixAuthor("focus on the lodash PR", "Drew"), "From Drew (Slack):\nfocus on the lodash PR"); + }); + + test("returns the text unchanged when no name resolved", () => { + assert.equal(prefixAuthor("hello", undefined), "hello"); + assert.equal(prefixAuthor("hello", " "), "hello"); + }); +}); + describe("formatReceivedMessage", () => { test("marks the injected prompt as a received message and italicizes the body", () => { const out = formatReceivedMessage("Good morning, review the open Dependabot PRs."); diff --git a/cli/src/slack/bridge.ts b/cli/src/slack/bridge.ts index 0927111e..ff14cd95 100644 --- a/cli/src/slack/bridge.ts +++ b/cli/src/slack/bridge.ts @@ -2,7 +2,7 @@ import { existsSync, statSync, openSync, readSync, closeSync } from "node:fs"; import { join } from "node:path"; import { SocketModeClient } from "@slack/socket-mode"; import { SlackClient } from "./client.js"; -import { routeSlackMessage, ChannelMapping } from "./inbound.js"; +import { routeSlackMessage, prefixAuthor, ChannelMapping } from "./inbound.js"; import { formatTranscriptLines, formatCodexRolloutLines, TERMINAL_STOP_REASONS } from "./format.js"; import { loadAgentsConfig } from "../agents/config.js"; import { agentIdentity } from "../agents/identity.js"; @@ -197,6 +197,17 @@ export async function runSlackBridge(opts: BridgeOptions): Promise { // track recent relays here and drop the matching echo. Keyed by slug. const recentRelays = new Map(); const RELAY_ECHO_TTL_MS = 90_000; + // Resolve a Slack user id to a display name once and reuse it, so the agent + // sees who is steering it on every inbound message without an API call per + // message. A failed lookup is not cached, so a transient error can recover. + const userNameCache = new Map(); + const resolveUserNameCached = async (userId: string): Promise => { + const hit = userNameCache.get(userId); + if (hit) return hit; + const name = await client.resolveUserName(userId); + if (name) userNameCache.set(userId, name); + return name; + }; const consumeRelayEcho = (slug: string, mirrored: string): boolean => { const list = recentRelays.get(slug); if (!list?.length) return false; @@ -647,15 +658,21 @@ export async function runSlackBridge(opts: BridgeOptions): Promise { const prompt = formatPromptWithAttachments(decision.text, downloaded); if (!prompt) return; // text empty AND every attachment failed -> nothing to relay + // Prefix the sender so the agent can see who is steering it. Only human + // messages reach here (bot posts are dropped upstream), so this never + // double-labels an agent's own "From :" line. + const authorName = decision.user ? await resolveUserNameCached(decision.user) : undefined; + const authored = prefixAuthor(prompt, authorName); + // Mark this relay so the daemon does not echo it back to the channel // (it already appears there as that person's Slack message). Recorded // before the paste so the marker is in place before UserPromptSubmit. recordAnnounceSuppress(agentIdentity(decision.slug).sessionId); // Also remember it for the in-process Codex rollout-echo guard above. const relays = recentRelays.get(decision.slug) ?? []; - relays.push({ text: prompt, ts: Date.now() }); + relays.push({ text: authored, ts: Date.now() }); recentRelays.set(decision.slug, relays); - pasteTmuxPrompt(decision.slug, prompt); // tmux session name == slug + pasteTmuxPrompt(decision.slug, authored); // tmux session name == slug // Post a 👀 ack and light the working pill on it, ONLY if the agent // isn't already mid-turn. The eyes give the channel a visible diff --git a/cli/src/slack/client.ts b/cli/src/slack/client.ts index fe82034b..366a5873 100644 --- a/cli/src/slack/client.ts +++ b/cli/src/slack/client.ts @@ -65,6 +65,20 @@ export class SlackClient { }); } + /// Resolve a Slack user id to a human-readable name (display name preferred, + /// then real name, then the handle). Returns undefined if the lookup fails so + /// callers can fall back gracefully. + async resolveUserName(userId: string): Promise { + try { + const r = await this.web.users.info({ user: userId }); + const u = r.user as any; + const p = u?.profile ?? {}; + return p.display_name || p.real_name || u?.real_name || u?.name || undefined; + } catch { + return undefined; + } + } + /// Resolve "#name" / "name" to a channel id. Ids (C…/G…) are returned as-is. async resolveChannelId(nameOrId: string): Promise { if (/^[CG][A-Z0-9]+$/.test(nameOrId)) return nameOrId; diff --git a/cli/src/slack/inbound.ts b/cli/src/slack/inbound.ts index 834a8ca6..93f9ad8f 100644 --- a/cli/src/slack/inbound.ts +++ b/cli/src/slack/inbound.ts @@ -27,7 +27,18 @@ export interface SlackMessageEvent { export type InboundDecision = | { action: "ignore"; reason: string } - | { action: "deliver"; slug: string; text: string; files: SlackFile[] }; + | { action: "deliver"; slug: string; text: string; files: SlackFile[]; user?: string }; + +/// Prefix an inbound message with its Slack author so the agent can see who is +/// steering it. Agent-to-agent posts go through the bot token and are dropped +/// as bot messages before they reach here, so this only labels human messages +/// and never double-prefixes an agent's own "From :" line. With no +/// resolved name the text is returned unchanged. +export function prefixAuthor(text: string, authorName?: string): string { + const name = authorName?.trim(); + if (!name) return text; + return `From ${name} (Slack):\n${text}`; +} /// Convert Slack mrkdwn to plain text for the agent: unwrap links/mentions and /// unescape HTML entities Slack adds. @@ -66,5 +77,5 @@ export function routeSlackMessage( const files = event.files ?? []; if (!text && files.length === 0) return { action: "ignore", reason: "empty" }; - return { action: "deliver", slug, text, files }; + return { action: "deliver", slug, text, files, user: event.user }; }