Skip to content

feat(channel): add Slack and localize channel plugins - #5

Open
mackt wants to merge 8 commits into
mainfrom
feat/channel-i18n
Open

feat(channel): add Slack and localize channel plugins#5
mackt wants to merge 8 commits into
mainfrom
feat/channel-i18n

Conversation

@mackt

@mackt mackt commented Sep 7, 2026

Copy link
Copy Markdown
Member

Feishu, Slack, and Telegram currently expose fixed-language configuration, status, authentication, and command text. Register plugin-owned catalogs for en, es, ja, ko, zh-CN, and zh-TW and return deferred SDK messages across these surfaces. Direct platform notices translate at delivery; raw error diagnostics remain separate.

Slack inbound attachments are limited to 100 MiB per file and 60 seconds per download. Size metadata and Content-Length provide early rejection, streaming byte counts enforce the limit even when metadata is missing or incorrect, and cancellation removes partial files. Completed downloads are also cleaned up when delivery is cancelled or fails before the host accepts them. A token-only configuration can connect to Slack and return setup IDs; delivery remains blocked until the sender or channel is allowlisted. Update the configuration form, six locale catalogs, and setup guide accordingly.

Slack startup checks cancellation before and after authentication and connection, preventing a late socket from surviving shutdown. Download filenames stay within filesystem limits while retaining their extensions; valid HTML attachments are accepted while login pages remain rejected. Explicit DM threads use separate sessions, while unthreaded DM session keys stay unchanged.

Validate catalogs before publication, embed only name/description translations in the store registry, and include every registered locale file in release archives. Registry generation and release staging also compare source pluginMessage fallbacks with every catalog, so a placeholder renamed only in code fails publication even when the language files agree with each other. Document the literal-key/fallback convention and support SDK import aliases and namespace imports without executing plugin code. Resolve calls by lexical binding so parameters, local variables, catch bindings, and hoisted declarations can safely shadow SDK import names.

Includes the three existing, unmerged Slack implementation/documentation commits from the closed PR #3 because that remote base branch no longer exists. Plugin localization follows in separate commits. StackChan work is excluded.

Companion SDK/host change: https://github.com/marswaveai/cola/pull/5182

Validation:

  • 111 targeted tests passed across 17 files for Feishu, Slack, Telegram, and publication tooling, including shutdown races, attachment cleanup/limits/filenames/HTML handling, DM thread isolation, allowlist setup, and lexical binding validation.
  • Slack type check and build passed against the local SDK 0.0.5 from the companion Cola worktree.
  • All three plugins passed source/catalog validation; changed-file lint, formatting, and diff checks passed.

Post-merge release steps:

  • Build and publish SDK 0.0.5 after merge, then refresh the lockfile from the actual registry artifact to record its integrity. The lockfile now targets the 0.0.5 registry tarball; the old local candidate checksum has been removed. Packaging and publication are deferred until then; clean registry installs/CI currently fail because SDK 0.0.5 is unpublished.
  • Before publishing the channel packages, replace the temporary minColaVersion 1.99.0 with the actual first Cola release supporting i18n.
  • Real platform authentication was not exercised.

@mackt
mackt marked this pull request as ready for review September 7, 2026 14:29
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T09:42:16.728103Z a7ec9be Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93c2d4aefb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/slack/src/gateway.ts
Comment thread pnpm-lock.yaml Outdated
Comment thread plugins/slack/package.json
Comment thread plugins/slack/src/media.ts
Comment thread plugins/slack/src/config.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e669283a5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/slack/src/gateway.ts Outdated
Comment thread plugins/slack/src/media.ts
Comment thread plugins/slack/src/message.ts
Comment thread plugins/slack/src/outbound.ts
Comment thread scripts/plugin-message-validation.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa8c4c65d5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/slack/src/gateway.ts Outdated
Comment thread plugins/slack/src/media.ts
Comment thread plugins/slack/src/media.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7ec9bedc1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/slack/src/gateway.ts
@mackt

mackt commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7ec9bedc1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +219 to +223
for (const file of event.files ?? []) {
if (ctx.abortSignal.aborted) return;
const filePath = await downloadSlackFile(file, config.botToken, ctx.logger, {
signal: ctx.abortSignal,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Cap aggregate Slack attachment downloads

When one allowed Slack message contains multiple attachments, the new 100 MiB limit is reset for every downloadSlackFile call while all completed files remain on disk until the entire loop finishes. A message with N individually valid files can therefore consume N × 100 MiB and keep the handler busy for N × 60 seconds, so the per-file fix still permits substantial disk exhaustion; enforce a total byte/file-count or whole-message timeout and clean up once that aggregate limit is reached.

Useful? React with 👍 / 👎.

Comment on lines +81 to +83
const handle = async ({ event, ack }: SlackEventArgs) => {
await ack();
if (ctx.abortSignal.aborted) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Catch Slack acknowledgement failures

When the socket closes while an event is being acknowledged, or its WebSocket send otherwise rejects, ack() fails before execution enters this handler's try block. The Socket Mode client dispatches these async listeners through an event emitter without awaiting their returned promises, so this becomes an unhandled rejection instead of the logged handler failure and can terminate the Node host; include acknowledgement in the error boundary.

Useful? React with 👍 / 👎.

Comment on lines +194 to +198
const isDm = isDirectMessage(event);
const allowed = isDm
? (event.user !== undefined && config.allowedIds.has(event.user)) ||
config.allowedIds.has(event.channel)
: config.allowedIds.has(event.channel);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Filter unaddressed channel events before processing

When a channel ID is allowlisted, every message.channels or message.groups event passes this check regardless of whether the bot was mentioned. An ordinary top-level channel post therefore reaches the attachment-download and identity-binding path before the host can reject the eventual payload with mentionedBot: false, violating the documented mention-only behavior and making unrelated channel files consume the plugin's disk and network; reject unaddressed channel events before these side effects while preserving any explicitly supported active-thread replies.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant