Skip to content

Maestro Relay 0.1.x → main: Slack provider, installer hardening, docs split#40

Merged
chr1syy merged 36 commits into
mainfrom
rc
May 11, 2026
Merged

Maestro Relay 0.1.x → main: Slack provider, installer hardening, docs split#40
chr1syy merged 36 commits into
mainfrom
rc

Conversation

@chr1syy
Copy link
Copy Markdown
Collaborator

@chr1syy chr1syy commented May 10, 2026

Summary

Brings everything that landed on rc since v0.1.3 to main. The headline changes:

  • Slack provider is now built-in alongside Discord (@slack/bolt-based, Socket Mode + ExpressReceiver). Set ENABLED_PROVIDERS=slack or discord,slack.
  • Queue hardening: agent responses now surface even when the underlying CLI exits non-zero (soft-failures are logged but the user still sees the reply). Internal errors stay in the relay logs instead of being mirrored back into chat.
  • Installer hardening (back-port from main + new fixes): RETURN/EXIT trap scope fixes, can_read_tty helper to silence /dev/tty noise under bash -c \"\$(curl …)\", quote-stripping for ENABLED_PROVIDERS, tarball-name realignment.
  • Docs split: provider-specific content moved out of the README into docs/discord.md, docs/slack.md, docs/voice.md, plus a deep-dive AGENTS-providers.md for adding new platforms.
  • DB migrations: composite PK on agent_channels (provider, channel_id), agent_threads → discord_agent_threads, new slack_agent_conversations. WAL checkpoint on graceful shutdown.

What's in this PR

Slack provider (PRs #38, follow-ups)

  • src/providers/slack/{adapter,messageCreate,channelsDb,conversationsDb,config}.ts
  • Slash commands: /health, /agents list|new|disconnect|readonly, /session new
  • slack_agent_conversations thread registry keyed on thread_ts
  • Unicode → Slack emoji-name mapping (⏳ → hourglass_flowing_sand, etc.)
  • Tests: src/__tests__/slack-{config,conversationsDb,react}.test.ts
  • docs/slack.md (app setup, scopes, env vars, Socket Mode vs webhook, slash commands, runtime behavior, troubleshooting)
  • Installer: MAESTRO_RELAY_MODULE=slack adds Slack-aware credential prompting and config_complete validation

Kernel + DB

  • src/core/queue.ts: show response on soft-failure, hide internal errors from chat (PR fix: avoid leaking internal errors and normalize installer provider parsing #39)
  • src/index.ts: WAL checkpoint + db.close() on graceful shutdown
  • src/core/db/migrations.ts: provider-aware agent_channels, discord_agent_threads, slack_agent_conversations
  • src/core/providers.ts: dynamic adapter loading by ENABLED_PROVIDERS

Installer

  • RELEASE_TARBALL global fixes EXIT-trap scope under set -u
  • can_read_tty replaces [ -r /dev/tty ] to silence non-interactive warnings
  • Drops the RETURN-trap leak in install_release
  • Strips surrounding quotes from ENABLED_PROVIDERS before downstream matching
  • Tarball renamed maestro-relay-${tag}.tar.gz to match new repo name

Docs

  • README slimmed to a Providers table + cross-links; voice/Discord/Slack details split out
  • docs/discord.md, docs/slack.md, docs/voice.md, AGENTS-providers.md (+ CLAUDE-providers.md symlink)
  • AGENTS-providers.md worked example switched from Slack (now built-in) to a hypothetical Teams provider

Slack provider polish (this branch)

  • console.errorlogger.error in commands/{agents,session}.ts so failures land in the same error log
  • /agents readonly aligns with Discord: drop the <agent-id> positional, read the bound agent from channelDb

Test plan

  • npm test — 189/189 passing locally
  • Fresh install with ENABLED_PROVIDERS=discord — verify legacy Discord flow unchanged
  • Fresh install with ENABLED_PROVIDERS=slack — verify slash commands register and /health responds
  • Multi-provider install (ENABLED_PROVIDERS=discord,slack) — verify both adapters start
  • Quoted .env value (ENABLED_PROVIDERS=\"slack\") — verify installer accepts it
  • Run installer non-interactively under bash -c \"\$(curl …)\" — verify no /dev/tty: No such device warnings
  • Graceful shutdown — verify WAL checkpoint and clean DB close
  • Slack reaction appears and is removed on response
  • Slack /agents readonly on|off works without an agent-id argument

Migration notes

  • All DISCORD_* env vars unchanged; ENABLED_PROVIDERS defaults to discord if unset.
  • agent_channels schema upgrades automatically; existing rows default to provider='discord'.
  • agent_threadsdiscord_agent_threads rename happens in the same migration.
  • maestro-discord and maestro-bridge binaries remain as aliases of maestro-relay.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added Slack provider: channel/session management, slash commands for agents and health, threaded message handling, emoji reactions, and channel creation/unarchive flows.
  • Bug Fixes / Improvements

    • Improved error reporting for reactions/responses, provider-aware config validation, and graceful shutdown with DB checkpoint/close.
  • Documentation

    • Installer and docs updated for multi-provider support and Slack setup/troubleshooting.
  • Chores

    • Bumped package version, added Slack runtime dependency, and DB migration for Slack registries.
  • Tests

    • New Slack-focused unit tests for config, naming, reactions, and DB behavior.

Review Change Stack

chr1syy and others added 30 commits May 3, 2026 13:06
Split the agent->Discord CLI into `send`, `notify`, and `status` verbs so the
surface can grow beyond the original single-purpose flag form. Verbs use
Node's built-in parseArgs and live in src/cli/verbs/, with shared HTTP and
exec helpers in src/cli/lib.ts. All three currently ride the existing
/api/send endpoint; no server changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surface previously-unused maestro-cli capabilities as Discord slash commands:

- /playbook list|show|run — list playbooks, show details, run with completion
  summary posted to the channel (service layer was already wired)
- /agents show <id> — embed of agent stats and recent activity
- /gist [description] [public] — publish the channel's agent transcript as
  a GitHub gist; returns the URL in-channel
- /notes synopsis|history — Director's Notes (AI synopsis with 2 min LLM
  timeout, or unified history feed across agents)
- /auto-run start <doc> — launch an Auto Run for the channel's agent;
  bare filenames are resolved against the agent's autoRunFolderPath and
  autocomplete reads .md files from that directory

Adds five service methods (showAgent, createGist, directorSynopsis,
directorHistory, startAutoRun) and supporting types. Introduces a
CommandModule type in index.ts so the command registry Map type-unifies
across builders with different shapes (subcommand-only vs options-only).
Updates the README slash-command table.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nts show

Discord rejects embed payloads with description > 4096 chars or field value
> 1024 chars. Long playbook descriptions, long document path lists, long
cwds, and accumulated recent-activity text could all trip these limits and
fail editReply with a validation error.

Adds src/utils/embed.ts with clampDescription and clampFieldValue helpers
(append a "\n…" marker when truncating) and applies them to:

- /playbook show — description and Documents field
- /agents show — Cwd, Stats, and Recent activity fields

Also adds focused tests:

- embed.test.ts — clamp limits and edge cases
- playbook-command.test.ts — list/show happy paths, oversize-input clamp,
  load-failure error path
- agents-command.test.ts — show happy path, oversize-cwd clamp, load-failure
  error path

Test count: 110 -> 122 (all green).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- cli/lib: add 5s timeout + single-settle guard to postToSendApi; new
  parsePort() helper that strictly validates --port (digits only, 1-65535)
- cli/verbs/{send,notify,status}: use parsePort()
- cli/verbs/status: split runMaestroCli execution from JSON.parse so each
  surfaces a distinct error
- utils/embed: add EMBED_TITLE_MAX (256) + clampTitle()
- commands/agents: clamp title and groupName in handleShow; bound channel
  name to Discord's 100-char limit; replace `as TextChannel` cast with
  isSendable() guard + friendly error reply
- commands/auto-run: fetch autoRunFolderPath via showAgent (listAgents
  doesn't return it); resolve any non-absolute doc path against the agent
  folder, not only bare filenames
- commands/gist: normalize non-Error rejection values before slicing
- commands/playbook: clamp title and agent name in handleShow

Tests:
- new cli-lib.test.ts: parsePort validation + postToSendApi timeout,
  ECONNREFUSED, invalid-JSON, and double-settle protection
- new auto-run-command.test.ts: bare filename, subdir path regression,
  absolute passthrough, showAgent failure, missing folder
- new gist-command.test.ts: success, Error throw, string throw, object
  throw, long-message truncation
- extend agents/playbook/embed tests with clamp + bound regressions

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- agents: drop the unsafe `as TextChannel` cast (and import) since
  isSendable() already narrows the channel type
- tests/auto-run: add callCount guards before reading mock.calls[0] in
  absolute-path / showAgent-fail / missing-folder tests so a regression
  produces a clean assertion failure rather than a TypeError
- tests/cli-lib: drop the port-1 ECONNREFUSED trick (unreliable on
  Windows). Bind a server to a random free port, close it immediately,
  then connect — the OS keeps the port reserved long enough to refuse
  predictably across platforms
- tests/gist: tighten truncation test with a lower-bound assertion and
  prefix check so over-truncation regressions don't slip through

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d adapter

Restructure the codebase into src/core/ (provider-agnostic kernel) and
src/providers/discord/ (adapter implementing a new BridgeProvider
interface). All Discord-specific code now lives behind the adapter; the
kernel speaks only in IncomingMessage / ConversationRecord / ChannelTarget
types. A new provider can be added by dropping src/providers/<name>/
without touching the kernel.

Behavior preserved end-to-end:
- All 7 slash commands, voice transcription, and HTTP /api/send work
  identically from the user's perspective.
- All env vars unchanged (DISCORD_*, API_PORT, WHISPER_*); new optional
  ENABLED_PROVIDERS defaults to 'discord'.
- /api/send accepts an optional `provider` field (defaults to 'discord').
- DB schema upgrades automatically on first start: agent_channels gets
  a `provider` column and composite PK (provider, channel_id);
  agent_threads is renamed to discord_agent_threads with rows preserved.
- CLI binary maestro-discord unchanged.

Tests: 158/158 pass (151 baseline + 7 new). Includes a MockProvider
smoke test exercising the BridgeProvider interface end-to-end without
any Discord code, and a legacy-schema-upgrade migration test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rename the project to reflect the new architecture: a provider-agnostic
bridge between chat platforms and Maestro agents, with Discord as the
first provider.

Local changes only — does NOT rename the GitHub repo or migrate the
installer in RunMaestro/Maestro-Discord; both require manual follow-up.

Renames:
- package name: discord-maestro -> maestro-bridge
- bin: maestro-bridge (primary), maestro-discord kept as alias
  pointing at the same dist/cli/maestro-bridge.js
- src/cli/maestro-discord.ts -> src/cli/maestro-bridge.ts
- npm script: maestro-bridge (alias maestro-discord retained)

User-facing strings in CLI help, README, AGENTS.md, docs/api.md, and
docs/architecture.md updated to "Maestro Bridge". All DISCORD_* env
vars unchanged. .env.example reorganised into Core / Discord sections
with ENABLED_PROVIDERS=discord (default) added.

Migration: existing scripts calling `maestro-discord ...` keep working
unchanged via the alias bin. Tests, typecheck, and lint all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Imports the installer plumbing that lives on main (install.sh,
maestro-discord-ctl, systemd/launchd templates) into this branch and
renames it for Maestro Bridge.

This branch is stacked on feat/rename-to-bridge, so it includes the
provider-agnostic kernel + the package rename + the installer rename
together. Land the upstream branches first.

Renames:
- bin/maestro-discord-ctl.sh -> bin/maestro-bridge-ctl.sh
- templates/maestro-discord.service -> templates/maestro-bridge.service
- templates/sh.maestro.discord.plist -> templates/sh.maestro.bridge.plist
- install dir default: ~/.local/share/maestro-bridge
- config dir default: ~/.config/maestro-bridge
- systemd unit: maestro-bridge.service
- launchd label: sh.maestro.bridge
- repo default: RunMaestro/Maestro-Bridge

Backwards compatibility:
- MAESTRO_DISCORD_* env vars accepted as fallback for every
  MAESTRO_BRIDGE_* var (so v0.0.x `maestro-discord-ctl update` works).
- Legacy ~/.local/share/maestro-discord and ~/.config/maestro-discord
  are auto-detected when the new dirs do not exist.
- install_ctl creates BOTH `maestro-bridge-ctl` and `maestro-discord-ctl`
  symlinks pointing at the same wrapper.
- install.sh removes a legacy maestro-discord systemd unit / launchd
  plist on upgrade so two services do not run side by side.
- uninstall cleans up both legacy and new service files + symlinks.

Bot path fixes:
- ctl deploy now runs `dist/providers/discord/deploy.js` (the post-
  refactor location) instead of the old `dist/deploy-commands.js`.

Note: a matching change to .github/workflows/release.yml (tarball
staging name maestro-discord-<tag> -> maestro-bridge-<tag>) is required
but excluded from this commit because the bot OAuth token lacks
workflow scope. Apply the diff in a follow-up commit pushed with a
PAT that has 'workflow' scope (the file is left unstaged in the
working tree for that purpose).

Manual follow-ups (not in this PR):
- Rename the GitHub repo RunMaestro/Maestro-Discord to
  RunMaestro/Maestro-Bridge (auto-redirects existing curl URLs).
- Push the .github/workflows/release.yml change with workflow scope.
- Cut a v0.0.5+ release once the rename PRs merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… adapter (#27)

Conflict resolution favoured the incoming refactor's structure (-X theirs).
Two rc-only improvements were ported back manually after the merge to
preserve them on top of the new architecture:

1. Multilingual whisper transcription (rc commit 4460cdc):
   - Added `whisperLanguage` to src/core/config.ts (auto-merged)
   - Added `-l <language>` flag to whisper-cli invocation in
     src/core/transcription.ts (auto-merged)

2. Auto Run path-containment security fix (rc commit a9cc725):
   - Re-introduced `resolveContainedDocPath()` in
     src/providers/discord/commands/auto-run.ts to reject absolute paths
     outside the agent's Auto Run folder and `..` traversal escapes.
   - Replaced `src/__tests__/auto-run-command.test.ts` with rc's full
     18-test version (security + autocomplete recursion tests) and
     rewrote the imports to point at the post-refactor module paths.

Tests: 169/169 pass (previously 158 on the feature branch alone; +11 are
the rc-only auto-run security and autocomplete-recursion tests now
preserved on top of the refactor).

Typecheck: clean.
…31)

* chore(rename): transition bridge naming to maestro-relay with compat aliases

* feat(installer,cli): add module switch scaffold and complete relay rename
install.sh downloads maestro-relay-${tag}.tar.gz but the workflow was
still building maestro-discord-${tag}.tar.gz, so a fresh tag would 404
on download. Aligns the release artifact with the installer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e matching

The sed extraction in deploy_commands (install.sh) and cmd_deploy
(maestro-relay-ctl) captured everything that wasn't whitespace or '#',
so a quoted value like ENABLED_PROVIDERS="discord" became "discord"
(quotes included). The subsequent case ",$ep," in *,discord,*) check
then failed against ,"discord", and the installer/ctl falsely reported
Discord as not enabled, skipping slash-command deployment.

Strip a single layer of leading/trailing single or double quotes after
extraction. Reported by Codex review of PR #32.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
launchd does not inherit the user shell PATH, causing spawn ENOENT
when the service tries to run maestro-cli from /usr/local/bin.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements BridgeProvider for Slack using @slack/bolt. Supports both
Socket Mode (dev, no public URL needed) and ExpressReceiver (webhook,
production). Slack-specific code is fully contained in
src/providers/slack/ — src/core/ has no Slack imports.

- SlackProvider: start/stop/send/react/resolveConversation/findOrCreateAgentChannel
- conversationsDb: slack_agent_conversations table keyed by thread_ts
- channelsDb: provider='slack' wrapper over core channelDb
- commands: /health, /agents (list/new/disconnect/readonly), /session new
- migrations: ensureSlackConversationsTable added to runMigrations
- providers.ts: case 'slack' added to loadProvider switch
- package.json: @slack/bolt ^3.18.0

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rors

Slack reactions.add() requires emoji name strings (e.g. hourglass_flowing_sand)
not Unicode characters. Added UNICODE_TO_SLACK mapping in the Slack adapter.
Also added error logging to the queue react catch so failures are visible
in logs instead of silently swallowed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- adapter.ts: move UNICODE_TO_SLACK after imports; validate user field
  in app_mention before use (guard against missing/non-string user);
  remove this.client non-null assertion in react() remove closure
- conversationsDb.ts: INSERT OR IGNORE to prevent SQLITE_CONSTRAINT on
  duplicate thread_ts
- commands/agents.ts: replace say:any with SayFn; blocks:object[] with
  KnownBlock[]; log conversations.list and top-level catch errors
- commands/session.ts: replace say:any with SayFn
- config.ts: guard port getter against NaN from parseInt

Skipped: @slack/bolt v4 upgrade (major version, breaking changes)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…apping

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- findOrCreateAgentChannel: nest unarchive in own try/catch so an
  unarchive failure clears channelId and falls through to create path
  instead of returning an archived/unusable channel
- /agents readonly: validate mode strictly as on|off; return usage
  error on any other value instead of silently treating it as off
- conversationsDb: extract createConversationDb(db) factory so tests
  can wire an in-memory DB without the shared singleton
- Tests: convert all three Slack test files to import real production
  modules (slackConfig, toSlackEmojiName/isThreadTs, createConversationDb)
- adapter.ts: export toSlackEmojiName and isThreadTs; move UNICODE_TO_SLACK
  constant after imports per convention
- install.sh: add Slack module support to normalize_module, have_required
  detection, env prompting/generation, and config_complete validation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a SessionEnd hook fails (e.g. claude-mem), maestro-cli exits with
code 1 and sets success:false, but the agent response is still present.
Previously this discarded the response and surfaced the raw hook error to
users. Now: if result.response is non-null, display it regardless of the
success flag. When there is no response, sanitize hook-failure messages so
the full shell script is not blasted at users — log the raw error instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Raw maestro-cli error strings (process exit codes, hook failure scripts,
internal stack traces) were being forwarded verbatim to Discord/Slack.
Replace with a generic user-facing message; log the full error detail
for debugging. No provider-specific logic in this path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Brings in installer bugfixes (PRs #33-#35), .maestro/ untrack (#36),
and the docs split (#37) that landed on main after rc was branched.

Conflict resolutions:
- Kept rc's queue error-hiding (PR #39) and per-message logger.error.
- Kept rc's WAL-checkpoint shutdown alongside main's split DB import.
- Combined main's docs split with rc's Slack provider additions in
  README.md, AGENTS.md, .env.example, install.sh, providers.ts,
  migrations.ts.
- Extended config_complete in maestro-relay-ctl.sh to cover the
  Slack key set when ENABLED_PROVIDERS=slack.
- New docs/slack.md mirrors docs/discord.md: app setup, scopes, env-var
  table, Socket Mode vs webhook mode, slash commands, runtime behavior,
  storage, security, troubleshooting.
- AGENTS-providers.md: Slack is now built-in; switch the
  worked example from Slack to a hypothetical Teams provider so
  the example doesn't pretend Slack is still unbuilt.

README.md and AGENTS.md already link the new docs/slack.md as part of
the merge from origin/main.
- Drop the agent-id positional from /agents readonly. The bound agent
  is read from channelDb so the UX matches Discord (run inside the
  channel, just toggle on/off).
- Replace console.error in slack/agents and slack/session catch blocks
  with logger.error so failures land in the same error log as the rest
  of the bridge.
- Refresh /agents list help text and docs/slack.md to reflect the
  aligned readonly UX.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bb2c74b9-c5a7-4489-bfde-e815be8ed1b2

📥 Commits

Reviewing files that changed from the base of the PR and between 85affab and 0850c6e.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • package.json
  • src/__tests__/slack-findOrCreateSlackChannel.test.ts
  • src/providers/slack/adapter.ts
  • src/providers/slack/commands/agents.ts

📝 Walkthrough

Walkthrough

This PR adds a complete Slack chat provider implementation alongside the existing Discord provider: Slack Bolt adapter, slackConfig, conversation and channel persistence, DB migration, message and slash-command handlers, installer/ctl updates, tests, docs, and dependency addition.

Changes

Slack Provider

Layer / File(s) Summary
Configuration & Types
src/providers/slack/config.ts, .env.example, docs/slack.md
Adds slackConfig getters and documents Slack env vars (tokens, IDs, socket/webhook options, port, allowed user IDs, mention ID).
Database Schema & Persistence
src/core/db/migrations.ts, src/providers/slack/conversationsDb.ts, src/providers/slack/channelsDb.ts
Adds slack_agent_conversations table and conversation/channel DB wrappers with CRUD and listing APIs.
Core Adapter Implementation
src/providers/slack/adapter.ts
Adds adapter helpers (emoji mapping, thread-ts detection, channel naming), findOrCreateSlackChannel, and SlackProvider implementing BridgeProvider: Bolt app wiring, message routing, reactions, deterministic per-agent channel creation with unarchive/fallback, and channel caching.
Message & Event Handling
src/providers/slack/messageCreate.ts
Adds threaded message handler: ignores bots/empty messages, enforces owner rules, resolves conversation, and enqueues IncomingMessage to kernel.
Command Handlers
src/providers/slack/commands/health.ts, agents.ts, session.ts
Adds slash commands for health, agent list/new/disconnect/readonly, and session creation with allowlist enforcement and error handling.
Core Infrastructure Wiring
src/core/providers.ts, src/core/queue.ts, src/index.ts, bin/maestro-relay-ctl.sh
loadProvider recognizes slack; queue error logging/agent-response handling improved; index imports DB to run migrations and performs WAL checkpoint/close on shutdown; ctl validates provider-specific required keys based on ENABLED_PROVIDERS.
Installation & Configuration
install.sh, .env.example, templates/sh.maestro.relay.plist
Installer supports slack module, prompts/writes Slack env values (including optional Socket Mode token, port, allowed user IDs, mention ID), and template prepends PATH entries for Node discovery.
Dependencies & Build
package.json
Bumps version to 0.2.0 and adds @slack/bolt dependency; updates dev types.
Tests
src/__tests__/*
Adds tests for slackConfig parsing/validation, conversation DB CRUD, emoji mapping and thread-ts detection, channel-name/fallback/pagination behavior, and find-or-create channel flows.
Documentation
docs/slack.md, AGENTS.md, AGENTS-providers.md, README.md
Adds Slack setup and runtime docs, updates contributor and installer docs to include Slack as a built-in provider.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • RunMaestro/Maestro-Relay#40: Both PRs implement the same Slack provider integration (same files added/changed like src/providers/slack/*, conversations DB, config, commands, tests, and dynamic provider loading in src/core/providers.ts).
  • RunMaestro/Maestro-Relay#38: Similar Slack provider integration touching src/providers/slack/*, migrations, .env, and installer logic.
  • RunMaestro/Maestro-Relay#32: Provider-agnostic kernel changes that this PR builds upon (provider loading and core abstractions).

Poem

🐰 A rabbit found a Slack thread bright,

It hopped through channels and named them right,
Emoji mapped, a session sprang,
Threads and channels in a boisterous gang,
Hooray—providers now share the light.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: adding Slack provider support alongside Discord, hardening the installer, and reorganizing documentation into separate provider-specific files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rc

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/slack.md`:
- Line 72: The docs claim a "-<timestamp>" suffix is appended when unarchiving
an existing "maestro-<sanitized-agent-name>" channel fails, but the Slack
adapter doesn't implement that fallback; update docs/slack.md to remove or
revise that sentence (and the similar line at 91) to reflect actual behavior
(e.g., state that archived channels are attempted to be unarchived and if
unarchive fails the operation errors/returns the failure) and reference the
"maestro-<sanitized-agent-name>" naming rule and the Slack adapter unarchive
behavior rather than promising a timestamp suffix.

In `@package.json`:
- Line 25: Update the "@slack/bolt" dependency in package.json from "^3.18.0" to
"^4.6.0" (or the latest 4.x stable); after changing the version, run your
package manager (npm/yarn/pnpm) to install and then run tests/build to catch
breaking changes from the major bump; if any runtime/compile issues appear,
inspect code using the Slack Bolt constructs (e.g., App, Receiver, middleware
handlers) and apply the 4.x migration adjustments per the official upgrade
guide.

In `@src/providers/slack/adapter.ts`:
- Around line 264-304: The current channelName is built only from sanitizedName
(sanitized agent.name) which can collide for different agents; modify the logic
so channelName includes a stable unique identifier from the agent (e.g.,
agent.id or a short hash of it) when constructing channelName (use the same
channelName for both the conversations.list lookup and conversations.create),
reduce the substring max length for sanitizedName to leave room for the suffix
(e.g., shorten from 70 to ~60) and join with a hyphen (e.g.,
`maestro-${sanitizedName}-${agent.id.slice(0,8)}`), and keep the rest of the
flow (lookup via listRes.channels?.find(ch => ch.name === channelName),
unarchive, create, then channelDb.register(channelId, agent.id, agent.name))
unchanged so names remain unique and consistent.

In `@templates/sh.maestro.relay.plist`:
- Line 13: The PATH assignment in the plist template string can produce a
trailing colon when PATH is empty; update the export to conditionally append the
inherited PATH using parameter expansion, e.g. replace 'export
PATH="/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin:$PATH"' with 'export
PATH="/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin${PATH:+:$PATH}"' in the
template string so the current directory is not implicitly added when PATH is
empty.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1fc84ab6-c83a-40cb-9aae-f1dc224e93ae

📥 Commits

Reviewing files that changed from the base of the PR and between 1ed4811 and 1f2bdfa.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (24)
  • .env.example
  • AGENTS-providers.md
  • AGENTS.md
  • README.md
  • bin/maestro-relay-ctl.sh
  • docs/slack.md
  • install.sh
  • package.json
  • src/__tests__/slack-config.test.ts
  • src/__tests__/slack-conversationsDb.test.ts
  • src/__tests__/slack-react.test.ts
  • src/core/db/migrations.ts
  • src/core/providers.ts
  • src/core/queue.ts
  • src/index.ts
  • src/providers/slack/adapter.ts
  • src/providers/slack/channelsDb.ts
  • src/providers/slack/commands/agents.ts
  • src/providers/slack/commands/health.ts
  • src/providers/slack/commands/session.ts
  • src/providers/slack/config.ts
  • src/providers/slack/conversationsDb.ts
  • src/providers/slack/messageCreate.ts
  • templates/sh.maestro.relay.plist

Comment thread docs/slack.md Outdated
Comment thread package.json Outdated
Comment thread src/providers/slack/adapter.ts Outdated
Comment thread templates/sh.maestro.relay.plist Outdated
chr1syy added 2 commits May 10, 2026 20:05
- Bump @slack/bolt from ^3.18.0 to ^4.6.0. The only API impact is
  KnownBlock no longer being re-exported from @slack/bolt; pull it
  from @slack/types directly (Bolt 4 still depends on it transitively).
- Make Slack channel names collision-safe by appending an 8-char
  alphanumeric prefix of agent.id: maestro-<sanitized>-<idprefix>.
  Two different agents whose names normalize to the same string
  no longer collapse onto the same Slack channel.
- Hoist the channel-name + lookup-or-create logic into shared
  helpers (buildAgentChannelName, findOrCreateSlackChannel) so the
  HTTP-API push path (adapter.ts) and the /agents new slash command
  use identical behavior, including the unarchive-then-fallback
  timestamp dance that previously only existed in the slash command.
- launchd plist: replace `:$PATH` with `${PATH:+:$PATH}` so an empty
  inherited PATH doesn't leave a trailing colon (which silently adds
  the cwd to lookup).
- docs/slack.md: refresh the channel-naming bullet and the
  name_taken troubleshooting line so they describe the actual
  helper-driven behavior.
- Add slack-channelName.test.ts covering collision avoidance,
  empty-name fallback, and the 80-char cap.

196/196 tests passing.
- Paginate conversations.list when looking up an existing channel.
  Workspaces with >1000 public channels would previously miss matches
  on later pages and fail with name_taken on create. New
  findChannelByName helper walks response_metadata.next_cursor until
  it finds a hit or exhausts pages.
- buildFallbackChannelName reserves the -<timestamp> suffix BEFORE
  concatenation, so an 80-char base no longer has its suffix sliced
  away (which would re-collide with the original name).
- SLACK_PORT now rejects values outside 1..65535 (and falls back to
  3000), preventing app.start() from crashing on negative or oversized
  ports passed via env.
- Add tests for the three fixes:
  - findChannelByName paginates, returns null when absent, stops on
    empty next_cursor.
  - buildFallbackChannelName preserves the suffix at max length and
    trims base when needed.
  - SLACK_PORT rejects 0, -1, 65536, 70000 and accepts the boundaries
    1 and 65535.

204/204 tests passing.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/providers/slack/adapter.ts`:
- Around line 220-221: The comment and code disagree: the regex used in
text.replace(/<@[^>]+>/g, '') removes all mentions but the comment says "Strip
bot mention"; either update the comment to "Strip all mentions from text" to
reflect the existing behavior (referencing the cleanText variable and the
text.replace call) or change the code to only strip the bot's mention by
fetching the bot user id (e.g., via this.client!.auth.test() to get botUserId)
and replacing only `<@${botUserId}>`; adjust the cleanText assignment
accordingly and keep the replacement scoped to the botUserId.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3bac5296-4b73-4ad3-9784-db5fc81aeb20

📥 Commits

Reviewing files that changed from the base of the PR and between d451d79 and 4812716.

📒 Files selected for processing (4)
  • src/__tests__/slack-channelName.test.ts
  • src/__tests__/slack-config.test.ts
  • src/providers/slack/adapter.ts
  • src/providers/slack/config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/providers/slack/config.ts

Comment thread src/providers/slack/adapter.ts Outdated
chr1syy added 3 commits May 10, 2026 22:31
…havior

CodeRabbit flagged that the comment "Strip bot mention" was misleading
since /<@[^>]+>/g strips every Slack user mention, not just the bot's.
The behavior is intentional (other users' <@u123> tokens would be opaque
to the agent and Slack already notified them on the original message),
so just rewrite the comment to describe what the code actually does.
Four small cleanups so this PR ships Slack support without leaving known
gaps for follow-up work:

- Bump @types/node from ^20 to ^22 to match the Node 22+ runtime
  requirement (engines, install.sh, README).
- SlackProvider.send() and .react() now log slack/{send,react}:
  orphan-thread before throwing when a target thread_ts has no row in
  slack_agent_conversations. The kernel still surfaces the generic ❌,
  but operators can grep for the specific orphan event without trawling
  through queue:send-error noise.
- Align Slack /agents new lookup with Discord's: exact id, id-prefix,
  or exact name. Previously Slack only accepted exact id or
  case-insensitive name; matching Discord means an agent ID/name that
  works in one chat platform works in the other.
- Add slack-findOrCreateSlackChannel.test.ts covering the seven
  orchestration paths: existing-not-archived, existing-archived
  (unarchive succeeds), existing-archived (unarchive fails →
  timestamped fallback create), missing channel (primary create),
  list throws (still creates), create returns no id (throws),
  pagination across pages.

211/211 tests passing.
Slack is a new built-in provider — a feature, not a bugfix — so this
bumps to 0.2.0 per semver. Patch releases since 0.1.0 (0.1.1 through
0.1.3) were installer/workflow fixes; this is the first release with
a new chat platform onboarded.
@chr1syy chr1syy merged commit 3197bdc into main May 11, 2026
2 of 3 checks passed
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.

2 participants