Conversation
Bridge Codex into Slack alongside Telegram: DMs stream progress by editing a single message, channel mentions open threaded conversations, approvals arrive as buttons, files flow both ways, and a /telex slash command carries the bridge commands. Enabled by setting SLACK_BOT_TOKEN, SLACK_APP_TOKEN, and SLACK_ALLOWED_USER_IDS together; docs/slack.md documents the app manifest and setup.
- Deliver caption-less file uploads to Codex with attachment descriptions instead of silently dropping them after download. - Reject conversation-scoped /telex subcommands in channels with guidance (each thread is its own conversation; a slash command carries no thread), instead of acting on a conversation key no message flow creates. - Anchor thread replies to the latest scheduled-run notification published in that thread so reply context resolves despite Slack's flat threads. - Escape Slack entities in approval prompts, selection updates, and attachment-failure notices; raw < > & mangled approval text and could ping @channel through Codex-controlled content. - Deduplicate redelivered Socket Mode envelopes by envelope_id so slash commands and button clicks cannot execute twice after a reconnect. - Send only unposted chunks through the slash-command response webhook. - Refresh active-thread recency when scheduled results are published.
Telegram credentials become optional: each connector is an all-or-nothing env group and at least one must be configured. Without Telegram, the Telegram channel, the settings Mini App, and the quick tunnel stay off; the Slack connector runs standalone.
Multi-stage image running Telex as the unprivileged telex user with all state under a /data volume. The entrypoint seeds Codex config with sandbox_mode danger-full-access on fresh volumes: Codex's bubblewrap sandbox needs user namespaces that Docker's default confinement blocks, so the container itself is the isolation boundary. Includes a Compose example and docs/docker.md.
SLACK_ALLOWED_USER_IDS=* authorizes any regular member of the installed workspace. Membership is verified through users.info against the bot's team and cached for ten minutes: bots, deactivated accounts, guests, and Slack Connect participants from other workspaces stay rejected, and deactivating someone locks them out without a restart. Scheduled-run owner re-checks go through the same gate.
gh reads GH_TOKEN from the environment; when set, the entrypoint also wires git's HTTPS credential helper through gh so clones and fetches work headlessly.
/telex config in the bot DM now renders interactive Codex settings from Slack blocks (model, reasoning effort, speed tier, approvals, sandbox, web search) through CodexConfigService with optimistic versioning — replacing the Telegram-only Mini App pointer. SLACK_ADMIN_USER_IDS optionally restricts instance-wide commands (config, login, logout, reload, restart, update) to listed users; the gate also covers the mention-text command form and config buttons.
Slack rejects chat.update/postMessage payloads far below the documented 40k ceiling (msg_too_long observed at 12k), which both truncated long answers and, because delivery shared one try block, dropped every remaining chunk after the first failure. The message limit drops to 3,900 characters and each chunk now posts independently, with a notice when parts fail. The final answer no longer silently edits the thinking message — the progress message freezes without the streaming cursor and the answer arrives as fresh messages that actually notify. Markdown tables render as aligned monospace blocks instead of raw pipes.
A mention inside an existing thread calls the bot into a running discussion. The channel now fetches the earlier thread messages through conversations.replies (up to 100, oldest dropped over an 8k character budget) and prefixes them as context for Codex, with display names resolved and the triggering message excluded. Commands and already active threads skip the fetch, and a fetch failure degrades to the bare message.
Follow-ups in a thread the bot already answered no longer trigger it: with workspace-wide access, humans discussing inside such a thread had every message routed to Codex. A mention is now required each time; the engaged-thread set survives only to skip re-reading thread history on repeat mentions, and thread context still arrives on the first one.
Every inbound message logs the sender (user ID and display name), conversation, command, and text; per-turn child loggers carry that identity into the stream, which now mirrors the run: each Codex tool call once, reasoning summaries as they change, and the delivered answer. Rollouts never recorded who triggered a turn, so operators had no way to attribute usage.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds an optional Slack Socket Mode connector with authorization, message routing, threading, interactive controls, file handling, and shared progress utilities. Adds Docker packaging with persistent state initialization and an unprivileged runtime user. Refactors Telegram and Slack as independent optional connectors. ChangesSlack connector and container deployment
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/channels/slack/channel.ts (2)
764-773: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on the
response_urlPOST.Node's
fetchhas no default timeout, so a stalled Slack endpoint keeps this request — and the awaiting slash-command handler — pending indefinitely. Slack response URLs are only valid for 30 minutes; bound the call.🛡️ Proposed fix
await fetch(url, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ response_type: "ephemeral", text }), + signal: AbortSignal.timeout(10_000), }).catch((error: unknown) => {🤖 Prompt for 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. In `@src/channels/slack/channel.ts` around lines 764 - 773, Update respondThroughWebhook to bound the response_url fetch with an AbortController-based timeout, ensuring a stalled Slack POST is aborted and the existing debug logging handles the resulting error. Keep the current webhook payload and successful response behavior unchanged.
466-474: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential
users.infolookups add latency to the ingress path.Distinct thread participants are resolved one at a time before Codex ever sees the message. Resolving them concurrently keeps first-mention latency flat as the thread grows.
♻️ Proposed refactor
- const names = new Map<string, string>(); - for (const message of replies) { - if (message.user !== undefined && !names.has(message.user)) { - names.set( - message.user, - message.user === botUserId ? "Telex (this bot)" : await this.displayName(message.user), - ); - } - } + const participants = [ + ...new Set(replies.flatMap((message) => (message.user === undefined ? [] : [message.user]))), + ]; + const names = new Map( + await Promise.all( + participants.map( + async (user): Promise<[string, string]> => [ + user, + user === botUserId ? "Telex (this bot)" : await this.displayName(user), + ], + ), + ), + );🤖 Prompt for 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. In `@src/channels/slack/channel.ts` around lines 466 - 474, Update the participant-name resolution in the replies processing flow to perform distinct non-bot user lookups concurrently rather than awaiting displayName sequentially inside the loop. Preserve the names Map, bot label, deduplication, and resulting user-to-name mappings while using Promise.all or equivalent before Codex receives the message.src/channels/slack/reply.ts (2)
143-159: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winButton encoding failure aborts
publishSlackMessageafter text is already posted.
encodeSlackCommandValuethrows for unsafe/oversized action args, so a scheduled-run notification can land as text with the whole call rejecting afterwards. Consider dropping the offending action (with a warning) instead of throwing, so the message stays deliverable.Also applies to: 174-205
🤖 Prompt for 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. In `@src/channels/slack/reply.ts` around lines 143 - 159, Update commandButtonBlocks so failures from encodeSlackCommandValue do not propagate into publishSlackMessage. Encode each action independently, omit actions whose command value cannot be encoded, and emit a warning for each dropped action; preserve valid buttons and return undefined when none remain.
278-289: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on the fallback webhook call.
this.#fetchhas no default timeout, so a stalledresponse_urlrequest can pin the reply path indefinitely. ConsiderAbortSignal.timeout(...).⏱️ Suggested change
const response = await this.#fetch(url, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ response_type: "ephemeral", text: text.slice(0, slackTextLimit) }), + signal: AbortSignal.timeout(10_000), });🤖 Prompt for 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. In `@src/channels/slack/reply.ts` around lines 278 - 289, Update respondThroughWebhook to enforce a timeout on the fallback webhook request by passing an AbortSignal.timeout(...) signal in the this.#fetch options. Preserve the existing response handling and error behavior while ensuring stalled response_url calls terminate within the established webhook timeout.test/slack-format.test.ts (1)
42-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest name promises entity escaping but the fixture has no entities.
Add
</&inside the fence to actually lock the escaping behavior.🧪 Suggested case
- const input = "```ts\nconst a = b ** 2; // **not bold**\n```"; - expect(markdownToMrkdwn(input)).toBe("```ts\nconst a = b ** 2; // **not bold**\n```"); + const input = "```ts\nconst a = b ** 2; // **not bold** & a < b\n```"; + expect(markdownToMrkdwn(input)).toBe( + "```ts\nconst a = b ** 2; // **not bold** & a < b\n```", + );🤖 Prompt for 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. In `@test/slack-format.test.ts` around lines 42 - 45, Update the fenced-code fixture in the test “leaves fenced code untouched apart from entity escaping” to include ampersand and less-than characters, and change the expected output to assert they are entity-escaped while Markdown markers remain unchanged.test/slack-reply.test.ts (1)
208-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest does not assert what its name claims.
The override never records into
calls.posts, so onlyupdatesis checked. Assert the mock received the final text.🧪 Suggested tightening
- const { api, calls } = fakeApi({ - postMessage: vi - .fn<SlackMessagingApi["postMessage"]>() - .mockRejectedValueOnce(new Error("temporarily unavailable")) - .mockResolvedValue("1700.1"), - }); + const postMessage = vi + .fn<SlackMessagingApi["postMessage"]>() + .mockRejectedValueOnce(new Error("temporarily unavailable")) + .mockResolvedValue("1700.1"); + const { api, calls } = fakeApi({ postMessage }); const reply = stream(api); await reply.start(); await reply.complete("result"); expect(calls.updates).toHaveLength(0); + expect(postMessage).toHaveBeenLastCalledWith(expect.objectContaining({ text: "result" }));🤖 Prompt for 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. In `@test/slack-reply.test.ts` around lines 208 - 219, Update the test “posts the final text directly when no progress message exists” to assert that the postMessage mock recorded the final text “result” in calls.posts, while retaining the existing assertion that no updates occurred.
🤖 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 @.env.example:
- Around line 1-13: Comment out the optional Telegram sample assignments
TELEGRAM_BOT_TOKEN and TELEGRAM_ALLOWED_USER_IDS in the environment template,
matching the existing Slack examples. Preserve their explanatory comments so
users can uncomment and configure Telegram explicitly without enabling it
accidentally.
In `@Dockerfile`:
- Around line 30-31: The Dockerfile must start the entrypoint as root, while
docker/entrypoint.sh must chown the required /data paths before state creation,
then drop to UID 1001 and exec Telex as telex; update Dockerfile lines 30-31 and
docker/entrypoint.sh lines 7-9 accordingly.
- Around line 12-19: Update the GitHub CLI APT source in the Dockerfile
installation chain to use the platform architecture returned by dpkg
--print-architecture instead of hard-coding amd64, while preserving the existing
signed keyring and gh installation flow.
In `@src/channels/slack/channel.ts`:
- Around line 750-761: Update the user-name resolution flow around the
users.info lookup so failed Slack lookups are not written to `#displayNames`.
Cache and return the resolved name only after a successful response; on failure,
log the error and return the raw userId without modifying the cache.
- Line 520: Update the isDirect determination in the surrounding Slack payload
handling to inspect payload.channel_id using the existing “D” prefix convention,
rather than comparing the user-controlled payload.channel_name to
“directmessage”. Keep the resulting isPrivate behavior unchanged for genuine
direct-message channels.
- Around line 352-385: Ensure the message-level attachment directory created
before the normalized.files loop is removed after Slack message handling
completes, including when processing or downstream handling fails. Update the
enclosing attachment-processing flow around directory and attachments so cleanup
runs exactly once after use, while preserving the existing per-file download
failure behavior.
In `@src/channels/slack/file.ts`:
- Around line 90-102: Update isSlackFileHost to require the parsed URL’s
protocol to be exactly https: before accepting any Slack hostname, while
preserving the existing hostname allowlist and false result for invalid URLs.
- Around line 78-88: Cap downloads before and during the pipeline that writes
the Slack response to target. Reject files whose available file.size exceeds the
configured maximum, then add a byte-counting stream guard so mismatched or
missing sizes cannot exceed that limit. Preserve the existing unlink cleanup and
error propagation when the limit is exceeded.
In `@src/channels/slack/reply.ts`:
- Around line 511-517: Clamp the composed Slack preview in preview() so the
final returned string, including escaped progress, final text, separator, and
cursor, never exceeds slackTextLimit; truncate the escaped progress as needed
before calculating available. Apply the same composed-length clamping to the
freeze update near the existing preview/freeze construction so both draft update
paths remain within Slack’s limit.
---
Nitpick comments:
In `@src/channels/slack/channel.ts`:
- Around line 764-773: Update respondThroughWebhook to bound the response_url
fetch with an AbortController-based timeout, ensuring a stalled Slack POST is
aborted and the existing debug logging handles the resulting error. Keep the
current webhook payload and successful response behavior unchanged.
- Around line 466-474: Update the participant-name resolution in the replies
processing flow to perform distinct non-bot user lookups concurrently rather
than awaiting displayName sequentially inside the loop. Preserve the names Map,
bot label, deduplication, and resulting user-to-name mappings while using
Promise.all or equivalent before Codex receives the message.
In `@src/channels/slack/reply.ts`:
- Around line 143-159: Update commandButtonBlocks so failures from
encodeSlackCommandValue do not propagate into publishSlackMessage. Encode each
action independently, omit actions whose command value cannot be encoded, and
emit a warning for each dropped action; preserve valid buttons and return
undefined when none remain.
- Around line 278-289: Update respondThroughWebhook to enforce a timeout on the
fallback webhook request by passing an AbortSignal.timeout(...) signal in the
this.#fetch options. Preserve the existing response handling and error behavior
while ensuring stalled response_url calls terminate within the established
webhook timeout.
In `@test/slack-format.test.ts`:
- Around line 42-45: Update the fenced-code fixture in the test “leaves fenced
code untouched apart from entity escaping” to include ampersand and less-than
characters, and change the expected output to assert they are entity-escaped
while Markdown markers remain unchanged.
In `@test/slack-reply.test.ts`:
- Around line 208-219: Update the test “posts the final text directly when no
progress message exists” to assert that the postMessage mock recorded the final
text “result” in calls.posts, while retaining the existing assertion that no
updates occurred.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 08477283-9c25-4e88-9f3b-c34b3c7aafb9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
.dockerignore.env.exampleDockerfileREADME.mddocker/docker-compose.example.ymldocker/entrypoint.shdocs/docker.mddocs/slack.mdpackage.jsonsrc/channels/progress.tssrc/channels/slack/authorization.tssrc/channels/slack/channel.tssrc/channels/slack/config-ui.tssrc/channels/slack/file.tssrc/channels/slack/format.tssrc/channels/slack/message.tssrc/channels/slack/references.tssrc/channels/slack/reply.tssrc/channels/telegram/reply.tssrc/config/env.tssrc/index.tstest/env.test.tstest/slack-authorization.test.tstest/slack-config-ui.test.tstest/slack-format.test.tstest/slack-message.test.tstest/slack-references.test.tstest/slack-reply.test.ts
Comment on lines
+352
to
+385
| const directory = join(this.#attachmentDirectory, crypto.randomUUID()); | ||
| const attachments: InboundAttachment[] = []; | ||
| const failures: string[] = []; | ||
| for (const [index, file] of normalized.files.entries()) { | ||
| const description = describeSlackFile(file); | ||
| try { | ||
| const path = await downloadSlackFile(file, { | ||
| botToken: this.#botToken, | ||
| directory, | ||
| index, | ||
| }); | ||
| attachments.push({ kind: slackAttachmentKind(file), path, description }); | ||
| } catch (error) { | ||
| this.#logger.warn("Could not download Slack attachment", { | ||
| messageTs: event.ts, | ||
| description, | ||
| error: errorMessage(error).replaceAll(this.#botToken, "<redacted>"), | ||
| }); | ||
| const reason = | ||
| error instanceof SlackFileDownloadError | ||
| ? error.userMessage | ||
| : "Slack could not provide the file"; | ||
| failures.push(`[${description} was not attached: ${reason}.]`); | ||
| } | ||
| } | ||
|
|
||
| const caption = [normalized.text, ...failures].filter((part) => part.length > 0).join("\n\n"); | ||
| // A bare file upload has no text; describe the attachments so the message | ||
| // still reaches Codex instead of being dropped after the download. | ||
| const text = | ||
| caption.length > 0 | ||
| ? caption | ||
| : attachments.map((attachment) => `[Attached: ${attachment.description}]`).join("\n"); | ||
| if (text.length === 0) return; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find who owns cleanup of the attachment directory
rg -nP -C4 '\battachmentDirectory\b|attachment_?[Dd]ir' --type=ts
rg -nP -C3 '\b(rm|unlink|rmdir|cleanup|purge|sweep)\w*\s*\(' --type=ts -g '!test/**' | rg -i 'attach|tmp|temp|download'Repository: sadfun/telex
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked TS files near src/channels/slack/channel.ts =="
git ls-files | rg '(^|/)src/channels/slack/channel\.ts$|test/|spec/|\.test\.(ts|tsx)$|\.spec\.(ts|tsx)$' | sed -n '1,120p'
echo
echo "== attachmentDirectory references (all types) =="
rg -n -C3 'attachmentDirectory|attachment_[Dd]ir|randomUUID\(\)|downloadSlackFile' . --glob '!node_modules' --glob '!dist' --glob '!build' | sed -n '1,240p'
echo
echo "== cleanup/deletion references near attach/temp/download =="
rg -n -C2 '\b(unlink|unlinkSync|rmdir|rmdirSync|rm|rmSync|cleanup|purge|sweep|removeDir)\b|cleanupDir|delete.*Download|download.*Delete|attachmentDirectory' . --glob '!node_modules' --glob '!dist' --glob '!build' --glob '!test/**' --glob '!node_modules/**' | sed -n '1,240p'Repository: sadfun/telex
Length of output: 30078
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/channels/slack/channel.ts handle/construct lifecycle =="
sed -n '130,175p' src/channels/slack/channel.ts
sed -n '330,405p' src/channels/slack/channel.ts
echo
echo "== Slack inbound constructors / bridge service refs =="
rg -n -C3 'new Slack(\b|<)|slack/|SlackConfig|attachmentDirectory:|attachmentDirectory =' src test --glob '!test/**' | sed -n '1,260p'
echo
echo "== programmatic check: Slack channel has no rm/unlink cleanup =="
python3 - <<'PY'
from pathlib import Path
text = Path("src/channels/slack/channel.ts").read_text()
for target in ["rm(", "rmSync(", "unlink(", "unlinkSync(", "rmdir(", "rmdirSync("):
occurrences = [i for i in range(len(text)) if text.startswith(target, i)]
print(f"{target}: {len(occurrences)}")
print("rm/import present:", "rm" in text)
print("fs/promises imported:", "node:fs/promises" in text)
print("unlink present in file:", "unlink" in text)
PYRepository: sadfun/telex
Length of output: 13420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Slack constructor lifecycle (source lines 132-182) =="
sed -n '132,182p' src/channels/slack/channel.ts
echo
echo "== SlackChannel references creating/use =="
rg -n -C3 'new Slack\(.*SlackChannel|new SlackChannel|SlackChannel\s*=' src test --glob '!node_modules' --glob '!dist' --glob '!build' | sed -n '1,220p'
echo
echo "== filesystem cleanup calls in src/channels/slack =="
python3 - <<'PY'
from pathlib import Path
text = Path("src/channels/slack/channel.ts").read_text() + "\n" + Path("src/channels/slack/file.ts").read_text()
for term in ["rm(", "rmSync(", "unlink(", "unlinkSync(", "rmdir(", "rmdirSync(", "fs/promises", "unlink", "rm"]:
print(f"{term!r}: {term in text}")
PY
sed -n '24,88p' src/channels/slack/file.tsRepository: sadfun/telex
Length of output: 5246
Clean up Slack attachment directories after message handling.
src/channels/slack/channel.ts creates a UUID-named directory under join(config.workspace, ".telex", "attachments") for every Slack message with attachments, and downloadSlackFile() only removes the individual file on download failure. No code removes those message directories, so the Slack channel leaves dangling data behind a long-running container.
🤖 Prompt for 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.
In `@src/channels/slack/channel.ts` around lines 352 - 385, Ensure the
message-level attachment directory created before the normalized.files loop is
removed after Slack message handling completes, including when processing or
downstream handling fails. Update the enclosing attachment-processing flow
around directory and attachments so cleanup runs exactly once after use, while
preserving the existing per-file download failure behavior.
Codex referenced workspace files as markdown links to container paths, which render as dead links in Slack and Telegram. The remote-client context now instructs Codex to cite code as repo-relative inline paths or full repository URLs, and the Slack converter renders any non-URL link target as inline code instead of a broken <path|label> link.
- Comment out the Telegram samples in .env.example: uncommented placeholders fail validation in a Slack-only copy. - Let the entrypoint start as root only to chown freshly created volumes (non-recursively), then drop to the telex user via runuser; fresh named volumes are root-owned and previously broke first start. - Install gh for the image architecture instead of hard-coded amd64. - Cache display names only on successful lookups so a transient users.info failure does not pin the raw ID until restart. - Derive slash-command isDirect from the channel ID prefix; a channel literally named directmessage could spoof channel_name. - Require https in the Slack file-host check, cap downloads at 100 MB up front, and count streamed bytes so a wrong size cannot bypass it. - Clamp the streaming preview and the frozen progress text so entity escaping cannot push a chat.update past the message limit. - Drop unencodable command buttons with a warning instead of failing the whole scheduled delivery; bound both response webhooks with a 10s abort timeout; resolve thread-context names concurrently. - Extend the fenced-code and final-delivery tests per review.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docker/entrypoint.sh (1)
33-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConfigure GitHub CLI for the target host and fail on setup errors.
With only
GH_TOKENin a fresh container,gh auth setup-gitcannot select an authenticated host.|| truehides the failure, so later HTTPS Git operations cannot use the credential helper. Run it with--hostname "${GH_HOST:-github.com}" --forceand do not suppress errors.🤖 Prompt for 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. In `@docker/entrypoint.sh` around lines 33 - 37, Update the GH_TOKEN setup block in docker/entrypoint.sh to invoke gh auth setup-git with --hostname "${GH_HOST:-github.com}" and --force, and remove the || true suppression so setup failures propagate. Keep the existing GH_TOKEN and gh availability checks unchanged.src/channels/slack/file.ts (1)
60-115: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a timeout to the Slack file download request.
If the Slack file host stalls before headers or during body streaming, the sequential attachment loop waits indefinitely. Pass
signal: AbortSignal.timeout(downloadTimeoutMs)tofetch()so the request andresponse.bodyhave a deadline. The existing cleanup and failure handling then run when the download aborts.🤖 Prompt for 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. In `@src/channels/slack/file.ts` around lines 60 - 115, Update the fetch call in the Slack file download function to pass signal: AbortSignal.timeout(downloadTimeoutMs), ensuring both response headers and body streaming are bounded by the deadline. Preserve the existing catch, pipeline, cleanup, and SlackFileDownloadError handling so aborted requests follow the current failure path.
♻️ Duplicate comments (1)
src/channels/slack/channel.ts (1)
352-386: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up the Slack attachment directory after message handling.
This creates a UUID-named directory under
#attachmentDirectoryfor every message with attachments, anddownloadSlackFile()only removes the individual file on a per-file failure. Nothing removes the message-leveldirectoryafterdispatch()completes (lines 436-441), so a long-running container accumulates one directory per attachment-bearing message indefinitely on the persistent/datavolume.🧹 Proposed fix
try { await this.dispatch(inbound, event.channel, sender); } catch (error) { this.#logger.error("Slack message handler failed", error, { messageTs: inbound.id }); await responder.sendText(`Bridge error: ${errorMessage(error)}`).catch(() => undefined); + } finally { + if (attachments.length > 0) { + await rm(directory, { recursive: true, force: true }).catch(() => undefined); + } }Import
rmfromnode:fs/promisesif not already present.🤖 Prompt for 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. In `@src/channels/slack/channel.ts` around lines 352 - 386, Clean up the per-message attachment directory created in the Slack message handling flow after dispatch completes. Update the handler containing normalized, directory, attachments, and failures to wrap the downstream dispatch in a finally block that calls rm on directory with recursive and force options, ensuring cleanup occurs on success and failure; import rm from node:fs/promises if needed.
🧹 Nitpick comments (1)
src/channels/slack/channel.ts (1)
466-483: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the concurrency of thread-participant name lookups.
uniqueUsers.map(...)fires oneusers.infocall per unique thread participant, all at once viaPromise.all, for up to 100 replies fetched byfetchThreadReplies. In an active thread with many distinct participants this bursts dozens of concurrent Slack Web API calls on the first mention, on top of whatever else is happening on the same bot token, risking rate-limit responses that silently degrade to raw user IDs (viadisplayName's catch-and-fallback) instead of failing loudly.Resolve names sequentially, or in small batches, so the lookup burst does not compete with other Slack API traffic on the same token.
🔧 Proposed fix (sequential, reuses the existing cache)
- const names = new Map<string, string>( - await Promise.all( - uniqueUsers.map( - async (user): Promise<[string, string]> => [ - user, - user === botUserId ? "Telex (this bot)" : await this.displayName(user), - ], - ), - ), - ); + const names = new Map<string, string>(); + for (const user of uniqueUsers) { + names.set(user, user === botUserId ? "Telex (this bot)" : await this.displayName(user)); + }🤖 Prompt for 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. In `@src/channels/slack/channel.ts` around lines 466 - 483, Update the thread-participant name lookup around uniqueUsers and the names Map to resolve users sequentially or in small bounded batches instead of launching every displayName call through Promise.all. Preserve the botUserId mapping and existing displayName cache/fallback behavior while ensuring concurrent Slack users.info requests remain limited.
🤖 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 `@docker/entrypoint.sh`:
- Around line 10-18: The root branch of the entrypoint currently ignores
ownership failures and only updates top-level paths. In the id-u check around
mkdir/chown, recursively migrate ownership of ${data_dir} and ${workspace} to
telex:telex, and remove the silent failure fallback so any chown failure exits
before the runuser transition.
---
Outside diff comments:
In `@docker/entrypoint.sh`:
- Around line 33-37: Update the GH_TOKEN setup block in docker/entrypoint.sh to
invoke gh auth setup-git with --hostname "${GH_HOST:-github.com}" and --force,
and remove the || true suppression so setup failures propagate. Keep the
existing GH_TOKEN and gh availability checks unchanged.
In `@src/channels/slack/file.ts`:
- Around line 60-115: Update the fetch call in the Slack file download function
to pass signal: AbortSignal.timeout(downloadTimeoutMs), ensuring both response
headers and body streaming are bounded by the deadline. Preserve the existing
catch, pipeline, cleanup, and SlackFileDownloadError handling so aborted
requests follow the current failure path.
---
Duplicate comments:
In `@src/channels/slack/channel.ts`:
- Around line 352-386: Clean up the per-message attachment directory created in
the Slack message handling flow after dispatch completes. Update the handler
containing normalized, directory, attachments, and failures to wrap the
downstream dispatch in a finally block that calls rm on directory with recursive
and force options, ensuring cleanup occurs on success and failure; import rm
from node:fs/promises if needed.
---
Nitpick comments:
In `@src/channels/slack/channel.ts`:
- Around line 466-483: Update the thread-participant name lookup around
uniqueUsers and the names Map to resolve users sequentially or in small bounded
batches instead of launching every displayName call through Promise.all.
Preserve the botUserId mapping and existing displayName cache/fallback behavior
while ensuring concurrent Slack users.info requests remain limited.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 85b4df07-2b4c-492f-ace8-a8856f2a31a6
📒 Files selected for processing (8)
.env.exampleDockerfiledocker/entrypoint.shsrc/channels/slack/channel.tssrc/channels/slack/file.tssrc/channels/slack/reply.tstest/slack-format.test.tstest/slack-reply.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- .env.example
- Dockerfile
- test/slack-format.test.ts
- test/slack-reply.test.ts
- src/channels/slack/reply.ts
Comment on lines
+10
to
+18
| # Fresh named volumes are created owned by root; take ownership of the state | ||
| # roots (non-recursively), then continue as the unprivileged telex user. | ||
| if [ "$(id -u)" = "0" ]; then | ||
| mkdir -p "${data_dir}" "${workspace}" | ||
| chown telex:telex /data "${data_dir}" "${workspace}" 2>/dev/null || true | ||
| exec env HOME=/home/telex runuser -u telex -- "$0" "$@" | ||
| fi | ||
|
|
||
| mkdir -p "${data_dir}/codex-home" "${workspace}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- docker/entrypoint.sh ---'
cat -n docker/entrypoint.sh
printf '%s\n' '--- related Docker configuration ---'
rg -n -C 3 'entrypoint|data_dir|workspace|codex-home|/data|/workspace|runuser|USER ' Dockerfile* docker compose*.yml compose*.yml 2>/dev/null || true
printf '%s\n' '--- references to state paths ---'
rg -n -C 2 'codex-home|workspace|data_dir|HOME=/home/telex|/data' . --glob '!node_modules' --glob '!dist' --glob '!build'Repository: sadfun/telex
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Dockerfile ---'
cat -n Dockerfile
printf '%s\n' '--- docker/docs state and ownership guidance ---'
cat -n docs/docker.md
printf '%s\n' '--- application directory creation and persistence writes ---'
rg -n -C 4 'ensureDirectory|writeFile|appendFile|mkdir\(|rm\(|rename\(|dataDirectory|workspace|codexHome|outbound|conversations|automations' src/index.ts src --glob '*.ts' | head -n 500
printf '%s\n' '--- entrypoint behavior model ---'
python3 - <<'PY'
import os
import pathlib
import tempfile
with tempfile.TemporaryDirectory() as root:
data = pathlib.Path(root) / "data"
state = data / "telex"
workspace = data / "workspace"
nested = state / "codex-home" / "config.toml"
nested.parent.mkdir(parents=True)
workspace.mkdir(parents=True)
nested.write_text("existing")
os.chown(data, 0, 0)
os.chown(state, 0, 0)
os.chown(workspace, 0, 0)
os.chown(nested.parent, 0, 0)
os.chown(nested, 0, 0)
# This models chown telex:telex /data "$data_dir" "$workspace":
# only the named paths change; descendants do not.
target_uid = target_gid = os.getuid()
for path in (data, state, workspace):
os.chown(path, target_uid, target_gid)
print("data owner:", (data.stat().st_uid, data.stat().st_gid))
print("state owner:", (state.stat().st_uid, state.stat().st_gid))
print("workspace owner:", (workspace.stat().st_uid, workspace.stat().st_gid))
print("codex-home owner:", (nested.parent.stat().st_uid, nested.parent.stat().st_gid))
print("config owner:", (nested.stat().st_uid, nested.stat().st_gid))
PYRepository: sadfun/telex
Length of output: 34700
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant application startup and filesystem helpers ---'
cat -n src/index.ts | sed -n '45,115p'
cat -n src/shared/fs.ts
printf '%s\n' '--- persistent store write paths ---'
rg -n -C 5 'atomicWrite|writeFile|rename|mkdir' src --glob '*store*.ts' --glob 'src/automations/*.ts' --glob 'src/shared/fs.ts'
printf '%s\n' '--- deterministic entrypoint checks ---'
python3 - <<'PY'
from pathlib import Path
import re
text = Path("docker/entrypoint.sh").read_text()
match = re.search(r'^\s*chown\s+(.+?)\s+2>/dev/null\s+\|\|\s+true\s*$', text, re.M)
assert match, "The suppressed chown command was not found"
command = match.group(0)
print("suppressed chown:", command)
print("recursive flag present:", "-R" in match.group(1).split())
print("failure is ignored:", "|| true" in command)
print("runuser follows chown:", text.index("runuser") > text.index("chown"))
print("post-drop mkdir follows runuser:", text.index('mkdir -p "${data_dir}/codex-home"') > text.index("runuser"))
PYRepository: sadfun/telex
Length of output: 12309
Fail before dropping privileges when ownership setup is incomplete.
Line 14 ignores chown failures and changes ownership only on the named paths. A reused /data volume can retain root-owned descendants such as codex-home, toolchains, or workspace state. The telex process can then fail during startup or later writes.
Fail when chown fails and apply a scoped recursive ownership migration to ${data_dir} and ${workspace} before runuser, or validate all required descendants and exit.
🤖 Prompt for 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.
In `@docker/entrypoint.sh` around lines 10 - 18, The root branch of the entrypoint
currently ignores ownership failures and only updates top-level paths. In the
id-u check around mkdir/chown, recursively migrate ownership of ${data_dir} and
${workspace} to telex:telex, and remove the silent failure fallback so any chown
failure exits before the runuser transition.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds Slack as a second messaging channel next to Telegram, implementing the existing
MessagingChannelcontract over Socket Mode — no public URL, webhook, or reverse proxy needed, matching the Telegram long-polling model.Features
msg_too_long), with per-chunk delivery isolation.conversations.replies, capped) as context.files.uploadV2; inbound files (including Slack voice clips, which transcribe like Telegram voice messages) download through Slack's private URLs with host validation./telexslash command (Slack reserves bare/new-style messages) or@Bot /newmentions inside threads; conversation-scoped commands are rejected outside their thread with guidance./telex configrenders interactive Codex settings from Block Kit buttons (model, reasoning effort, service tier, approvals, sandbox, web search) throughCodexConfigServicewith optimistic versioning — a Slack counterpart of the Telegram Mini App, which stays Telegram-only.SLACK_ALLOWED_USER_IDS=*for every regular workspace member (verified viausers.infoagainst the bot's team — bots, deactivated accounts, guests, and Slack Connect outsiders stay rejected; cached with a TTL). An optionalSLACK_ADMIN_USER_IDSrestricts instance-wide commands (config/login/logout/reload/restart/update).<...>would mangle command text or ping@channel), and markdown tables rendered as aligned monospace blocks./datavolume (docs/docker.md explains the sandbox trade-off), plus a Compose example.envelope_idso redeliveries after a reconnect cannot double-execute commands or button clicks.Design notes
/telexhelp, config UI, admin gate) lives in the Slack channel via a dispatch seam in front of the shared handler.src/channels/progress.tsand re-exported, keeping the Telegram public API unchanged.docs/slack.mdwalks through app creation from a pasteable manifest, token collection, and usage.Tests cover formatting, routing, references, streaming/throttling, thread context, authorization, config UI, and env validation (307 passing);
npm run checkandnpm run buildare clean on top of currentmain.Summary by CodeRabbit
New Features
Documentation