Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions slack-bridge/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export SLACK_APP_TOKEN="xapp-..."

Settings in `settings.json` override environment variables.

Only the broker (and single-player mode) needs Slack tokens. A worker that joins a running broker with `/pinet follow` can run without them; it makes its Slack calls through the broker. Downloading Slack files is the exception and still needs a local bot token.

## Control who can use Pinet

Slack access is default-deny. Configure one of these:
Expand Down
27 changes: 23 additions & 4 deletions slack-bridge/broker/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1372,10 +1372,10 @@ describe("BrokerClient — adapter capabilities", () => {
expect(req.params.params.method).toBe("conversations.history");
expect((req.params.params.params as Record<string, unknown>).channel).toBe("C123");

mock.respondTo(mock.connections[0], req.id, { messages: [] });
mock.respondTo(mock.connections[0], req.id, { ok: true, messages: [] });

const result = await proxyPromise;
expect(result).toEqual({ messages: [] });
expect(result).toEqual({ ok: true, messages: [] });

client.disconnect();
});
Expand Down Expand Up @@ -1409,13 +1409,32 @@ describe("BrokerClient — adapter capabilities", () => {
expect(fallbackReq.params.method).toBe("conversations.history");
expect(fallbackReq.params.params.channel).toBe("C123");

mock.respondTo(mock.connections[0], fallbackReq.id, { messages: [] });
mock.respondTo(mock.connections[0], fallbackReq.id, { ok: true, messages: [] });

const result = await proxyPromise;
expect(result).toEqual({ messages: [] });
expect(result).toEqual({ ok: true, messages: [] });

client.disconnect();
});

it.each([null, {}, { ok: "true" }, { ok: false, error: 42 }])(
"rejects malformed Slack proxy response %j",
async (response) => {
const client = new BrokerClient(mock.connectOpts);
await client.connect();
try {
const result = expect(client.slackProxy("reactions.add", {})).rejects.toThrow(
"Invalid Slack proxy response",
);
await waitFor(() => mock.received.length > 0);
const request = JSON.parse(mock.received[0]);
mock.respondTo(mock.connections[0], request.id, response);
await result;
} finally {
client.disconnect();
}
},
);
});

describe("BrokerClient — error handling", () => {
Expand Down
33 changes: 21 additions & 12 deletions slack-bridge/broker/client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as net from "node:net";
import type { SlackResult } from "../slack-api.js";
import { computeBackoffDelay, withTimeout } from "@pinet/transport-core/async";
import { readMeshSecret } from "./auth.js";
import { DEFAULT_SOCKET_PATH as PINET_DEFAULT_SOCKET_PATH } from "./paths.js";
Expand Down Expand Up @@ -717,19 +718,27 @@ export class BrokerClient {
* The broker RPC boundary remains adapter-neutral; Slack-specific payloads are
* interpreted inside the Slack adapter's api.call capability.
*/
async slackProxy(
method: string,
params: Record<string, unknown>,
): Promise<Record<string, unknown>> {
try {
return await this.invokeAdapterCapability("slack", "api.call", { method, params });
} catch (err) {
if (!isRpcMethodNotFoundError(err, "adapter.capability")) {
throw err;
}
async slackProxy(method: string, params: Record<string, unknown>): Promise<SlackResult> {
const result = await this.invokeAdapterCapability("slack", "api.call", {
method,
params,
}).catch((err) => {
if (!isRpcMethodNotFoundError(err, "adapter.capability")) throw err;
return this.request("slack.proxy", { method, params });
});
if (
typeof result !== "object" ||
result === null ||
!("ok" in result) ||
typeof result.ok !== "boolean"
) {
throw new Error("Invalid Slack proxy response: expected a boolean ok field.");
}

return (await this.request("slack.proxy", { method, params })) as Record<string, unknown>;
const error = "error" in result ? result.error : undefined;
if (error !== undefined && typeof error !== "string") {
throw new Error("Invalid Slack proxy response: expected a string error field.");
}
return { ...result, ok: result.ok, error };
}

// ─── Events ──────────────────────────────────────────
Expand Down
35 changes: 21 additions & 14 deletions slack-bridge/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ import { consumePinetReadConfirmationReplies } from "./pinet-confirmation-replie
import { createAgentPromptGuidance } from "./agent-prompt-guidance.js";
import { createAgentEventRuntime } from "./agent-event-runtime.js";
import { createSessionUiRuntime } from "./session-ui-runtime.js";
import { createSlackRequestRuntime } from "./slack-request-runtime.js";
import { createSlackRequestRuntime, type SlackRequestCall } from "./slack-request-runtime.js";
import { getSlackMessageAgeMs, isStaleSlackMessage } from "./stale-slack-messages.js";
import {
formatSlackAgentsDashboard,
Expand Down Expand Up @@ -143,11 +143,17 @@ export default function (pi: ExtensionAPI) {

let botToken = settings.botToken ?? process.env.SLACK_BOT_TOKEN;
let appToken = settings.appToken ?? process.env.SLACK_APP_TOKEN;

if (!botToken || !appToken) return;
// Followers need no Slack tokens; only the broker and single-player mode talk to the Slack app.
const hasSlackTokens = () => Boolean(botToken && appToken);

const slackRequestRuntime = createSlackRequestRuntime();
const { slack } = slackRequestRuntime;
// An empty token means "no local Slack app": a follower then calls Slack through the broker.
const slack: SlackRequestCall = async (method, token, body) => {
if (token) return slackRequestRuntime.slack(method, token, body);
if (brokerRole === "follower" && brokerClient)
return brokerClient.client.slackProxy(method, body ?? {});
throw new Error("Slack bot token is not configured.");
};

// allowedUsers / allowAllWorkspaceUsers: settings.json takes priority, env vars as fallback
let allowedUsers = buildAllowlist(
Expand Down Expand Up @@ -455,13 +461,13 @@ export default function (pi: ExtensionAPI) {
let isSinglePlayerConnected = () => false;
const slackThreadStatuses = new SlackThreadStatusManager({
slack,
getBotToken: () => botToken!,
getBotToken: () => botToken ?? "",
formatError: msg,
logger: console,
});
const slackRuntimeAccess = createSlackRuntimeAccess({
slack,
getBotToken: () => botToken!,
getBotToken: () => botToken ?? "",
userNames,
channelCache,
persistState,
Expand Down Expand Up @@ -1217,6 +1223,11 @@ export default function (pi: ExtensionAPI) {
): Promise<void> {
return runPinetLifecycle(async () => {
signal?.throwIfAborted();
if (mode !== "off" && mode !== "follower" && !hasSlackTokens()) {
throw new Error(
`Pinet ${mode} mode needs SLACK_BOT_TOKEN and SLACK_APP_TOKEN. Without them, join a running broker with /pinet follow.`,
);
}
if (currentRuntimeMode === mode) {
if (mode === "off") {
setExtStatus(ctx, "off");
Expand Down Expand Up @@ -1327,7 +1338,7 @@ export default function (pi: ExtensionAPI) {
const reloadWork = runPinetLifecycle(async () => {
let reloadBrokerInPlace = false;
const validateRefreshedState = () => {
if (!botToken || !appToken) {
if (brokerRole !== "follower" && !hasSlackTokens()) {
throw new Error("Slack tokens are not configured after reload.");
}
if (brokerRole === "broker") {
Expand Down Expand Up @@ -1405,12 +1416,7 @@ export default function (pi: ExtensionAPI) {
const toolRegistrationRuntime = createToolRegistrationRuntime({
buildPromptGuidelines: agentPromptGuidance.buildPromptGuidelines,
slackTools: {
getBotToken: () => {
if (!botToken) {
throw new Error("Slack bot token is not configured.");
}
return botToken;
},
getBotToken: () => botToken ?? "",
getDefaultChannel: () => settings.defaultChannel,
getSecurityPrompt: () => securityPrompt,
inbox,
Expand Down Expand Up @@ -1905,10 +1911,11 @@ export default function (pi: ExtensionAPI) {

refreshSettings();
maybeWarnSlackUserAccess(ctx);
const startupMode = resolveSlackBridgeStartupRuntimeMode(settings, {
const configuredMode = resolveSlackBridgeStartupRuntimeMode(settings, {
brokerSocketExists: fs.existsSync(resolveBrokerSocketPath()),
brokerManagedFollowerLaunch: isBrokerManagedFollowerLaunch(),
});
const startupMode = configuredMode === "follower" || hasSlackTokens() ? configuredMode : "off";

if (startupMode === "off") {
await transitionToRuntimeMode(ctx, startupMode);
Expand Down
10 changes: 10 additions & 0 deletions slack-bridge/slack-file-access.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@ describe("fetchSlackFileToCache", () => {
expect(fetchImpl).not.toHaveBeenCalled();
});

it("refuses to download without a bot token instead of fetching a login page", async () => {
const slack = vi.fn();
const fetchImpl = vi.fn();

await expect(
fetchSlackFileToCache("F123", {}, { slack, token: "", fetchImpl }),
).rejects.toThrow("needs a local Slack bot token");
expect(fetchImpl).not.toHaveBeenCalled();
});

it("rejects files above the configured safe size limit before downloading", async () => {
const cacheDir = await makeTempDir();
const slack = vi.fn(
Expand Down
4 changes: 4 additions & 0 deletions slack-bridge/slack-file-access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ export async function fetchSlackFileToCache(
if (!trimmedFileId) {
throw new Error("file_id is required.");
}
// Private file URLs need the bot token itself; the broker's Slack proxy cannot fetch bytes.
if (!deps.token) {
throw new Error("Downloading Slack files needs a local Slack bot token; this worker has none.");
}

const fetchImpl = deps.fetchImpl ?? fetch;
const now = deps.now?.() ?? new Date();
Expand Down
124 changes: 124 additions & 0 deletions slack-bridge/token-free-follower.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { afterEach, expect, it, vi } from "vitest";
import type {
CommandDefinition,
ExtensionAPI,
ExtensionContext,
ToolDefinition,
} from "@earendil-works/pi-coding-agent";
import extension from "./index.js";
import { BrokerDB } from "./broker/schema.js";
import { BrokerSocketServer } from "./broker/socket-server.js";

afterEach(() => {
vi.unstubAllEnvs();
vi.restoreAllMocks();
});

type Handler = (event: object, ctx: ExtensionContext) => Promise<void>;

it("joins a real broker without Slack tokens and calls Slack through the broker", async () => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "pinet-token-free-"));
const socket = path.join(root, "broker.sock");
fs.mkdirSync(path.join(root, ".pi", "agent"), { recursive: true });
// runtimeMode "broker" mirrors a shared settings.json; without tokens it must not start a broker.
fs.writeFileSync(
path.join(root, ".pi", "agent", "settings.json"),
JSON.stringify({ "slack-bridge": { runtimeMode: "broker", allowAllWorkspaceUsers: true } }),
);
vi.stubEnv("HOME", root);
vi.stubEnv("SLACK_BOT_TOKEN", undefined);
vi.stubEnv("SLACK_APP_TOKEN", undefined);
vi.stubEnv("PINET_SOCKET_PATH", socket);
vi.stubEnv("PINET_MESH_SECRET", "test-mesh-secret");

const db = new BrokerDB(":memory:");
db.initialize();
const server = new BrokerSocketServer(db, socket, { meshSecret: "test-mesh-secret" });
const slackCalls: object[] = [];
server.setOutboundMessageAdapters([
{
name: "slack",
send: vi.fn(),
invokeCapability: async ({ params }) => {
slackCalls.push(params);
return { result: { ok: true } };
},
},
]);
await server.start();

const events = new Map<string, Handler>();
const commands = new Map<string, CommandDefinition>();
const tools = new Map<string, ToolDefinition>();
const pi = {
registerCommand: (name: string, definition: CommandDefinition) =>
commands.set(name, definition),
registerTool: (definition: ToolDefinition) => tools.set(definition.name, definition),
registerMessageRenderer: vi.fn(),
on: (name: string, fn: Handler) => events.set(name, fn),
appendEntry: vi.fn(),
sendMessage: vi.fn(),
sendUserMessage: vi.fn(),
getActiveTools: () => ["read", "slack", "pinet"],
setActiveTools: vi.fn(),
getSessionName: () => undefined,
} as ExtensionAPI;
const notify = vi.fn();
const context = {
cwd: root,
hasUI: true,
isIdle: () => true,
ui: {
notify,
onTerminalInput: vi.fn(),
setStatus: vi.fn(),
select: vi.fn(),
custom: vi.fn(),
theme: { fg: (_color: string, text: string) => text },
},
sessionManager: {
getEntries: () => [],
getBranch: () => [],
getLeafId: () => "token-free-test",
getSessionFile: () => path.join(root, "session.jsonl"),
},
} as ExtensionContext;

try {
extension(pi);
expect(commands.has("pinet")).toBe(true);
await events.get("session_start")!({}, context);
expect(db.getAgents()).toHaveLength(0);

await commands.get("pinet")!.handler("follow", context);
await expect.poll(() => db.getAgents().length, { timeout: 10_000 }).toBe(1);

await tools.get("slack")!.execute!(
"call-1",
{
action: "react",
args: { emoji: "eyes", timestamp: "1712345678.000100", channel: "C0TEST" },
},
undefined,
undefined,
context,
);
expect(slackCalls).toContainEqual({
method: "reactions.add",
params: { channel: "C0TEST", timestamp: "1712345678.000100", name: "eyes" },
});

// Asking a token-less follower to become broker fails before it leaves the mesh.
await commands.get("pinet")!.handler("start", context);
expect(notify).toHaveBeenCalledWith(expect.stringContaining("needs SLACK_BOT_TOKEN"), "error");
expect(db.getAgents()[0].disconnectedAt).toBeFalsy();
} finally {
await events.get("session_shutdown")?.({}, context);
await server.stop();
db.close();
fs.rmSync(root, { force: true, recursive: true });
}
}, 20_000);
Loading