diff --git a/packages/cap-discord/src/index.ts b/packages/cap-discord/src/index.ts index d42110e..f5b3f69 100644 --- a/packages/cap-discord/src/index.ts +++ b/packages/cap-discord/src/index.ts @@ -37,15 +37,24 @@ export default async function (pi: ExtensionAPI): Promise { config, }); - // Open the gateway. A connect failure is logged but NOT fatal: the agent - // session (and the outbound tools) should still come up. The persistent - // runtime (PR4) owns reconnection/KeepAlive. The error never includes the - // token (it lives only inside the client; login errors name only the cause). - try { - await wired.start(); - } catch (err) { - const reason = err instanceof Error ? err.message : "gateway connect failed"; - console.error(`discord capability: gateway connect failed (continuing): ${reason}`); + // Open the gateway (inbound listener) ONLY in the persistent runtime. A + // one-shot `bob run` keeps run minimal: it gets the outbound REST tools (which + // need no login) but opens no gateway — so it never duplicates the persistent + // session's bot login (the second-connection conflict) or pays the connect + // cost for a task that never receives a message. Bob sets BOB_PERSISTENT=1 in + // the persistent runtime (createPiRunSession); it's unset for `bob run`. + if (process.env.BOB_PERSISTENT === "1") { + // A connect failure is logged but NOT fatal: the session + outbound tools + // still come up. The error never includes the token (it lives only inside + // the client; login errors name only the cause). + try { + await wired.start(); + } catch (err) { + const reason = err instanceof Error ? err.message : "gateway connect failed"; + console.error(`discord capability: gateway connect failed (continuing): ${reason}`); + } + } else { + console.error("discord capability: run mode — outbound REST tools only, gateway not opened"); } } diff --git a/packages/cap-discord/test/e2e.test.ts b/packages/cap-discord/test/e2e.test.ts index 8f41b6d..4b9635e 100644 --- a/packages/cap-discord/test/e2e.test.ts +++ b/packages/cap-discord/test/e2e.test.ts @@ -12,8 +12,10 @@ import { CONFIG_ENV_VAR } from "../src/config.js"; // The load-bearing proof (mirrors the cap-fixture e2e): a REAL pi AgentSession // built with the discord extension source surfaces discord_reply/react/fetch in -// session.getAllTools(). No live gateway, no real token — the extension's -// connect failure on an invalid token is non-fatal, so tools still register. +// session.getAllTools(). No live gateway, no real token. This runs in RUN mode +// (BOB_PERSISTENT unset), so the extension opens NO gateway at all — the +// outbound REST tools register regardless. (Persistent mode would additionally +// attempt the gateway connect, which is non-fatal on a bad token.) const extensionPath = resolve(dirname(fileURLToPath(import.meta.url)), "..", "src", "index.ts"); @@ -27,10 +29,13 @@ describe("discord capability — end to end with a real pi session", () => { writeFileSync(tokenFile, "invalid.token.value\n", "utf8"); const prevEnv = process.env[CONFIG_ENV_VAR]; + const prevPersist = process.env.BOB_PERSISTENT; process.env[CONFIG_ENV_VAR] = JSON.stringify({ tokenFile, channelIds: ["123456789012345678"], }); + // RUN mode — no gateway. Outbound REST tools must still register. + process.env.BOB_PERSISTENT = ""; try { const loader = new DefaultResourceLoader({ @@ -58,6 +63,8 @@ describe("discord capability — end to end with a real pi session", () => { } finally { if (prevEnv === undefined) delete process.env[CONFIG_ENV_VAR]; else process.env[CONFIG_ENV_VAR] = prevEnv; + if (prevPersist === undefined) delete process.env.BOB_PERSISTENT; + else process.env.BOB_PERSISTENT = prevPersist; rmSync(cwd, { recursive: true, force: true }); rmSync(agentDir, { recursive: true, force: true }); } diff --git a/packages/discord/src/index.ts b/packages/discord/src/index.ts index 5ca9ae3..3b2e24b 100644 --- a/packages/discord/src/index.ts +++ b/packages/discord/src/index.ts @@ -3,17 +3,20 @@ // Importing this package pulls discord.js (~30MB of WS + REST). Agents // that don't need Discord shouldn't depend on this package — keep the // shell-only install slim. +// +// OUTBOUND vs INBOUND are decoupled (so a one-shot `bob run` stays minimal): +// * OUTBOUND (reply/react/fetch) goes through `client.rest` — discord.js's +// REST manager. It needs only the token (setToken in the constructor); it +// does NOT require login()/a gateway connection. It still sends the correct +// `User-Agent` and honors `Retry-After` on 429 (the @discordjs/rest +// RequestManager hygiene — the thing the failed raw `curl` lacked). This is +// NOT a raw fetch: it's the client's own REST path. +// * INBOUND (the message listener) needs the gateway, so it requires +// connect() (login). Only the PERSISTENT runtime calls connect(); a one-shot +// run gets the outbound REST tools with no gateway, no duplicate login. import type { DiscordClient, DiscordMessage } from "@tpsdev-ai/bob-shell"; -import { Client, Events, GatewayIntentBits, type Message, type TextBasedChannel } from "discord.js"; - -// All REST/gateway traffic flows through discord.js, which sends a correct -// `User-Agent` (`DiscordBot (https://discord.js.org, )`) and honors the -// `Retry-After` header on 429 by default (see @discordjs/rest RequestManager: -// it reads `Retry-After`, sleeps, and retries). The discord capability relies -// on that — the failed `bin/post-discord` curl sent NO User-Agent and ignored -// `retry-after`, which fed the Cloudflare-1015 class. Do NOT introduce a raw -// fetch on this path; keep everything on the client so that hygiene holds. +import { Client, Events, GatewayIntentBits, type Message, Routes } from "discord.js"; export interface DiscordJsClientOptions { // Bot token. Read from a secret file in production; passed inline in @@ -37,6 +40,17 @@ export function shouldProcessMessage(authorId: string, ownBotId: string | undefi return !ownBotId || authorId !== ownBotId; } +// The slice of a raw Discord API message (REST GET /channels/:id/messages) that +// fetchRecent reads. (Raw API uses snake_case + a mentions ARRAY — not the +// discord.js Message object.) +interface RawApiMessage { + id: string; + channel_id: string; + author: { id: string; username: string }; + content: string; + mentions?: Array<{ id: string }>; +} + export class DiscordJsClient implements DiscordClient { private readonly client: Client; private readonly token: string; @@ -54,6 +68,10 @@ export class DiscordJsClient implements DiscordClient { GatewayIntentBits.MessageContent, ], }); + // Enable the REST manager WITHOUT logging in — outbound works in a one-shot + // run with no gateway connection. (login() also sets the token; doing it + // here makes REST usable before/without connect().) + this.client.rest.setToken(this.token); this.client.on(Events.ClientReady, (c) => { // Pin the bot user ID for mention detection once the gateway is live. @@ -83,6 +101,8 @@ export class DiscordJsClient implements DiscordClient { this.messageHandler = handler; } + // Open the gateway (login). INBOUND-only — outbound already works via REST. + // The persistent runtime calls this; a one-shot run does not. async connect(): Promise { await this.client.login(this.token); } @@ -91,55 +111,42 @@ export class DiscordJsClient implements DiscordClient { await this.client.destroy(); } + // --- OUTBOUND (REST, no gateway needed) ------------------------------- + async reply(channelId: string, text: string, opts?: { replyTo?: string }): Promise { - const channel = await this.requireTextChannel(channelId); - await (channel as unknown as { send: (payload: unknown) => Promise }).send({ - content: text, - reply: opts?.replyTo ? { messageReference: opts.replyTo } : undefined, + await this.client.rest.post(Routes.channelMessages(channelId), { + body: { + content: text, + // Raw API reply shape. fail_if_not_exists:false → if the referenced + // message is gone, post a normal message instead of erroring. + message_reference: opts?.replyTo + ? { message_id: opts.replyTo, fail_if_not_exists: false } + : undefined, + }, }); } async react(channelId: string, messageId: string, emoji: string): Promise { - const channel = await this.requireTextChannel(channelId); - // channel.messages.react goes through discord.js REST (correct UA + - // retry-after). emoji is a unicode glyph or a "name:id" custom-emoji ref. - await ( - channel as unknown as { - messages: { react: (m: string, e: string) => Promise }; - } - ).messages.react(messageId, emoji); + // PUT /channels/:c/messages/:m/reactions/:emoji/@me — emoji must be URL- + // encoded (unicode glyph or a "name:id" custom-emoji ref). + await this.client.rest.put( + Routes.channelMessageOwnReaction(channelId, messageId, encodeURIComponent(emoji)), + ); } async fetchRecent(channelId: string, limit: number): Promise { - const channel = await this.requireTextChannel(channelId); - // discord.js `messages.fetch` returns a Collection (a Map subclass). A bare - // `for…of` over a Map yields [key, value] PAIRS, not Message objects — so - // reading `m.author.id` threw "Cannot read properties of undefined - // (reading 'id')". Iterate `.values()` to get the Messages themselves. - const collection = await ( - channel as unknown as { - messages: { fetch: (o: { limit: number }) => Promise<{ values(): Iterable }> }; - } - ).messages.fetch({ limit }); - const out: DiscordMessage[] = []; - for (const m of collection.values()) { - out.push({ - id: m.id, - channelId: m.channelId, - authorId: m.author.id, - authorName: m.author.username, - content: m.content, - mentionsBot: this.resolvedBotUserId ? m.mentions.users.has(this.resolvedBotUserId) : false, - }); - } - return out; - } - - private async requireTextChannel(channelId: string): Promise { - const channel = await this.client.channels.fetch(channelId); - if (!channel?.isTextBased() || !("send" in channel)) { - throw new Error(`channel ${channelId} not text-based or not fetchable`); - } - return channel; + const raw = (await this.client.rest.get(Routes.channelMessages(channelId), { + query: new URLSearchParams({ limit: String(limit) }), + })) as RawApiMessage[]; + return raw.map((m) => ({ + id: m.id, + channelId: m.channel_id, + authorId: m.author.id, + authorName: m.author.username, + content: m.content, + mentionsBot: this.resolvedBotUserId + ? (m.mentions ?? []).some((u) => u.id === this.resolvedBotUserId) + : false, + })); } } diff --git a/packages/discord/test/client.test.ts b/packages/discord/test/client.test.ts index 70ad9fe..33c9c74 100644 --- a/packages/discord/test/client.test.ts +++ b/packages/discord/test/client.test.ts @@ -28,43 +28,87 @@ describe("DiscordJsClient", () => { }); }); -describe("fetchRecent", () => { - // discord.js `messages.fetch` resolves a Collection (a Map subclass) keyed by - // snowflake. The bug iterated the Collection directly (`for…of`), yielding - // [key, Message] PAIRS, so `m.author.id` threw. This reproduces that shape - // with a plain Map and asserts we iterate `.values()`. - function fakeMessage(id: string, authorId: string, content: string) { - return { - id, - channelId: "chan-1", - author: { id: authorId, username: `u-${authorId}` }, - content, - mentions: { users: new Map([["999", {}]]) }, - }; +describe("outbound via REST (no gateway / login)", () => { + interface RestCall { + method: string; + route: string; + options?: { body?: Record; query?: URLSearchParams }; + } + // Stub the underlying discord.js client's REST manager (the only thing + // outbound touches now — no channels.fetch, no login). GET returns raw API + // messages (snake_case + a mentions ARRAY, as the real REST endpoint does). + function stubRest(client: DiscordJsClient): RestCall[] { + const calls: RestCall[] = []; + const rec = + (method: string) => + async (route: string, options?: RestCall["options"]): Promise => { + calls.push({ method, route, options }); + if (method !== "get") return undefined; + return [ + { + id: "m1", + channel_id: "chan-1", + author: { id: "111", username: "alice" }, + content: "hi", + mentions: [{ id: "999" }], + }, + { + id: "m2", + channel_id: "chan-1", + author: { id: "222", username: "bobby" }, + content: "yo", + mentions: [], + }, + ]; + }; + // biome-ignore lint/suspicious/noExplicitAny: test stub for a private field + (client as any).client = { rest: { post: rec("post"), put: rec("put"), get: rec("get") } }; + return calls; } - it("maps a Collection of messages, iterating values not [key,value] entries", async () => { + it("reply POSTs to the channel-messages route with content + reply reference", async () => { const client = new DiscordJsClient({ token: "fake", botUserId: "999" }); - const collection = new Map([ - ["m1", fakeMessage("m1", "111", "hello")], - ["m2", fakeMessage("m2", "999", "self")], - ]); - // Stub the underlying discord.js client so requireTextChannel + fetch resolve. - // biome-ignore lint/suspicious/noExplicitAny: test stub for a private field - (client as any).client = { - channels: { - fetch: async () => ({ - isTextBased: () => true, - send: () => undefined, - messages: { fetch: async () => collection }, - }), - }, - }; + const calls = stubRest(client); + await client.reply("chan-1", "hello", { replyTo: "m9" }); + expect(calls).toHaveLength(1); + expect(calls[0]?.method).toBe("post"); + expect(calls[0]?.route).toContain("chan-1"); + expect(calls[0]?.options?.body?.content).toBe("hello"); + expect((calls[0]?.options?.body?.message_reference as { message_id: string })?.message_id).toBe( + "m9", + ); + }); + it("reply omits the reference when there's no replyTo", async () => { + const client = new DiscordJsClient({ token: "fake", botUserId: "999" }); + const calls = stubRest(client); + await client.reply("chan-1", "plain"); + expect(calls[0]?.options?.body?.message_reference).toBeUndefined(); + }); + + it("react PUTs the own-reaction route with the URL-encoded emoji", async () => { + const client = new DiscordJsClient({ token: "fake", botUserId: "999" }); + const calls = stubRest(client); + await client.react("chan-1", "m9", "✅"); + expect(calls[0]?.method).toBe("put"); + expect(calls[0]?.route).toContain("chan-1"); + expect(calls[0]?.route).toContain(encodeURIComponent("✅")); + }); + + it("fetchRecent maps raw API messages (snake_case + mentions array → mentionsBot)", async () => { + const client = new DiscordJsClient({ token: "fake", botUserId: "999" }); + stubRest(client); const out = await client.fetchRecent("chan-1", 10); expect(out.map((m) => m.id)).toEqual(["m1", "m2"]); - expect(out[0]?.authorId).toBe("111"); - expect(out[1]?.mentionsBot).toBe(true); // mentions.users carries botUserId 999 + expect(out[0]).toEqual({ + id: "m1", + channelId: "chan-1", + authorId: "111", + authorName: "alice", + content: "hi", + mentionsBot: true, // botUserId 999 is in m1's mentions + }); + expect(out[1]?.mentionsBot).toBe(false); // 999 not in m2's mentions }); }); diff --git a/packages/shell/src/persistent.ts b/packages/shell/src/persistent.ts index 3321d61..d0b0794 100644 --- a/packages/shell/src/persistent.ts +++ b/packages/shell/src/persistent.ts @@ -97,6 +97,11 @@ export async function startPersistent(opts: RunPersistentOptions): Promise; + // True only for the PERSISTENT runtime (`bob serve`/runPersistent). Surfaced + // to capabilities via BOB_PERSISTENT so "serving" capabilities (e.g. discord's + // inbound gateway listener) only open their connection persistently — a + // one-shot `bob run` stays minimal (outbound tools, no gateway). Defaults + // falsy (ephemeral run). + persistent?: boolean; } // The injectable seam. Production builds a real pi AgentSession; tests inject a @@ -332,6 +338,10 @@ export async function createPiRunSession( for (const [key, value] of Object.entries(config.capabilityEnv)) { process.env[key] = value; } + // Runtime-mode signal for "serving" capabilities (discord's inbound gateway): + // set BEFORE the extensions load. "1" only for the persistent runtime; a + // one-shot run clears it so capabilities stay outbound-only. + process.env.BOB_PERSISTENT = config.persistent ? "1" : ""; const resourceLoader = new DefaultResourceLoader(loaderOpts); await resourceLoader.reload();