Skip to content

Add DM-only mode for local Slack - #93

Open
KishanBagaria wants to merge 22 commits into
mainfrom
kb/dm-only
Open

Add DM-only mode for local Slack#93
KishanBagaria wants to merge 22 commits into
mainfrom
kb/dm-only

Conversation

@KishanBagaria

Copy link
Copy Markdown

Depends on beeper/slackgo#4

Copilot AI lite review requested due to automatic review settings August 7, 2026 17:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a “DM-only” operating mode for the Slack connector, intended to restrict discovery/sync/event handling to direct messages (IMs) and group DMs (MPIMs), while omitting channel data where possible.

Changes:

  • Introduces dm_only configuration and config upgrader support.
  • Uses Slack client boot options to omit channel data when dm_only is enabled.
  • Filters incoming Slack events and adjusts channel sync logic to ignore non-DM conversations in DM-only mode.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/connector/startchat.go Blocks channel creation when dm_only is enabled.
pkg/connector/login-cookie.go Uses boot options to omit channels during cookie/token login when dm_only is enabled.
pkg/connector/handleslack.go Filters events to DM channels only; adds helper to extract channel ID from events.
pkg/connector/example-config.yaml Documents the new dm_only config option.
pkg/connector/config.go Adds DMOnly to config struct and config upgrade path.
pkg/connector/client.go Tracks DM channel IDs; boots and syncs differently under dm_only.
pkg/connector/capabilities.go Hides public/private channel creation capability when dm_only is enabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/connector/handleslack.go
Comment thread pkg/connector/client.go Outdated
for _, ch := range s.BootResp.Channels {
ch.IsMember = true
channels = append(channels, &ch.Channel)
if s.Main.Config.DMOnly || (s.IsRealUser && (strings.HasPrefix(token, "xoxs-") || s.Main.Config.Backfill.ConversationCount == -1)) {
Comment thread pkg/connector/client.go Outdated
Comment thread pkg/connector/client.go Outdated
Comment thread pkg/connector/client.go Outdated
Comment thread pkg/connector/login-cookie.go Outdated
Comment thread pkg/connector/client.go
}
// TODO do actual warm boots by saving last received ts somewhere
bootResp, err = s.Client.ClientUserBootContext(ctx, time.Time{})
bootResp, err = s.Client.ClientUserBootContext(ctx, time.Time{}, s.Main.Config.DMOnly)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slack is actually ignoring omit_channels and still returning it. keeping it in case bot logins are different

Comment thread pkg/connector/chatinfo.go
pages := 0
output = make(map[networkid.UserID]bridgev2.ChatMember)
for limit > 0 {
chunkLimit := limit

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug in main - chunkLimit was unused

Comment thread pkg/connector/client.go
const maxSlackPaginationPages = 1000

func slackPageLimit(unlimited bool, remaining int) int {
const maxPage = 200

@KishanBagaria KishanBagaria Aug 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tulir any reason to do 200 max here? i tested users.conversations accepts limit=999 (it returned my 828 DMs/MPIMs in 1 req)

this will reduce users.conversations from 5 requests to 1 for me

@KishanBagaria

Copy link
Copy Markdown
Author

@tulir good for re-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants