Skip to content
Open
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
76 changes: 76 additions & 0 deletions .v0/plans/add-is-external-channel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Add `isExternalChannel` support to chat SDK

## Background

Slack sends `is_ext_shared_channel: boolean` on event callback payloads. When the bot is in an external/shared channel (Slack Connect), it may be leaking internal context (repo summaries, etc.) to outsiders. We need to surface this at the `Thread` level so consumers can gate behavior.

## API Design Decision: `isExternalChannel` (boolean) vs `visibility` (enum)

**Recommendation: `isExternalChannel: boolean`**

Reasons:
- **Matches the source data**: Slack (the only platform with this concept today) sends a simple `is_ext_shared_channel` boolean. No need to over-abstract.
- **Follows the `isDM` pattern**: The codebase already uses `isDM: boolean` on Thread/Postable with the exact same architecture (adapter method -> Chat.createThread -> ThreadImpl property). Adding `isExternalChannel` is perfectly consistent.
- **Discord has no equivalent**: Discord doesn't support cross-server channel sharing at all.
- **Teams**: Teams has "shared channels" (`membershipType: "shared"`) but that's a different Teams-specific concept. It could map to `isExternalChannel = true` in the future.
- **Google Chat**: GChat has external spaces but it's not exposed in webhook payloads -- would need a separate API call. Can be added later.
- **GitHub/Linear**: No concept of external channels.
- A `visibility` enum (`"private" | "external" | "public"`) conflates two orthogonal concerns -- DM/private vs external. A channel can be both public within a workspace AND externally shared. Keeping them separate (`isDM` + `isExternalChannel`) is cleaner.

## Implementation Plan

### 1. Core types (`packages/chat/src/types.ts`)

- Add `isExternalChannel?(threadId: string): boolean` to the `Adapter` interface (optional, like `isDM?`)
- Add `readonly isExternalChannel: boolean` to the `Postable` interface (alongside `isDM`)
- Add `isExternalChannel?: boolean` to `ThreadInfo` interface
- Add `isExternalChannel?: boolean` to `ChannelInfo` interface

### 2. Thread implementation (`packages/chat/src/thread.ts`)

- Add `isExternalChannel?: boolean` to `SerializedThread`
- Add `isExternalChannel?: boolean` to both `ThreadImplConfigWithAdapter` and `ThreadImplConfigLazy`
- Add `readonly isExternalChannel: boolean` property to `ThreadImpl` class
- Set it in constructor: `this.isExternalChannel = config.isExternalChannel ?? false`
- Include it in `toJSON()` and `fromJSON()` serialization

### 3. Channel implementation (`packages/chat/src/channel.ts`)

- Add `isExternalChannel` to `ChannelImpl` (same pattern as `isDM`)

### 4. Chat class (`packages/chat/src/chat.ts`)

- In `createThread()`, call `adapter.isExternalChannel?.(threadId) ?? false` and pass to ThreadImpl constructor (same pattern as `isDM`)

### 5. Slack adapter (`packages/adapter-slack/src/index.ts`)

- Add `is_ext_shared_channel?: boolean` to the `SlackWebhookPayload` interface
- Cache `is_ext_shared_channel` per channel ID in a `Set<string>` from incoming payloads
- Also read `is_ext_shared` from `conversations.info` API responses in `fetchThread` and `fetchChannelInfo`
- Implement `isExternalChannel(threadId)` that checks the cache

### 6. Other adapters (Discord, Teams, GChat, GitHub, Linear)

- `isExternalChannel` is optional on the `Adapter` interface, so these don't need explicit stubs
- The `Chat.createThread()` fallback (`adapter.isExternalChannel?.(threadId) ?? false`) handles the default

### 7. Mock adapter (`packages/chat/src/mock-adapter.ts`)

- Add `isExternalChannel` mock returning `false` (same pattern as `isDM`)

### 8. Tests

- Serialization round-trip tests for `isExternalChannel` on Thread
- Channel inheritance tests (`thread.channel.isExternalChannel`)
- Backward compatibility test (missing `isExternalChannel` in JSON defaults to `false`)

## Key Files Modified

1. `packages/chat/src/types.ts` - Core interfaces
2. `packages/chat/src/thread.ts` - ThreadImpl
3. `packages/chat/src/channel.ts` - ChannelImpl
4. `packages/chat/src/chat.ts` - Thread creation
5. `packages/chat/src/mock-adapter.ts` - Test mock
6. `packages/adapter-slack/src/index.ts` - Slack implementation (main one)
7. `packages/chat/src/serialization.test.ts` - Serialization tests
8. `packages/chat/src/channel.test.ts` - Channel tests
52 changes: 51 additions & 1 deletion packages/adapter-slack/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ interface SlackWebhookPayload {
| SlackAppHomeOpenedEvent;
event_id?: string;
event_time?: number;
/** Whether this event occurred in an externally shared channel (Slack Connect) */
is_ext_shared_channel?: boolean;
team_id?: string;
type: string;
}
Expand Down Expand Up @@ -300,6 +302,12 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
private readonly formatConverter = new SlackFormatConverter();
private static USER_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hour

/**
* Cache of channel IDs known to be external/shared (Slack Connect).
* Populated from `is_ext_shared_channel` in incoming webhook payloads.
*/
private readonly _externalChannels = new Set<string>();

// Multi-workspace support
private readonly clientId: string | undefined;
private readonly clientSecret: string | undefined;
Expand All @@ -308,6 +316,7 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
private readonly requestContext = new AsyncLocalStorage<{
token: string;
botUserId?: string;
isExtSharedChannel?: boolean;
}>();

/** Bot user ID (e.g., U_BOT_123) used for mention detection */
Expand Down Expand Up @@ -742,6 +751,19 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
if (payload.type === "event_callback" && payload.event) {
const event = payload.event;

// Track external/shared channel status from payload-level flag
if (payload.is_ext_shared_channel) {
let channelId: string | undefined;
if ("channel" in event) {
channelId = (event as SlackEvent).channel;
} else if ("item" in event) {
channelId = (event as SlackReactionEvent).item.channel;
}
if (channelId) {
this._externalChannels.add(channelId);
}
}

if (event.type === "message" || event.type === "app_mention") {
const slackEvent = event as SlackEvent;
if (!(slackEvent.team || slackEvent.team_id) && payload.team_id) {
Expand Down Expand Up @@ -2373,7 +2395,17 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
const result = await this.client.conversations.info(
this.withToken({ channel })
);
const channelInfo = result.channel as { name?: string } | undefined;
const channelInfo = result.channel as
| {
name?: string;
is_ext_shared?: boolean;
}
| undefined;

// Update external channel cache from API response
if (channelInfo?.is_ext_shared) {
this._externalChannels.add(channel);
}

this.logger.debug("Slack API: conversations.info response", {
channelName: channelInfo?.name,
Expand All @@ -2384,6 +2416,7 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
id: threadId,
channelId: channel,
channelName: channelInfo?.name,
isExternalChannel: channelInfo?.is_ext_shared ?? false,
metadata: {
threadTs,
channel: result.channel,
Expand Down Expand Up @@ -2439,6 +2472,16 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
return channel.startsWith("D");
}

/**
* Check if a thread is in an external/shared channel (Slack Connect).
* Uses the `is_ext_shared_channel` flag from incoming webhook payloads,
* cached per channel ID.
*/
isExternalChannel(threadId: string): boolean {
const { channel } = this.decodeThreadId(threadId);
return this._externalChannels.has(channel);
}

decodeThreadId(threadId: string): SlackThreadId {
const parts = threadId.split(":");
if (parts.length < 2 || parts.length > 3 || parts[0] !== "slack") {
Expand Down Expand Up @@ -2750,15 +2793,22 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
name?: string;
is_im?: boolean;
is_mpim?: boolean;
is_ext_shared?: boolean;
num_members?: number;
purpose?: { value?: string };
topic?: { value?: string };
};

// Update external channel cache from API response
if (info?.is_ext_shared) {
this._externalChannels.add(channel);
}

return {
id: channelId,
name: info?.name ? `#${info.name}` : undefined,
isDM: Boolean(info?.is_im || info?.is_mpim),
isExternalChannel: info?.is_ext_shared ?? false,
memberCount: info?.num_members,
metadata: {
purpose: info?.purpose?.value,
Expand Down
29 changes: 29 additions & 0 deletions packages/chat/src/channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,35 @@ describe("thread.channel", () => {

expect(thread.channel.isDM).toBe(true);
});

it("should inherit isExternalChannel from thread", () => {
const mockAdapter = createMockAdapter();
const mockState = createMockState();

const thread = new ThreadImpl({
id: "slack:C123:1234.5678",
adapter: mockAdapter,
channelId: "C123",
stateAdapter: mockState,
isExternalChannel: true,
});

expect(thread.channel.isExternalChannel).toBe(true);
});

it("should default isExternalChannel to false", () => {
const mockAdapter = createMockAdapter();
const mockState = createMockState();

const thread = new ThreadImpl({
id: "slack:C123:1234.5678",
adapter: mockAdapter,
channelId: "C123",
stateAdapter: mockState,
});

expect(thread.channel.isExternalChannel).toBe(false);
});
});

describe("thread.messages (newest first)", () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/chat/src/channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface SerializedChannel {
adapterName: string;
id: string;
isDM: boolean;
isExternalChannel?: boolean;
}

/**
Expand All @@ -45,6 +46,7 @@ interface ChannelImplConfigWithAdapter {
adapter: Adapter;
id: string;
isDM?: boolean;
isExternalChannel?: boolean;
stateAdapter: StateAdapter;
}

Expand All @@ -55,6 +57,7 @@ interface ChannelImplConfigLazy {
adapterName: string;
id: string;
isDM?: boolean;
isExternalChannel?: boolean;
}

type ChannelImplConfig = ChannelImplConfigWithAdapter | ChannelImplConfigLazy;
Expand All @@ -79,6 +82,7 @@ export class ChannelImpl<TState = Record<string, unknown>>
{
readonly id: string;
readonly isDM: boolean;
readonly isExternalChannel: boolean;

private _adapter?: Adapter;
private readonly _adapterName?: string;
Expand All @@ -88,6 +92,7 @@ export class ChannelImpl<TState = Record<string, unknown>>
constructor(config: ChannelImplConfig) {
this.id = config.id;
this.isDM = config.isDM ?? false;
this.isExternalChannel = config.isExternalChannel ?? false;

if (isLazyConfig(config)) {
this._adapterName = config.adapterName;
Expand Down Expand Up @@ -333,6 +338,7 @@ export class ChannelImpl<TState = Record<string, unknown>>
id: this.id,
adapterName: this.adapter.name,
isDM: this.isDM,
...(this.isExternalChannel ? { isExternalChannel: true } : {}),
};
}

Expand All @@ -344,6 +350,7 @@ export class ChannelImpl<TState = Record<string, unknown>>
id: json.id,
adapterName: json.adapterName,
isDM: json.isDM,
isExternalChannel: json.isExternalChannel,
});
if (adapter) {
channel._adapter = adapter;
Expand Down
4 changes: 4 additions & 0 deletions packages/chat/src/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,9 @@ export class Chat<
// Check if this is a DM
const isDM = adapter.isDM?.(threadId) ?? false;

// Check if this is an external/shared channel (e.g., Slack Connect)
const isExternalChannel = adapter.isExternalChannel?.(threadId) ?? false;

return new ThreadImpl<TState>({
id: threadId,
adapter,
Expand All @@ -1584,6 +1587,7 @@ export class Chat<
initialMessage,
isSubscribedContext,
isDM,
isExternalChannel,
currentMessage: initialMessage,
streamingUpdateIntervalMs: this._streamingUpdateIntervalMs,
});
Expand Down
1 change: 1 addition & 0 deletions packages/chat/src/mock-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export function createMockAdapter(name = "slack"): Adapter {
isDM: vi
.fn()
.mockImplementation((threadId: string) => threadId.includes(":D")),
isExternalChannel: vi.fn().mockReturnValue(false),
openModal: vi.fn().mockResolvedValue({ viewId: "V123" }),
channelIdFromThreadId: vi
.fn()
Expand Down
66 changes: 66 additions & 0 deletions packages/chat/src/serialization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,41 @@ describe("Serialization", () => {
expect(json.isDM).toBe(true);
});

it("should serialize external channel thread correctly", () => {
const mockAdapter = createMockAdapter("slack");
const mockState = createMockState();

const thread = new ThreadImpl({
id: "slack:C123:1234.5678",
adapter: mockAdapter,
channelId: "C123",
stateAdapter: mockState,
isExternalChannel: true,
});

const json = thread.toJSON();

expect(json._type).toBe("chat:Thread");
expect(json.isExternalChannel).toBe(true);
});

it("should omit isExternalChannel when false", () => {
const mockAdapter = createMockAdapter("slack");
const mockState = createMockState();

const thread = new ThreadImpl({
id: "slack:C123:1234.5678",
adapter: mockAdapter,
channelId: "C123",
stateAdapter: mockState,
isExternalChannel: false,
});

const json = thread.toJSON();

expect(json.isExternalChannel).toBeUndefined();
});

it("should produce JSON-serializable output", () => {
const mockAdapter = createMockAdapter("teams");
const mockState = createMockState();
Expand Down Expand Up @@ -163,6 +198,37 @@ describe("Serialization", () => {
expect(restored.adapter.name).toBe(original.adapter.name);
});

it("should round-trip isExternalChannel correctly", () => {
const mockAdapter = createMockAdapter("slack");

const original = new ThreadImpl({
id: "slack:C123:1234.5678",
adapter: mockAdapter,
channelId: "C123",
stateAdapter: mockState,
isExternalChannel: true,
});

const json = original.toJSON();
const restored = ThreadImpl.fromJSON(json);

expect(restored.isExternalChannel).toBe(true);
});

it("should default isExternalChannel to false when missing from JSON", () => {
const json: SerializedThread = {
_type: "chat:Thread",
id: "slack:C123:1234.5678",
channelId: "C123",
isDM: false,
adapterName: "slack",
};

const thread = ThreadImpl.fromJSON(json);

expect(thread.isExternalChannel).toBe(false);
});

it("should serialize currentMessage", () => {
const mockAdapter = createMockAdapter("slack");
const currentMessage = createTestMessage("msg-1", "Hello", {
Expand Down
Loading