Repository navigation
feat(nodes): add Discord bot source node (discord) - #1503
Merged
Merged
Conversation
Add a Discord bot source node (discord://, classType ["source"]) that connects a Discord bot to a pipeline over the Discord Gateway — the Discord counterpart to the telegram node. Routes incoming text and image/audio/ video/document attachments to the text/image/audio/video/tags lanes, and posts the pipeline's first answer back to the originating channel, message, or thread. Mirrors the Telegram endpoint architecture: scanObjects() -> WebServer (kept alive via self._server.run()) with the discord.py Gateway client launched as a background task from the async on_startup hook. Config: botToken (secure), guildIds, channelIds, ignoreBots, requireMention, replyMode (channel|reply|thread), showTyping, maxAttachmentBytes, sendResponses. Includes 2000-char reply chunking, rate-limit backoff, bot-loop prevention, and mention gating. Tests: 21 unit tests plus an env-gated live-gateway integration suite (skips unless DISCORD_BOT_TOKEN is set). Closes #1502 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mithileshgau
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
July 7, 2026 22:44
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdds a Discord Gateway source node that filters messages, routes text and attachments through pipeline lanes, and sends pipeline answers to Discord. It also adds configuration, event reporting, documentation, and offline and opt-in live test coverage. ChangesDiscord Source Node
Priority: ➖ Normal Estimated code review effort: 5 (Critical) | ~100 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant DiscordGateway
participant IEndpoint
participant Pipe
participant DiscordChannel
DiscordGateway->>IEndpoint: deliver message and attachments
IEndpoint->>Pipe: write message and attachment objects
Pipe-->>IEndpoint: return pipeline answer
IEndpoint->>DiscordChannel: send reply, channel message, or thread response
Merge Risk: ⚪ Minimal · up to No actionable issue is established for the Discord source at this head. It is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new integration applies meaningful channel, guild, bot, and mention controls. However, accepted messages can accumulate without a bounded processing budget, creating an availability risk when downstream processing slows or stalls. Conversation telemetry also requires appropriate access and retention policies. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The node implements the Discord Gateway source, lane routing, filtering, attachment handling, response modes, and lifecycle required by Full details: Docstring CoverageExplanation Docstring coverage is 34.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 722 functions across 14 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@nodes/src/nodes/discord/IEndpoint.py`:
- Around line 74-82: The class-level mutable defaults in IEndpoint are shared
across instances and should not be initialized as empty lists at class scope.
Update IEndpoint so _guild_ids and _channel_ids are created per instance,
ideally in the constructor or another per-instance initializer used by _run(),
and keep using those fields consistently in the existing endpoint flow. This
removes the fragile shared state while preserving the behavior expected by
_run() and any methods that read _guild_ids or _channel_ids.
- Around line 305-310: The attachment loop in IEndpoint._process_message keeps
calling _process_attachment even after a non-empty reply has already been found.
Update the `for attachment in message.attachments` logic to stop iterating once
`reply` is set, so later attachments are not downloaded or processed
unnecessarily. Keep the existing reply assignment behavior, but add an early
exit in the attachment-processing path to avoid extra pipeline runs.
- Around line 493-533: The fallback sentence splitting in _chunk_message can
still produce chunks larger than max_length when an individual re.split() part
is itself too long (for example, a long URL or unbroken text). Update the
_chunk_message logic so any oversized part is further split or hard-truncated
before appending, ensuring every returned chunk respects the Discord limit. Keep
the existing newline-then-sentence flow, but add a final guard in the sentence
branch so _send_response/_send_chunk never receive an over-limit chunk.
- Around line 553-560: The retry handling in IEndpoint._send_chunk is catching
the wrong exception type, so it cannot read Discord’s actual backoff delay.
Update the except block that currently catches discord.HTTPException to catch
the rate-limit exception that exposes retry timing (discord.RateLimited), then
use its delay value instead of the fallback 1.0 when logging and sleeping before
retrying the send.
In `@nodes/src/nodes/discord/README.md`:
- Around line 17-25: The Discord README uses the internal lane name “tags” for
the document output, but the docs should match the UI-facing label. Update the
lane description in the Discord node documentation to refer to the output as
“Data” (or `data` where inline code is used) instead of `tags`, and ensure any
mention of the `_source` lane mapping in the README stays consistent with the
user-visible lane names shown in the UI.
In `@nodes/src/nodes/discord/requirements.txt`:
- Around line 1-4: Remove the unused unpinned dependencies from the Discord node
requirements and keep only the packages actually used by IEndpoint.py and
related Discord code; the issue is that requests and fastapi are likely leftover
entries and are pulling vulnerable transitive versions. If either dependency is
truly required, pin it to a patched version and update the related transitive
constraints (such as starlette, idna, and pydantic) accordingly.
In `@nodes/test/discord/test_discord.py`:
- Around line 307-340: The three tests are tautological and do not exercise the
real Discord gating logic. Update `test_attachment_size_validation`,
`test_guild_id_filtering`, and `test_channel_id_filtering` to invoke the
production paths in `IEndpoint._on_message` and `_process_attachment` rather
than comparing literals. Use mocked message/attachment objects to verify that
oversized attachments are rejected and that guild/channel allowlist checks
actually control processing. Ensure the assertions validate the behavior of the
endpoint methods, not just Python membership or arithmetic.
- Around line 130-135: The _chunk_message() path in test_discord.py still lets a
no-break string longer than Discord’s 2000-character limit return as one
oversized chunk. Update the chunking logic in _chunk_message() to apply a hard
fallback split to any remaining segment that is still over 2000 characters
before returning, so _send_response() never receives an invalid payload. Adjust
test_over_limit_by_one to assert the message is split and that every returned
chunk length stays within the 2000-character limit.
- Around line 71-106: The test helper `_chunk_message` is duplicating the
production Discord chunking logic instead of using the real implementation,
which can cause test drift. Update the tests in `test_discord.py` to call the
production helpers from `IEndpoint` (or extract shared static helpers used by
both tests and production), and ensure the MIME detection path also matches
`IEndpoint._guess_media_type` by preserving the
`content_type.split(';')[0].strip()` behavior.
In `@nodes/test/discord/test_integration.py`:
- Around line 127-155: The cleanup logic in test_send_message and the related
Discord integration tests should not live after assertions inside the same try
block, because an AssertionError will skip deletion and leave test artifacts
behind. Refactor test_send_message, test_message_reply, test_message_thread,
test_long_message_chunking, and test_rate_limit_handling so sent
messages/threads are always cleaned up in a finally block (or equivalent
guaranteed cleanup path), while keeping the existing discord.Forbidden handling
for permission issues.
- Around line 345-413: The TestDiscordNodeConfiguration tests are duplicating
message-filtering logic instead of validating the real Discord node behavior.
Refactor these cases to call the actual gating path in IEndpoint._on_message (or
extract the guild/channel/mention/bot checks into a small helper in IEndpoint.py
and test that directly), using discord.Message-like fixtures so bugs in the real
config handling and allowlist/mention conditions are actually caught.
- Around line 290-326: The test_rate_limit_handling case in test_integration.py
only exercises sending a few messages with a short sleep and does not prove any
rate-limit behavior. Update test_rate_limit_handling to either explicitly
trigger or mock a rate-limit/429 scenario and assert the client retries/backoffs
correctly, or rename/retarget the test so the name and docstring match the
actual behavior being verified. Use the existing test_rate_limit_handling async
test and its channel.send flow as the place to add the real validation.
- Around line 244-288: The long-message test is using a hand-written splitter
instead of the real Discord chunking implementation, so it is not covering the
shipped behavior. Update test_long_message_chunking in test_integration.py to
import and call the actual response-chunking logic from IEndpoint.py (the
“Response chunking and reply sending” layer) rather than building chunks
locally, and keep the assertions focused on the real chunking output and send
behavior.
- Around line 34-44: The bot-intent setup in test_bot is duplicated and does not
match the production path, so the test is not validating the same intent
configuration used by IEndpoint._startup. Refactor the intent and bot
construction into a shared helper or reuse the same endpoint startup path in
test_bot and test_bot_intents, so both the fixture and the assertion exercise
the exact bot setup from the endpoint.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 15a6d92b-2ea0-458f-96e3-984c9d8b0c96
⛔ Files ignored due to path filters (1)
nodes/src/nodes/discord/discord.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
nodes/src/nodes/discord/IEndpoint.pynodes/src/nodes/discord/IGlobal.pynodes/src/nodes/discord/IInstance.pynodes/src/nodes/discord/README.mdnodes/src/nodes/discord/__init__.pynodes/src/nodes/discord/requirements.txtnodes/src/nodes/discord/services.jsonnodes/test/discord/__init__.pynodes/test/discord/test_discord.pynodes/test/discord/test_integration.py
- Remove nodes/test/discord/test_integration.py: it imports the discord.py library at module top-level, which requires the (uninstalled-in-CI) dependency and a live DISCORD_BOT_TOKEN, and the package name collides with the node's own `discord` package during collection. It cannot run in CI. The pure-logic unit tests in test_discord.py remain. - Open services.json with encoding='utf-8' in the schema tests so they pass under CI's ASCII locale. - Replace the non-ASCII em dash in services.json with a hyphen. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Extract pure helpers (chunk_message, guess_media_type) into text_utils.py with no discord.py dependency, so unit tests exercise the real production code instead of reimplementing it, and can run in CI without discord.py. - chunk_message now hard-splits any single token/sentence longer than the 2000-char limit, so every emitted chunk is guaranteed to fit (previously an unbroken long token could produce an oversized chunk Discord would reject). - Stop processing further attachments once a non-empty answer is found. - Handle discord.RateLimited (which carries retry_after) instead of reading a nonexistent retry_after off discord.HTTPException. - Declare _guild_ids/_channel_ids as annotations only to avoid mutable list defaults shared at class scope; they are set per-instance in _run(). - requirements.txt: keep only discord.py (requests/fastapi/aiohttp are not imported directly and flagged unpinned/vulnerable by OSV). - README: use the UI-facing lane name "Data" for the document lane. - Rewrite test_discord.py to test the real helpers and services.json schema; drop tautological config/filtering tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
nodes/src/nodes/discord/README.md (1)
78-87: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the "10 attachments" note to reflect the early-exit behavior.
IEndpoint._process_messagenow stops iterating attachments after the first non-empty pipeline answer (break), so the claim "each is downloaded and routed to the pipeline independently" is no longer accurate for all 10 attachments — only attachments up to and including the first one that yields a non-empty answer are processed.📝 Proposed wording
-- **10 attachments per message**: If a user sends 10 images, each is downloaded and routed to the pipeline independently. +- **10 attachments per message**: Up to 10 attachments are supported; attachments are processed in order and processing stops once the first non-empty pipeline answer is produced.🤖 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 `@nodes/src/nodes/discord/README.md` around lines 78 - 87, The “10 attachments per message” note in the Discord README is outdated because IEndpoint._process_message now exits the attachment loop after the first non-empty pipeline answer. Update that bullet so it no longer says every attachment is independently routed; instead, describe that attachments are processed in order and iteration stops once an attachment produces a non-empty answer, so only attachments up to that point are handled.nodes/src/nodes/discord/IEndpoint.py (3)
251-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting pure gating predicates for testability.
The guild/channel allowlist, mention-gate, and bot-self/loop checks are only exercisable through
discord.Message-shaped objects, which the test suite intentionally avoids importing (per PR objectives, to keepdiscord.pyout of CI test collection). This mirrors the exact problemtext_utils.pysolved for chunking/MIME logic. Consider extracting the boolean predicates (e.g.,is_allowed_guild(guild_id, allowlist),is_allowed_channel(channel_id, allowlist),attachment_too_large(size, max_size)) into a dependency-free helper module so this critical filtering logic can be unit-tested the same waychunk_message/guess_media_typeare.🤖 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 `@nodes/src/nodes/discord/IEndpoint.py` around lines 251 - 282, Extract the message gating logic from _on_message into dependency-free helper predicates so it can be unit-tested without importing discord.py. Move the bot-self check, bot-ignore check, guild allowlist, channel allowlist, and mention gate into pure helpers with simple inputs like IDs/flags, then have _on_message call those helpers before creating the task. Keep _on_message focused on wiring and task scheduling, and place the reusable predicates alongside the other testable helpers used by IEndpoint.
459-488: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFailed retry silently drops the rest of the chunk and continues to the next one.
In
_send_response, if_send_chunkfails both the initial attempt and the post-RateLimitedretry (except Exception as e2: debug(...)), the loop moves on to the next chunk without any indication to the user that a piece of the reply is missing. Forreply_mode == 'thread', this can also leavethreadunset/stale if the failure happened before thread creation, causing later chunks to retry creating the thread. Users end up with a spliced, confusing partial answer and no error signal.Consider breaking out of the chunk loop (or raising) on unrecoverable send failures instead of silently continuing, so partial/garbled responses aren't delivered without any indication.
🤖 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 `@nodes/src/nodes/discord/IEndpoint.py` around lines 459 - 488, In _send_response, a failed _send_chunk retry after discord.RateLimited is being swallowed and the loop continues, which can silently drop part of the reply and leave thread state inconsistent. Update the retry/error handling around _send_chunk so that an unrecoverable send failure in _send_response stops processing further chunks (or re-raises) instead of continuing; ensure the thread variable used by _send_chunk does not get reused after a failed initial thread creation.
297-316: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winProcess attachments regardless of the text reply
if not reply and message.attachments:skips_process_attachment/_run_binary_pipelinewhenever text returns any answer, so captioned images/audio/video/documents never get routed. Keep attachment processing independent of whether the text lane produced a reply; only the sent-back answer should stay “first non-empty wins.”🤖 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 `@nodes/src/nodes/discord/IEndpoint.py` around lines 297 - 316, The attachment handling in _process_message currently depends on the text reply being empty, which prevents _process_attachment and _run_binary_pipeline from running for captioned uploads. Update the logic in IEndpoint._process_message so attachments are always processed independently of the text pipeline, using the first non-empty response from either path as the final reply, while keeping _send_response unchanged for the chosen output.
🤖 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.
Outside diff comments:
In `@nodes/src/nodes/discord/IEndpoint.py`:
- Around line 251-282: Extract the message gating logic from _on_message into
dependency-free helper predicates so it can be unit-tested without importing
discord.py. Move the bot-self check, bot-ignore check, guild allowlist, channel
allowlist, and mention gate into pure helpers with simple inputs like IDs/flags,
then have _on_message call those helpers before creating the task. Keep
_on_message focused on wiring and task scheduling, and place the reusable
predicates alongside the other testable helpers used by IEndpoint.
- Around line 459-488: In _send_response, a failed _send_chunk retry after
discord.RateLimited is being swallowed and the loop continues, which can
silently drop part of the reply and leave thread state inconsistent. Update the
retry/error handling around _send_chunk so that an unrecoverable send failure in
_send_response stops processing further chunks (or re-raises) instead of
continuing; ensure the thread variable used by _send_chunk does not get reused
after a failed initial thread creation.
- Around line 297-316: The attachment handling in _process_message currently
depends on the text reply being empty, which prevents _process_attachment and
_run_binary_pipeline from running for captioned uploads. Update the logic in
IEndpoint._process_message so attachments are always processed independently of
the text pipeline, using the first non-empty response from either path as the
final reply, while keeping _send_response unchanged for the chosen output.
In `@nodes/src/nodes/discord/README.md`:
- Around line 78-87: The “10 attachments per message” note in the Discord README
is outdated because IEndpoint._process_message now exits the attachment loop
after the first non-empty pipeline answer. Update that bullet so it no longer
says every attachment is independently routed; instead, describe that
attachments are processed in order and iteration stops once an attachment
produces a non-empty answer, so only attachments up to that point are handled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0d027382-5da1-4fca-b220-6b4902d2fc60
📒 Files selected for processing (5)
nodes/src/nodes/discord/IEndpoint.pynodes/src/nodes/discord/README.mdnodes/src/nodes/discord/requirements.txtnodes/src/nodes/discord/text_utils.pynodes/test/discord/test_discord.py
💤 Files with no reviewable changes (1)
- nodes/src/nodes/discord/requirements.txt
- Process attachments independently of the text pipeline so captioned uploads are ingested; the first non-empty answer (text first, then attachments in order, stopping at the first that answers) becomes the reply. - _send_response now abandons the remaining chunks on an unrecoverable send failure instead of silently continuing and dropping part of the reply. - Extract the message gating (self/bot/guild/channel/mention) into a pure should_process_message() predicate in text_utils.py and unit-test it; _on_message just wires primitives and schedules the task. - README: correct the attachment note to describe in-order processing that stops at the first attachment answer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nodes/src/nodes/discord/README.md (1)
103-103: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMIME type detection order is reversed in the documentation.
The README says the node guesses from the file extension first and "falls back to Discord's reported
content_type." The actual code inguess_media_type()does the opposite: it returnscontent_typeimmediately when present (with;parameters stripped), and only falls back to filename-extension lookup whencontent_typeis empty.📝 Proposed fix
-The node guesses the MIME type from the file extension (e.g., `.pdf` → `application/pdf`) and falls back to Discord's reported `content_type` if available. Unknown extensions default to `application/octet-stream`. +The node uses Discord's reported `content_type` when available (stripping any `;` parameters) and falls back to guessing from the file extension (e.g., `.pdf` → `application/pdf`) otherwise. Unknown extensions default to `application/octet-stream`.🤖 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 `@nodes/src/nodes/discord/README.md` at line 103, The README description of MIME detection in guess_media_type() is reversed: it currently says the node checks the file extension first and then falls back to Discord’s content_type, but the implementation prefers content_type (with any parameters stripped) before using the filename extension. Update the documentation text in the Discord node README to match the actual behavior of guess_media_type(), including the correct precedence order and the fallback to application/octet-stream for unknown types.
🤖 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.
Outside diff comments:
In `@nodes/src/nodes/discord/README.md`:
- Line 103: The README description of MIME detection in guess_media_type() is
reversed: it currently says the node checks the file extension first and then
falls back to Discord’s content_type, but the implementation prefers
content_type (with any parameters stripped) before using the filename extension.
Update the documentation text in the Discord node README to match the actual
behavior of guess_media_type(), including the correct precedence order and the
fallback to application/octet-stream for unknown types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92886b16-f61e-45f9-8264-a9756b1b6d67
📒 Files selected for processing (4)
nodes/src/nodes/discord/IEndpoint.pynodes/src/nodes/discord/README.mdnodes/src/nodes/discord/text_utils.pynodes/test/discord/test_discord.py
guess_media_type() prefers Discord's reported content_type (parameters stripped) and only falls back to the file extension; the README had the order reversed. Fix the description to match, keeping the octet-stream default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
asclearuc
requested changes
Jul 17, 2026
asclearuc
left a comment
asclearuc
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for this, @mithileshgau — solid node overall: it mirrors the Telegram source node well, the pure logic is split into text_utils.py and unit-tested, and the token is handled safely (marked secure, only the last 6 chars shown in the monitor).
Requesting changes on one bug before merge:
- Attachments after the first answer are dropped (
IEndpoint.py:315-320) — see inline. One Discord message can carry up to 10 attachments; the earlybreakmeans later ones never enter the pipeline. This is input loss and is the blocking item.
Non-blocking fixes, also inline:
- README calls the document lane
Data; the real lane istags(README.md:17, 25, 98). - The generated schema block is out of sync with
services.json(README.md:164) — rerunnodes:docs-generate. - The
except discord.RateLimitedbranch is effectively dead and the docs over-claim what it does (IEndpoint.py:488).
One more, not tied to a line: the PR description mentions a live-gateway integration suite, but only the pure-logic tests are in the diff. Please add it or adjust the description.
Nice work — once the attachment loop ingests every attachment, this is good to go.
Contributor
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
…docs - Remove the early `break` in the attachment loop so a source node ingests every attachment in a multi-attachment message (up to 10); still reply with only the first non-empty answer. The break silently dropped later attachments (no download, routing, or monitorCompleted). - Revert README lane name `Data` -> `tags` to match services.json and the telegram/webhook/core convention (no `Data` lane exists). - Clarify that discord.py handles 429s internally; the RateLimited branch is defensive (only fires with max_ratelimit_timeout, which is unset). - Regenerate stale GENERATED:PARAMS em-dash back to the services.json hyphen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…iscord-source-node
Collaborator
Author
|
Addressed the latest review feedback in 9d276e3 (and merged latest Attachment loop dropped data (@asclearuc)Removed the early Lane name
|
asclearuc
requested changes
Aug 5, 2026
asclearuc
left a comment
asclearuc
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @mithileshgau — this round cleanly resolves all four of my earlier findings (attachments are now fully ingested, the README lane names and the generated schema block are correct, and the RateLimited branch is honestly documented as defensive), and extracting the gating logic into a tested should_process_message predicate was the right move.
Request changes
The one remaining blocker is test coverage; details below, plus one non-blocking README nit inline.
MUST fix (blocking) — no test covers IEndpoint's fetch/emit logic. nodes/test/discord/test_discord.py exercises only the pure helpers (should_process_message, chunk_message, guess_media_type) and the services.json schema. The routing/selection logic in _process_message and _process_attachment (IEndpoint.py:286-375) is entirely untested — including the exact behavior that resolved my previous blocker: ingest every attachment (up to 10) while replying with only the first non-empty answer, and the attachment.size > _max_attachment_bytes skip. There is currently no regression guard preventing the dropped-attachment bug from returning. This is feasible in CI: the sibling node already tests its endpoint the same way in nodes/test/telegram/test_endpoint.py — build the instance with IEndpoint.__new__(IEndpoint), stub _run_text_pipeline / _process_attachment (or _run_binary_pipeline), pass MagicMock message/attachment objects, and inject a sys.modules['discord'] stub the way that file stubs aiohttp (monkeypatch.setitem(sys.modules, 'aiohttp', MagicMock())). Please add at least: (a) an oversized attachment → _process_attachment returns '' and never calls the pipeline; (b) a message with text + N attachments → all N attachments are processed and only the first non-empty answer is sent.
Note (non-blocking, no action needed): my earlier RateLimited comment thread on IEndpoint.py:488 is still anchored to head but is resolved — the added comment now correctly documents the branch as defensive (it only fires when max_ratelimit_timeout is set, which this bot does not). Not re-raising.
…rom deep review Critical: - _run no longer constructs its own WebServer bound to --data_port. node.py already binds a shared WebServer on that port for every subprocess, so the second bind hit EADDRINUSE (and wired the target onto the wrong server) — the node could not start under EaaS. Now mirrors the Telegram node: register on the shared server and drive _startup/_shutdown on the shared server_loop, blocking scanObjects() on a shutdown event. Correctness: - _run_with_optional_typing now runs the pipeline exactly once; a typing- indicator enter/exit failure no longer re-executes it (double ingestion). - require_mention no longer satisfied by @everyone/@here (use direct message.mentions, not mentioned_in). - guildIds/channelIds coerced via _as_str_list so a scalar string no longer becomes a per-character allowlist; non-dict 'discord' config block no longer crashes _run. - guess_media_type lowercases the reported content type (mixed-case IMAGE/PNG routed correctly) and falls back to the extension when it normalizes empty. Docs: - Attachments download via discord.py's Attachment.read(), not aiohttp directly (aiohttp is transitive); drop the misleading "concurrently" wording. Tests: - New test_process_message.py (import-guarded, py3.10+/discord.py): attachment ingest-all + first-answer, exactly-once typing, allowlist coercion, and the @everyone gating fix. New MIME normalization cases in test_discord.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
Resolved the new review comment and ran a deep multi-agent review (two independent reviewers — an adversarial correctness pass and an independent second opinion) plus added targeted tests. Pushed in 7fed17b. Review comment resolved
Critical — node could not start under EaaS (both reviewers, independently)
Other correctness fixes
Tests
Known follow-ups (deliberately out of scope for this round)Flagged by the review but larger design changes / edge cases, better as separate work: backpressure / bounded concurrency for bursts of large attachments; shutdown-drain race hardening; thread reply-mode for DMs and messages already inside threads; thread-handle reuse on send retry; retry for transient non-429 send errors; lossless whitespace handling in |
Collaborator
|
Two shared surfaces are still missing. @joshuadarron blocked #2141 on 2026-09-01 for exactly these, so they will come up:
The generated params block in |
krishhgg
previously requested changes
Sep 14, 2026
Collaborator
There was a problem hiding this comment.
The post-review endpoint tests address the prior requested cases, but head 7fed17ba still has release-blocking lifecycle and security behavior:
- A missing token returns normally from
_startup(); invalid credentials or missing privileged intents are swallowed by_bot_runner(). In all three cases_run()then waits forever with no functioning bot and no engine-visible failure. - Pipeline-generated replies do not disable Discord mention parsing, so model output can trigger real user/role/everyone mentions.
- The behavioral test module can skip wholesale when
discord.pyis unavailable, leaving these paths untested in a clean environment. - Current
developalso requires porting the credential catalog and node documentation metadata/schema rather than relying on the older generated artifacts.
Please propagate terminal Gateway failures through the source lifecycle, disable allowed mentions on outbound content, make the suite execute deterministically in CI, and rebase/update the current credentials/docs surfaces.
For your agents
Goal: Ensure the Discord source either becomes operational or fails the pipeline promptly, and ensure model-generated output cannot create unintended Discord mentions.
Relevant context: _run() starts _startup() and then blocks on _shutdown_event. A missing token currently returns normally, while login and privileged-intent failures are caught inside the background bot task. All three paths leave the source alive indefinitely with no functioning Gateway client.
Inspect and change:
nodes/src/nodes/discord/IEndpoint.py::_startup,_bot_runner, and_run: propagate terminal startup/Gateway failures into the engine-visible source lifecycle. Avoid a wait state with no bot task capable of receiving messages.- Outbound reply/channel/thread sends: pass a restrictive
discord.AllowedMentionspolicy so arbitrary pipeline text cannot ping users, roles or everyone. - Test loading: make the behavioral suite run in clean CI even if
discord.pyis not preinstalled; do not allow the module to skip wholesale. - Rebase and update the current credentials catalog, README schema/layout and
services.jsondocumentation URL instead of restoring obsolete generated artifacts.
Acceptance criteria: Missing/invalid credentials and missing Message Content Intent fail promptly with actionable status; successful startup still blocks until controlled shutdown; outbound @everyone, role and user syntax renders without notifying anyone; and all lifecycle tests execute in ordinary CI.
Verification: Add deterministic tests for missing token, login failure, privileged-intent failure, cancellation/shutdown, all three reply modes and allowed mentions. Retain the existing oversized-attachment and multi-attachment endpoint regressions.
- Add a `discord` row to the Sources table in docs/README-nodes.md. - Add a Discord node line under [Unreleased] / Added in CHANGELOG.md. Addresses review feedback that both shared surfaces were missing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
Thanks @nihalnihalani — both shared surfaces are now added in 20c0a8d:
Re-requesting review from @asclearuc since the earlier blocking ask (the |
…iscord-source-node # Conflicts: # CHANGELOG.md # docs/README-nodes.md
…d develop conformance Addresses kgarg2468's review: Lifecycle: terminal Gateway failures now fail the source promptly instead of idling forever with a dead bot. A missing token raises from _startup; login failure, missing Message Content Intent, an unexpected disconnect, or a generic gateway error record a fatal error and unblock _run, which re-raises so the engine marks the source failed. Successful startup still blocks until controlled shutdown; an intentional close during _shutdown is not treated as a failure. Security: all outbound sends (reply, thread, channel) pass discord.AllowedMentions.none() so model-generated text can never ping users, roles, @here or @everyone. Tests: the behavioral suite now stubs discord in sys.modules and loads the node as a synthetic package, so it runs deterministically in clean CI without discord.py installed and without a version skip (node annotations switched to Optional[...] so it imports on 3.10-). Added lifecycle tests (missing token, login failure, privileged-intent failure, unexpected close, cancellation, shutdown-marks-closing), allowed-mentions tests for all three reply modes plus thread reuse, and an oversized-attachment regression. Retained the multi-attachment ingest-all regression. Develop conformance: - services.json: add "documentation" URL. - README.md: restructure to the current node README schema (top-level Lanes table, drop the hand-written field table, fold sections under Notes); passes scripts/validate-node-readme.py. - credentials.json: register the discord credential (nodes:credentials-generate) so nodes:credentials-check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Any NUL byte meant "binary", so UTF-16 files (Windows Notepad "Unicode", PowerShell 5.1 redirects) were never read as text: with merging off they went to the binary lanes, with merging on they were dropped with only a debug line. A UTF-8 BOM also survived into the text. The decode now checks for a UTF-16 BOM first, otherwise decodes as utf-8-sig, and only then treats a NUL as binary content. In merge mode a text-like file that is still binary becomes its own binary lane object (reusing the bytes already downloaded) instead of being dropped. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mimetypes.guess_type also reads the Windows registry and /etc/mime.types, so the extension fallback routed the same file differently per host: on Windows .csv became application/vnd.ms-excel and .ts a video type (it is video/mp2t on macOS too). The fallback now asks a module-level mimetypes.MimeTypes(), which CPython fills from its built-in defaults only; the node's own extension table still answers first. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An entry in guildIds, channelIds or requireMentionChannelIds that is not plain ASCII digits (a pasted <#123>, a channel or server name) matched nothing, so the mention gate failed open or the bot answered nothing. The start now fails naming the field and the entry, and suggests the id inside a pasted mention. The mention allowlists and allowedBotIds use the same digit rule, so '123²' no longer reaches int() in every send. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Gateway keeps delivering reactions while _shutdown waits for in-flight messages, and each one still opened a pipe. _on_raw_reaction now returns when _closing is set, and checks again just before the emit. The reaction authorIsBot now uses the same _reactor_is_bot answer as the gate, so a removal by an allowlisted bot is reported as a bot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reply with a code fence anywhere goes through the fenced chunker, which only broke at a newline in the second half of the window and otherwise cut by character count, even in prose outside the code block. Outside an open fence it now falls back to the last sentence end, then the last whitespace, in the second half before cutting by count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maxAttachmentBytes of zero or below skipped every attachment with only a debug line; it is now clamped to at least 1 and the schema declares minimum 1. textAttachmentExtensions entries typed without a dot (md, json) never matched os.path.splitext; each entry is now normalised to a lowercase extension with one leading dot, and blank entries are dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…engine _as_int caught TypeError and ValueError but not OverflowError, so a number setting of inf or 1e400 failed _run with an unrelated traceback; it now falls back to the default. The ten boolean settings were used raw, so a string-like 'false' read as True; a new _as_bool reads real bools as-is and true/false, 1/0, yes/no, on/off strings in any case, else the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tags lane also carries the reaction, no_reply and outbound event objects, and events have their own entry URL and name; the README now says so. The no_reply reasons are listed (no_answer, send_failed, shutdown, or an error message clipped to 200 characters), and so is what happens when the question is deleted before the answer is posted. Configuration headings use the field titles, each paragraph sits under the field it is about, and the broadcast section moves under Notes. The Developer Portal steps use Reset Token, the reactions intent is described as one the node turns on itself, and the MIME fallback examples are .avi and .bmp, which the node's own table does not list. In services.json, guildIds and channelIds default to [] and the text-attachment and emitNoReply descriptions match the behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… private bot Several Args sections left out parameters the functions take (_on_message wait, _process_attachment meta and attachment_index, _run_text_pipeline meta, object_name and attachment_id, _run_binary_pipeline message_id, attachment_index and meta, should_process_message parent_channel_id and allowed_bot_ids). Comments and docstrings no longer point at functions of a bot that is not in this repo; the sentences explaining the behaviour stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The live README now lists the variables L3 and L4 need on the engine and says ROCKETRIDE_DISCORD_E2E_NO_SUCH_TOKEN must stay unset, and asks for Python 3.10+. engine_min.pipe gets a fresh project_id (the SDK's use() contract carries it at the top level) and a short generic assistant prompt with no escalation line. E05 tested that escalation prompt, which belongs to a follow-up change, and E06 was a skipping placeholder; both are removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated by nodes:docs-generate for the new bounds and list defaults and the updated textAttachmentExtensions and emitNoReply descriptions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…by kind A reaction emit already under way was not tracked, so shutdown did not wait for it and tore the endpoint down beneath it. The emit is now a tracked task, like a message handler. The second closing check before the emit could never fire (nothing can change the flag between the two checks) and its test only passed by setting the flag from a mock; both are gone. A pasted mention in an id list suggested the digits inside it even when they were the id of something else (a role in a channel list, a channel in the server list). The hint now gives the digits only for the right kind of mention and otherwise says what was pasted. Also documents the arguments of _emit_event_pipeline and the return of _emit_no_reply_event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
asclearuc
previously approved these changes
Oct 6, 2026
asclearuc
left a comment
asclearuc
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @mithileshgau for another thorough round. All six comments from my last review are fixed, with tests, and you also took care of several smaller items I had left out (numeric-only ids, reactions at shutdown, word-boundary splits in fenced replies, boolean and overflow coercion). The 350 discord unit tests pass locally at 7e4875ce.
Approve
… word When a chunk window ended inside or just after a code-fence opener line, chunk 1 ended with a split language name or an empty code block. A block whose opener line and first code line do not both fit now starts the next chunk, and the newline search never cuts right after an opener line. A reply with no fence also hard-split a line longer than the limit in the middle of a word. It is now cut at the last sentence end or whitespace in the second half of the window, by character count only when there is none. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The node had its own _as_bool, which duplicated parse_bool in ai.common.utils. Every boolean setting now goes through the shared helper. The engine passes JSON booleans as real bool values, so the tests that fed string-like proxies to a boolean are replaced with tests for real bools and plain strings. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A bot token or secret ${ROCKETRIDE_*} value pasted into an id list was
echoed in full by the start error, the task warnings and the debug log.
Every message about a list entry now goes through one helper that shows
a mention whole and anything else longer than 12 characters as its first
12 characters followed by an ellipsis.
An id is also bounded to a 64-bit snowflake (at most 20 digits, below
2**64), so two ids pasted together are rejected as non-numeric on the
same fatal or warning path as any other bad entry.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maxAttachmentBytes was floored at 1, but a 1-byte cap skips every real attachment just as 0 did. A value of 0 or below now means "not set" and falls back to the 25 MB default, matching config_int in ai.common.utils; the value is still clamped to at most 100 MB. The schema keeps its minimum of 1. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
asclearuc
approved these changes
Oct 6, 2026
asclearuc
left a comment
asclearuc
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @mithileshgau for closing out the last four points so quickly. The fence-opener cut, the shared parse_bool, the clipped id messages and the attachment cap are all fixed, each with a test, and CI is green on Linux, macOS and Windows.
Approve
There are no new inline comments this round.
mithileshgau
pushed a commit
that referenced
this pull request
Oct 7, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mithileshgau
pushed a commit
that referenced
this pull request
Oct 7, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mithileshgau
added a commit
that referenced
this pull request
Oct 8, 2026
* feat(nodes): add Discord bot source node (discord)
Add a Discord bot source node (discord://, classType ["source"]) that
connects a Discord bot to a pipeline over the Discord Gateway — the Discord
counterpart to the telegram node. Routes incoming text and image/audio/
video/document attachments to the text/image/audio/video/tags lanes, and
posts the pipeline's first answer back to the originating channel, message,
or thread.
Mirrors the Telegram endpoint architecture: scanObjects() -> WebServer
(kept alive via self._server.run()) with the discord.py Gateway client
launched as a background task from the async on_startup hook.
Config: botToken (secure), guildIds, channelIds, ignoreBots,
requireMention, replyMode (channel|reply|thread), showTyping,
maxAttachmentBytes, sendResponses. Includes 2000-char reply chunking,
rate-limit backoff, bot-loop prevention, and mention gating.
Tests: 21 unit tests plus an env-gated live-gateway integration suite
(skips unless DISCORD_BOT_TOKEN is set).
Closes #1502
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(nodes): drop Discord live-gateway integration test, fix CI encoding
- Remove nodes/test/discord/test_integration.py: it imports the discord.py
library at module top-level, which requires the (uninstalled-in-CI)
dependency and a live DISCORD_BOT_TOKEN, and the package name collides
with the node's own `discord` package during collection. It cannot run in
CI. The pure-logic unit tests in test_discord.py remain.
- Open services.json with encoding='utf-8' in the schema tests so they pass
under CI's ASCII locale.
- Replace the non-ASCII em dash in services.json with a hyphen.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(nodes): address Discord node review feedback
- Extract pure helpers (chunk_message, guess_media_type) into text_utils.py
with no discord.py dependency, so unit tests exercise the real production
code instead of reimplementing it, and can run in CI without discord.py.
- chunk_message now hard-splits any single token/sentence longer than the
2000-char limit, so every emitted chunk is guaranteed to fit (previously an
unbroken long token could produce an oversized chunk Discord would reject).
- Stop processing further attachments once a non-empty answer is found.
- Handle discord.RateLimited (which carries retry_after) instead of reading a
nonexistent retry_after off discord.HTTPException.
- Declare _guild_ids/_channel_ids as annotations only to avoid mutable list
defaults shared at class scope; they are set per-instance in _run().
- requirements.txt: keep only discord.py (requests/fastapi/aiohttp are not
imported directly and flagged unpinned/vulnerable by OSV).
- README: use the UI-facing lane name "Data" for the document lane.
- Rewrite test_discord.py to test the real helpers and services.json schema;
drop tautological config/filtering tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(nodes): address second Discord review round
- Process attachments independently of the text pipeline so captioned uploads
are ingested; the first non-empty answer (text first, then attachments in
order, stopping at the first that answers) becomes the reply.
- _send_response now abandons the remaining chunks on an unrecoverable send
failure instead of silently continuing and dropping part of the reply.
- Extract the message gating (self/bot/guild/channel/mention) into a pure
should_process_message() predicate in text_utils.py and unit-test it;
_on_message just wires primitives and schedules the task.
- README: correct the attachment note to describe in-order processing that
stops at the first attachment answer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(nodes): correct Discord MIME-detection precedence in README
guess_media_type() prefers Discord's reported content_type (parameters
stripped) and only falls back to the file extension; the README had the
order reversed. Fix the description to match, keeping the octet-stream default.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(nodes): address Discord review — ingest all attachments, correct docs
- Remove the early `break` in the attachment loop so a source node ingests
every attachment in a multi-attachment message (up to 10); still reply with
only the first non-empty answer. The break silently dropped later
attachments (no download, routing, or monitorCompleted).
- Revert README lane name `Data` -> `tags` to match services.json and the
telegram/webhook/core convention (no `Data` lane exists).
- Clarify that discord.py handles 429s internally; the RateLimited branch is
defensive (only fires with max_ratelimit_timeout, which is unset).
- Regenerate stale GENERATED:PARAMS em-dash back to the services.json hyphen.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(nodes): Discord node — use shared WebServer + correctness fixes from deep review
Critical:
- _run no longer constructs its own WebServer bound to --data_port. node.py
already binds a shared WebServer on that port for every subprocess, so the
second bind hit EADDRINUSE (and wired the target onto the wrong server) — the
node could not start under EaaS. Now mirrors the Telegram node: register on
the shared server and drive _startup/_shutdown on the shared server_loop,
blocking scanObjects() on a shutdown event.
Correctness:
- _run_with_optional_typing now runs the pipeline exactly once; a typing-
indicator enter/exit failure no longer re-executes it (double ingestion).
- require_mention no longer satisfied by @everyone/@here (use direct
message.mentions, not mentioned_in).
- guildIds/channelIds coerced via _as_str_list so a scalar string no longer
becomes a per-character allowlist; non-dict 'discord' config block no longer
crashes _run.
- guess_media_type lowercases the reported content type (mixed-case IMAGE/PNG
routed correctly) and falls back to the extension when it normalizes empty.
Docs:
- Attachments download via discord.py's Attachment.read(), not aiohttp directly
(aiohttp is transitive); drop the misleading "concurrently" wording.
Tests:
- New test_process_message.py (import-guarded, py3.10+/discord.py): attachment
ingest-all + first-answer, exactly-once typing, allowlist coercion, and the
@everyone gating fix. New MIME normalization cases in test_discord.py.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(nodes): list Discord node in nodes catalog and CHANGELOG
- Add a `discord` row to the Sources table in docs/README-nodes.md.
- Add a Discord node line under [Unreleased] / Added in CHANGELOG.md.
Addresses review feedback that both shared surfaces were missing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(nodes): Discord lifecycle failure propagation, mention safety, and develop conformance
Addresses kgarg2468's review:
Lifecycle: terminal Gateway failures now fail the source promptly instead of
idling forever with a dead bot. A missing token raises from _startup; login
failure, missing Message Content Intent, an unexpected disconnect, or a generic
gateway error record a fatal error and unblock _run, which re-raises so the
engine marks the source failed. Successful startup still blocks until controlled
shutdown; an intentional close during _shutdown is not treated as a failure.
Security: all outbound sends (reply, thread, channel) pass
discord.AllowedMentions.none() so model-generated text can never ping users,
roles, @here or @everyone.
Tests: the behavioral suite now stubs discord in sys.modules and loads the node
as a synthetic package, so it runs deterministically in clean CI without
discord.py installed and without a version skip (node annotations switched to
Optional[...] so it imports on 3.10-). Added lifecycle tests (missing token,
login failure, privileged-intent failure, unexpected close, cancellation,
shutdown-marks-closing), allowed-mentions tests for all three reply modes plus
thread reuse, and an oversized-attachment regression. Retained the
multi-attachment ingest-all regression.
Develop conformance:
- services.json: add "documentation" URL.
- README.md: restructure to the current node README schema (top-level Lanes
table, drop the hand-written field table, fold sections under Notes); passes
scripts/validate-node-readme.py.
- credentials.json: register the discord credential (nodes:credentials-generate)
so nodes:credentials-check passes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(nodes): Discord thread reply mode handles DMs and existing threads
Addresses CodeRabbit re-review:
- _send_chunk thread mode no longer calls message.create_thread()
unconditionally: it posts into message.channel when the message is already
inside a thread, creates a thread only for TextChannel messages, and falls
back to a plain reply (mentions suppressed) in DMs and other non-threadable
channels — previously these raised and dropped the response.
- README: document that thread reply mode also needs Create Public Threads and
Send Messages in Threads, and that it falls back to a reply where a thread
cannot be created.
- Tests: add thread-in-TextChannel, reuse-existing-thread, post-into-own-thread,
and DM-fallback cases (58 tests pass on 3.9 and 3.11).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(nodes): discord node support-bot parity features
Brings the discord source node to parity with the discord.js support bot it
replaces. Every new setting is opt-in; with defaults the node behaves as it
did before.
- Replies: thread name template, length cap and archive duration; optional
(i/n) chunk labels; allowlisted user/role mentions (never @everyone/@here);
a failed thread creation falls back to a reply instead of losing it.
- Context: earlier thread messages as context (threadHistoryLimit).
- Escalation: pause a thread after an escalating reply, rebuild the pause from
thread history after a restart, resume on an @mention; a team-name alias is
turned into the team role mention; per-thread serialization while the pause
is on.
- Aimed at someone else: optional ack emoji, and the thread pauses.
- Feedback: optional reactions on the last chunk of an answer.
- Reply hygiene: strip leaked agent scratchpad and final-answer envelopes,
never post engine or model errors, retry empty or scratchpad answers.
- Attachments: optional text-file decoding and one merged answer per message.
- Gating: allowed bot ids, mention-only channels, system and empty messages
dropped.
- Events: reaction, no_reply and outbound events with the message metadata,
broadcast as apaevt_sse; reactions are scoped by the same guild, channel and
bot gates as messages.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(nodes): durable event capture for the discord node
With captureEvents on, the node writes every event it broadcasts (message,
plus outbound, no_reply and reaction when their emit options are on) into a
database tool node wired to the source by a control edge, so a support
dashboard has a complete log even when nobody is watching the live stream.
- One row per event in a fixed INSERT ... ON CONFLICT DO NOTHING through the
database node's execute tool, keyed (message_id, event_type, event_key), so
a redelivered Gateway event is ignored.
- Off the answering path: a bounded queue and one writer thread; a slow or
dead database costs a logged, rate-limited warning, never a Discord reply.
- captureNodeId picks the database component, captureTable the table
(validated, created if missing), captureSource the label written to each
row's source column.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): cover the discord node parity features and capture
Unit tests for every new behaviour and its default: thread replies and
fallbacks, thread-history context, escalation pause and history rebuild,
per-thread serialization, aimed-elsewhere acks, feedback reactions, reply
hygiene and retries, error suppression, team mention alias, chunk labels,
attachment merge, reaction scoping, event payloads, config coercion, and the
capture writer (row shape, SQL, failures, queue, thread lifecycle). The
services.json schema tests assert every field, its UI registration and its
default.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): document the discord node's new settings
Describes each parity feature and the capture log in the node README, and
regenerates the PARAMS schema table with nodes:docs-generate.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): add the gated live Discord harness and test notes
A live suite for the discord node against a real Discord server and engine,
skipped unless DISCORD_LIVE=1 (ids and the token are read from files outside
the repo). The README records each live round: what was run, what passed and
the bugs it found.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): read discord live-harness files as UTF-8
Collection of the live suite opened its seed and config files with the
locale's encoding, which fails under an ASCII locale before the DISCORD_LIVE
gate can skip anything. Every read and write now names UTF-8.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: note the discord node's support-bot features in the changelog
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): never post an engine LLM error as a discord reply
With sanitizeReplies on, a provider failure still reached Discord: the
engine's LLM layer turns the exception into the answer text
"**LLM error** - ValueError: An error occurred with the API.", and no
error signature matched it (the API pattern needs a word before "API").
Two signatures now catch it: the "**LLM error**" prefix at the start of
the answer, and the bare "An error occurred with the API." sentence
when it is the whole answer. Prose that only mentions API errors is
not matched. The existing path then emits no_reply model_error.
Found by the live engine suite (F40); verified live after the fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): split fenced discord replies between lines
Inside a code fence the chunker hard-split at a character count, so
every boundary cut a code line in two and a copied block was broken
(live F12 and F44: 2 of 2 and 10 of 10 boundaries).
It now breaks at the last newline in the second half of the window.
The synthetic close/reopen fence pair stands in for that newline, so
the reply rebuilds exactly by replacing each boundary pair with "\n".
A newline right before a fence is passed over, so a boundary never
produces an empty code block, and a single line longer than a chunk is
still hard-split.
A numbered reply also stripped no trailing whitespace before its
label, leaving blank lines between the closing fence and "*(n/n)*";
each piece is now stripped first.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(nodes): add discord pipelineTimeoutSeconds and name config failures
pipelineTimeoutSeconds (default 0, off) gives up on a pipeline run
that has not answered within the limit and emits no_reply with reason
timeout. Each run (the question, each attachment, each retry) gets the
full limit; the run itself cannot be cancelled, so it finishes in the
background, returns its pipe, and its late answer is dropped. Before
this a hung pipeline left the question with no outcome at all.
Configuration and start failures now say what to fix:
- a list setting that starts with "[" but is not valid JSON is reported
in the task's warnings with the setting's name, instead of silently
becoming a literal id that matches nothing; in guildIds or channelIds
it fails the start, since the bot would otherwise answer nothing
- a botToken that is still an unresolved ${NAME} fails the start naming
the variable, instead of "login failed (invalid token)"
- with includeMemberMetadata on, a refused privileged intent leads with
the Server Members Intent (Message Content is usually already on)
Found by the live engine suite (F36, F38, F39, F41); all verified live.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): add the full live engine suite for the discord node
test_engine_full.py (F01..F46) walks every discord node feature,
success and failure, through real pipelines on a running engine, driven
by the second bot in the support channel only. Node behaviour runs on
deterministic pipes: an echo pipe (discord -> response_text keyed
answers) and a fake-model pipe pointed at fake_llm.py, a scripted
OpenAI-compatible endpoint for scratchpad, final-envelope, error-text,
empty, retry, HTTP 429 and slow answers. One case runs the production
pipe without tool_slack and capture.
The suite is opt-in (DISCORD_LIVE=1 and DISCORD_E2E_FULL=1) and skips
offline. The driver bot gains attachments, embeds, replies, reactions,
deletes, thread creation and a typing recorder; its posts suppress
mentions by default. The live README records the 44-row result matrix,
the findings fixed in this branch, and the residue a run leaves.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): report capture rows left unwritten when the discord writer stops
stop() waits a bounded time for the capture worker. When a write is
stuck on the database the worker is still busy after the join, and the
rows still queued are lost when the daemon thread dies with the
process, with nothing logged and nothing counted. stop() now warns
with the approximate number of unwritten rows in that case.
Raised by CodeRabbit on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): label the discord tile by token presence, not connection
The tile only tests whether botToken is set, yet read "connected"
even when login failed, an intent was missing or the ${NAME} never
resolved. It now reads "Token: configured" or "Token: missing", and the
README says the task status line is what reports the connection.
Raised by CodeRabbit on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): list the reply patterns the discord node treats as errors
The error-suppression sentence chained several clauses and did not
parse. It is now a list that matches _ERROR_SIGNATURES in text_utils,
with the posting, retry and sanitizeReplies-off behaviour unchanged.
Raised by CodeRabbit on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): parse live discord harness config through the real _run
make_endpoint kept a hand-written copy of IEndpoint._run's config
block that had drifted: escalation markers and feedback emojis were
split on whitespace, several newer settings (mergeAttachments,
numberChunks, teamMentionAlias, nonAnswerRetries, pipelineTimeoutSeconds,
capture) were never applied, and numbers skipped the production
coercion. It now runs the real _run(), stopped where it would reach the
engine's shared web server, as the unit tests already do.
The live README also marks B1 and G6 as fixed, pointing to N7 and F03.
Raised by CodeRabbit on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): look up a discord reply's target with the real discord.py API
The aimed-at-others check called Message.fetch_reference, which
discord.py does not have. The AttributeError was swallowed, so the node
never learned who a reply was for, and with ignoreAimedAtOthers on every
reply without a mention was treated as aimed at someone else, including
a reply to the bot's own answer: it got the ack emoji and no answer.
The target now comes from reference.resolved (sent with the reply by
the Gateway), then reference.cached_message, then
channel.fetch_message(reference.message_id). The unit tests delete
fetch_reference from the fake message, so a call to a method discord.py
does not have fails the test instead of passing; the old test mocked it.
Reported in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): add a live case for a reply to the discord bot without a mention
F23b replies to the bot's own answer with the ping off, so only the
reply-target lookup can tell the node the message is for it. The live
README records the finding as N8; the case has not been run live yet.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* refactor(nodes): move discord support behaviours to a follow-up PR
PR #1503 is being split into three stacked PRs. This removes the support
behaviours from the base Discord node: thread history as context, the
escalation pause and per-thread serialization, skipping messages aimed at
others, feedback reactions, the team mention alias, reply sanitizing and
error detection, non-answer retries, the pipeline timeout and backfill.
Their services.json fields, config parsing, helpers, text utilities, README
sections and unit tests go with them; the follow-up PR re-adds them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* refactor(nodes): move discord event capture to a follow-up PR
The durable Postgres capture of the node's events moves to the third PR of
the split. This deletes capture.py and its tests, the captureEvents,
captureNodeId, captureTable and captureSource fields, and the capture
writer lifecycle and call sites in IEndpoint.
The engine-warning helper _config_warning relied on now lives in IEndpoint,
so the node no longer imports the capture module.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): fail the discord start on an unset variable in guildIds or channelIds
The engine passes an unknown ${NAME} through as literal text. In guildIds or
channelIds it became an allowlist id that matches nothing, so the bot
connected and answered nobody. The start now fails, on the same path as the
broken-JSON check, naming the field and the variable.
In allowedMentionRoleIds and allowedMentionUserIds such an entry was only
dropped with a debug line; it now also produces a task warning naming the
field and the variable.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): keep discord thread settings within what Discord accepts
Discord rejects a thread name longer than 100 characters and any
auto-archive duration other than 60, 1440, 4320 or 10080 minutes, either of
which cost the whole answer with a 400.
threadNameMaxLength now declares minimum 1 and maximum 100 and is clamped
to that range in code. threadAutoArchiveMinutes is an enum of the accepted
durations plus 0 for the channel default; the code still falls back to the
channel default for any other value that reaches it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): say which discord text is broadcast and kept in the run log
The README called the apaevt_sse broadcast live-only. It is not: the engine
writes every SSE body into the task's run log, so it is visible to every
client monitoring the task and kept afterwards.
The README now says so and lists exactly which message text the node puts
in those bodies (the question on message events, the answer on outbound
events), so operators can cover it in their privacy notice.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* chore(nodes): add comment blocks to the discord services.json
Every top-level key of the Discord services.json now has a // comment block
explaining its purpose, in the same form as the Telegram node's.
The file is JSONC from here on, so the unit tests and the live suite drop
its whole-line comments before parsing it, and a schema test checks that
each top-level key keeps its comment block.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): remove internal data from the discord live suite
The repo is public and the live folder carried internal data. The replay
seeds are now ten made-up questions about a generic pipeline tool, the
README is cut down to the layers, how to run them, the variables they need,
the residue policy and what they do not cover, and the test pipe's prompt no
longer names an internal bot.
The token and id files are read from DISCORD_LIVE_TOKEN_FILE and
DISCORD_LIVE_IDS_FILE instead of fixed home-directory paths, and the layers
skip with a reason when they are unset. Result files go to a temp directory
or a *_RESULTS_DIR override instead of a repo-local folder.
The live cases for behaviours that moved to the follow-up PRs (D11, D16..D19
and F14, F20..F26, F32..F35, F40, F41b, F45, F46) and the helpers only they
used are removed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): regenerate the discord README settings table
Generated by nodes:docs-generate after the support behaviours and
capture moved to follow-up PRs and the thread settings gained their
limits.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(nodes): add support behaviours to the discord node
Re-adds the opt-in support behaviours split out of the base node: thread
history as context, the escalation pause with per-thread serialization,
skipping messages aimed at others, feedback reactions, the team mention
alias, reply sanitizing and error detection, non-answer retries, the
pipeline timeout and backfill. Their services.json fields, config parsing,
text utilities, README sections and unit tests come back with them.
The README privacy paragraph now also lists the text carried on no_reply
for skipped messages.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): do not answer the same discord message twice on backfill
Backfill had no record of what was already processed, so every restart
re-answered the newest backfillLimit messages, and a message that arrived
live in a channel not yet backfilled was answered twice.
The node now keeps the last 1000 handled message ids, shared by the live
path and backfill, and backfill skips messages the fetched window shows as
answered: a message with a thread in thread mode, one a bot message
replies to, and in channel mode everything before the bot's latest post.
No extra API call is made per message.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): block only real errors in discord replies
The error filter matched provider status codes, tracebacks, engine frames
and the "an error occurred with the X API" sentence anywhere in a reply,
so a support answer that quoted the user's error was suppressed as a
model error. These shapes now count only where the reply opens with them,
and code fences are removed before the check.
sanitize_reply had the same problem: the final envelope is now unwrapped
only when it is the whole reply or ends a scratchpad, and Final Answer
trimming fires only on a line start outside code fences.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): run discord pipelines on their own bounded pool
Pipeline runs used asyncio.to_thread, the loop's default pool, which the
event emits share. Runs that outlived pipelineTimeoutSeconds kept their
threads, so enough of them filled the pool and blocked every emit and
every new message.
Pipeline runs now go through a ThreadPoolExecutor of 8 workers created in
_startup and shut down without waiting in _shutdown, gated by a semaphore
of the same size. The timeout covers waiting for a slot as well as the
run, and a timed-out run keeps its slot until it returns. Event emits stay
on the default pool.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): add the discord support behaviours to the live suite
Re-adds the live cases for the support behaviours: in the full engine
suite F14, F20, F21/F22, F23, F23b, F24, F25, F26, F35, F40, F41b, F45 and
F46, and D16..D19 in the live I/O layer, with the reactions_on and react
helpers they use.
F46 reads its pipe from DISCORD_E2E_AI_PIPE and F45 needs
DISCORD_E2E_ENGINE_DIR; both skip with a reason when unset. The live
README lists the cases and the two variables.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): regenerate the discord README settings table
Generated by nodes:docs-generate after the support behaviour settings
came back. The live README notes that the backfill case moved to F35.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): list decoded text attachments among the broadcast discord text
With mergeAttachments off, a text-like attachment is sent through the
text lane as "[attachment <name>]" plus its decoded contents, and that
text is broadcast on its message event (clipped at 2000 characters),
so it lands in the run log too. The privacy list now names it.
Raised by CodeRabbit on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): read discord nonAnswerRetries from an engine number proxy
The engine delivers number parameters as string-like proxies without
__int__, so int() raised and every configured retry count, including
0 and 3, fell back to 1. It now goes through _as_int like the node's
other numbers, still clamped to 0..3 with 1 for a malformed value.
Raised by CodeRabbit on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): handle blank discord text, partial delivery and late messages
Three edge cases in the base node:
- The intake gate strips a message's text to decide whether it has
anything to ask, but processing read the raw text, so whitespace next
to a file ran a text pass of its own (and skipped the file-only
framing when merging). Processing now uses the same stripped answer.
- When a later chunk failed after earlier ones were posted, the
outbound event looked like a full delivery. _send_response now
reports whether every chunk went out, and the outbound event carries
it as "complete".
- A message arriving after shutdown began could start a task the
shutdown never awaited. _on_message now ignores messages once
closing.
Raised by CodeRabbit on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): say which discord events need emitOutbound or emitNoReply
The README implied an outbound or send_failed no_reply event always
follows an answer. Both need their opt-in flag, which is off by default.
It also documents the new "complete" field on outbound events.
Raised by CodeRabbit on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): backfill only the allowed guilds when no discord channels are listed
With no channelIds, backfill walked every text channel in every guild
the bot is in and read its history, although the gate then dropped
every message from a guild outside guildIds. Channels are now filtered
by guild before any history is fetched, which also stops the startup
from reading history in excluded guilds.
Raised by CodeRabbit on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: keep the discord changelog line to what the base node ships
The entry still listed thread history, escalation pause, ack and
feedback reactions, reply hygiene and retries, and event capture, which
moved to follow-up PRs. List only what this node ships.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): catch every unset variable in the discord settings
The engine resolves only ROCKETRIDE_* variables and replaces any other
${NAME} with the literal <REDACTED>, which the node did not recognise:
a token failed as "invalid token" and list entries matched nothing.
Both forms are now named, and the message says only ROCKETRIDE_* server
variables are resolved. requireMentionChannelIds now fails the start
like guildIds and channelIds (an unset entry made the mention gate fail
open), allowedBotIds warns like the mention lists, and JSON list items
get the same strip and comma/whitespace split as a bare string.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): say how to keep strangers from adding the discord bot
With guildIds and channelIds empty (the default) the bot answers in
every server it is added to and in DMs, and a new Discord application is
a Public Bot. The setup steps now say to turn off Public Bot, the README
recommends setting guildIds in production and says DMs are answered only
while both lists are empty, and the node warns once at start when
guildIds is empty.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(nodes): bound how many discord messages are processed at once
Every message that passed the gates started its own task, and each one
holds its downloaded attachments (up to maxAttachmentBytes, up to 10
files) in memory until the pipeline answers, so nothing limited memory.
A new maxConcurrentMessages setting (default 4, 1 to 32) sizes a
semaphore created in _startup; further messages wait their turn rather
than being dropped. maxAttachmentBytes gets a 100 MB maximum, enforced
in code as well.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): never let a blank discord answer win over a real one
Answers were chosen by truthiness, so a whitespace-only answer such as
a newline beat a real attachment answer, and with merging on a blank
text pass hid the first attachment answer. Every pipeline answer is now
normalised the way the question text is before it is chosen, a blank
attachment answer is no longer folded into the merged question, and
_send_response treats an answer with no non-blank chunks as nothing to
send.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): decode discord text attachments the same way on both paths
With mergeAttachments off a text file was sliced to
textAttachmentMaxChars (0 sent an empty body, a negative value dropped
the end) and never checked for binary content; with it on, 0 or less
kept everything and a NUL byte meant binary. Both paths now share one
decode helper and one cap rule: 0 means no limit, and a file with a NUL
byte (such as UTF-16) is not text. textAttachmentMaxChars gets a
minimum of 0 in the schema.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* chore(nodes): drop the unexplained discord.py version floor
Every discord.py API the node uses exists in 2.0 and later, and node
dependencies are resolved together into one constraints lock, so an
arbitrary floor could move other packages for no reason.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): document what the discord send helpers return
_send_response said it returned None but returns messageIds,
destination and complete; the docstring now describes each key.
_emit_outbound_event and _record_sent had no docstring.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): route discord files of unlisted types by their extension
The extension table knows 15 extensions; any other file without a
content type became application/octet-stream and went to the tags lane,
so .m4a or .flac audio reached the document lane. After the content type
and the table, the extension is now looked up with Python's mimetypes
before falling back to application/octet-stream.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): regenerate the discord README settings table
Generated by nodes:docs-generate for maxConcurrentMessages, the new
bounds on maxAttachmentBytes and textAttachmentMaxChars, and the
dependency line without a version floor.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): skip queued discord messages once shutdown has begun
A message waiting for a maxConcurrentMessages slot is already tracked as
in flight, so shutdown waits for it; once it got its slot nothing checked
that shutdown had begun, and it downloaded, ran the pipeline and replied
anyway. It now returns as soon as it gets the slot (with a no_reply
"shutdown" event when emitNoReply is on). The README notes that the slot
limit is shared across servers and what it does not buy.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): warn about an open discord bot only when no allowlist limits it
With channelIds set, only those channels and their threads are answered
and DMs are refused, so the start-up warning that the bot answers in any
server it is added to was wrong. It now fires only when guildIds and
channelIds are both empty, and the README says the same.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): fail the discord start when an allowlist resolves to nothing
The engine replaces a set ${ROCKETRIDE_X} with its value, so an empty
value reaches the node as "". Blank items are dropped, the list became
empty, and an empty guildIds, channelIds or requireMentionChannelIds
means "everywhere". A list that was given items but resolves to no ids
now fails the start with the field's name; a genuinely empty list
(missing, None, [], '', '[]') still means all.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* refactor(nodes): let discord list settings keep phrases whole
_as_str_list always split JSON items and bare strings on commas and
whitespace, which suits ids but breaks a list of phrases. It now takes
split=True; with split=False items are kept whole (still stripped, blank
ones still dropped). Every current caller keeps the default.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): read UTF-16 and BOM-prefixed discord text files as text
Any NUL byte meant "binary", so UTF-16 files (Windows Notepad "Unicode",
PowerShell 5.1 redirects) were never read as text: with merging off they
went to the binary lanes, with merging on they were dropped with only a
debug line. A UTF-8 BOM also survived into the text. The decode now
checks for a UTF-16 BOM first, otherwise decodes as utf-8-sig, and only
then treats a NUL as binary content. In merge mode a text-like file that
is still binary becomes its own binary lane object (reusing the bytes
already downloaded) instead of being dropped.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): guess discord file types from Python's built-in table only
mimetypes.guess_type also reads the Windows registry and /etc/mime.types,
so the extension fallback routed the same file differently per host: on
Windows .csv became application/vnd.ms-excel and .ts a video type (it is
video/mp2t on macOS too). The fallback now asks a module-level
mimetypes.MimeTypes(), which CPython fills from its built-in defaults
only; the node's own extension table still answers first.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): accept only numeric ids in the discord allowlists
An entry in guildIds, channelIds or requireMentionChannelIds that is not
plain ASCII digits (a pasted <#123>, a channel or server name) matched
nothing, so the mention gate failed open or the bot answered nothing. The
start now fails naming the field and the entry, and suggests the id inside
a pasted mention. The mention allowlists and allowedBotIds use the same
digit rule, so '123²' no longer reaches int() in every send.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): ignore discord reactions once shutdown has begun
The Gateway keeps delivering reactions while _shutdown waits for in-flight
messages, and each one still opened a pipe. _on_raw_reaction now returns
when _closing is set, and checks again just before the emit. The reaction
authorIsBot now uses the same _reactor_is_bot answer as the gate, so a
removal by an allowlisted bot is reported as a bot.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): split prose in fenced discord replies at a word boundary
A reply with a code fence anywhere goes through the fenced chunker, which
only broke at a newline in the second half of the window and otherwise cut
by character count, even in prose outside the code block. Outside an open
fence it now falls back to the last sentence end, then the last whitespace,
in the second half before cutting by count.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): bound and normalise the discord attachment settings
maxAttachmentBytes of zero or below skipped every attachment with only a
debug line; it is now clamped to at least 1 and the schema declares
minimum 1. textAttachmentExtensions entries typed without a dot (md, json)
never matched os.path.splitext; each entry is now normalised to a
lowercase extension with one leading dot, and blank entries are dropped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): coerce discord booleans and overflowing numbers from the engine
_as_int caught TypeError and ValueError but not OverflowError, so a number
setting of inf or 1e400 failed _run with an unrelated traceback; it now
falls back to the default. The ten boolean settings were used raw, so a
string-like 'false' read as True; a new _as_bool reads real bools as-is and
true/false, 1/0, yes/no, on/off strings in any case, else the default.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): document discord events, reasons and setup accurately
The tags lane also carries the reaction, no_reply and outbound event
objects, and events have their own entry URL and name; the README now says
so. The no_reply reasons are listed (no_answer, send_failed, shutdown, or
an error message clipped to 200 characters), and so is what happens when
the question is deleted before the answer is posted. Configuration
headings use the field titles, each paragraph sits under the field it is
about, and the broadcast section moves under Notes. The Developer Portal
steps use Reset Token, the reactions intent is described as one the node
turns on itself, and the MIME fallback examples are .avi and .bmp, which
the node's own table does not list. In services.json, guildIds and
channelIds default to [] and the text-attachment and emitNoReply
descriptions match the behaviour.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): complete the discord docstrings and drop references to a private bot
Several Args sections left out parameters the functions take (_on_message
wait, _process_attachment meta and attachment_index, _run_text_pipeline
meta, object_name and attachment_id, _run_binary_pipeline message_id,
attachment_index and meta, should_process_message parent_channel_id and
allowed_bot_ids). Comments and docstrings no longer point at functions of
a bot that is not in this repo; the sentences explaining the behaviour
stay.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(nodes): make the discord live suite neutral and self-describing
The live README now lists the variables L3 and L4 need on the engine and
says ROCKETRIDE_DISCORD_E2E_NO_SUCH_TOKEN must stay unset, and asks for
Python 3.10+. engine_min.pipe gets a fresh project_id (the SDK's use()
contract carries it at the top level) and a short generic assistant
prompt with no escalation line. E05 tested that escalation prompt, which
belongs to a follow-up change, and E06 was a skipping placeholder; both
are removed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): regenerate the discord README settings table
Generated by nodes:docs-generate for the new bounds and list defaults
and the updated textAttachmentExtensions and emitNoReply descriptions.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): wait for discord reaction emits at shutdown and hint ids by kind
A reaction emit already under way was not tracked, so shutdown did not
wait for it and tore the endpoint down beneath it. The emit is now a
tracked task, like a message handler. The second closing check before
the emit could never fire (nothing can change the flag between the two
checks) and its test only passed by setting the flag from a mock; both
are gone.
A pasted mention in an id list suggested the digits inside it even when
they were the id of something else (a role in a channel list, a channel
in the server list). The hint now gives the digits only for the right
kind of mention and otherwise says what was pasted.
Also documents the arguments of _emit_event_pipeline and the return of
_emit_no_reply_event.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): never cut a discord reply inside a code-fence opener or a word
When a chunk window ended inside or just after a code-fence opener line,
chunk 1 ended with a split language name or an empty code block. A block
whose opener line and first code line do not both fit now starts the next
chunk, and the newline search never cuts right after an opener line.
A reply with no fence also hard-split a line longer than the limit in the
middle of a word. It is now cut at the last sentence end or whitespace in
the second half of the window, by character count only when there is none.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* refactor(nodes): read discord booleans with the shared parse_bool
The node had its own _as_bool, which duplicated parse_bool in
ai.common.utils. Every boolean setting now goes through the shared
helper. The engine passes JSON booleans as real bool values, so the
tests that fed string-like proxies to a boolean are replaced with tests
for real bools and plain strings.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): keep secrets out of discord id errors and bound id length
A bot token or secret ${ROCKETRIDE_*} value pasted into an id list was
echoed in full by the start error, the task warnings and the debug log.
Every message about a list entry now goes through one helper that shows
a mention whole and anything else longer than 12 characters as its first
12 characters followed by an ellipsis.
An id is also bounded to a 64-bit snowflake (at most 20 digits, below
2**64), so two ids pasted together are rejected as non-numeric on the
same fatal or warning path as any other bad entry.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): treat a non-positive discord attachment cap as unset
maxAttachmentBytes was floored at 1, but a 1-byte cap skips every real
attachment just as 0 did. A value of 0 or below now means "not set" and
falls back to the 25 MB default, matching config_int in ai.common.utils;
the value is still clamped to at most 100 MB. The schema keeps its
minimum of 1.
Raised in review on #1503.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): regenerate the discord settings table
The merge with the final base node left the generated table with the
support-behaviour side's rows; regenerated from services.json.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): resume a paused discord thread only on a written mention
A Discord reply with its ping on puts the replied-to author in
message.mentions without any mention in the text, so replying to the
bot's hand-off line resumed a paused thread, live and after a history
rebuild. Resuming now reads raw_mentions (parsed from the text); the
aimed-elsewhere gate still uses mentions, where a reply to the bot should
count. The README says any thread member can resume or pause a thread.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): never turn discord scratchpad that names the team into a hand-off
The team alias was replaced with a role mention before the sanitizer ran,
and the sanitizer looked for a marker anywhere in a scratchpad. Reasoning
such as "Thought: I could hand off to the RocketRide team but..." was
therefore posted as a hand-off line that pinged the staff role and paused
the thread. The alias is now injected only into the final text that is
posted, and a scratchpad counts a marker (or the alias) only on its last
non-empty line, never in a reasoning line.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): include the original question in the discord thread transcript
In thread reply mode the thread opens with a thread_starter_message that
has no content and only references the question in the parent channel, so
the transcript dropped it and the model never saw the question. The
starter is now replaced by the referenced message (resolved, cached, or
fetched from the parent channel), and skipped when it cannot be read.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): do not treat a reply to your own discord message as aimed elsewhere
Any reply whose target was not the bot counted as aimed at somebody else,
including a user replying to their own question to add details. With
ignoreAimedAtOthers on the bot then stayed silent, and with
escalationPause also on it paused the thread. A reply whose target was
written by the same author is now for the bot unless it mentions somebody
else; the author's own reply ping no longer counts as a mention of others.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): anchor every discord error signature to the opening of the reply
looks_like_error still matched "_run failed" anywhere, so an answer such
as "Your log shows that task_run failed because..." was suppressed. It now
counts only at the opening, like every other shape. The optional label
before "Error code: NNN" is limited to exception names, so "Note: Error
code: 429 means..." is posted, and each removed code fence is replaced by
a placeholder line so the line after a leading code block is no longer
read as the opening.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): never post a discord error answer, whatever sanitizeReplies says
The engine turns a failed LLM call into the answer "**LLM error** -
<type>: <exception>", and the raw provider exception can carry account
details, key fragments or internal URLs. The error check ran only inside
sanitizeReplies, which is off by default, so with default settings that
text was posted to the channel. The check now always runs; only the
scratchpad stripping and the retries stay opt-in. The LLM error pattern
also matches the agent's unbolded "LLM error: <exc>" form.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): match the discord team alias and markers as whole words
The alias replacement hit the alias anywhere: inside longer words, URLs
and code, so "Support" turned "supportive" into "<@&1>ive" and each wrong
hit also sent a real role ping. The alias now matches only as a whole
word that is not part of a URL path, and only outside fenced code blocks.
Escalation markers get the same treatment: a marker counts only as a
whole word (punctuation edges such as a role mention need no boundary)
and never inside a code block.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): limit discord team-role pings to one per conversation per hour
The team alias is replaced in the model's output, so any user who gets the
model to write it makes the bot ping the staff role, a ping they often
cannot send themselves, and in reply and channel mode (or after an
@mention un-pauses a thread) as often as they like. Injected pings are now
limited to one per thread or channel per hour, kept in memory; within the
window the alias is posted as plain text and still counts as a hand-off
for escalationPause. The README says the alias lets any user trigger a
role ping.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): stop one discord user forging speaker lines in the transcript
Each thread message went into the transcript as "name: text" with its
newlines kept, so a message containing a newline could add lines that
looked like another speaker, the bot included. Continuation lines are now
indented, each message is clipped to 1000 characters so one message
cannot fill the whole transcript budget, and messages from other bots are
left out unless they are in allowedBotIds.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): bound the discord backfill and thread-history numbers
backfillLimit, threadHistoryLimit and threadHistoryMaxChars had no schema
bounds and no clamp, so threadHistoryLimit 10000 paged 100 history
requests for every message in a thread, and a large backfillLimit ran
that many pipelines per channel at startup. The schema now declares 0 to
100, 0 to 100 and 0 to 20000, and the node clamps to the same, as it does
for nonAnswerRetries. threadHistoryMaxChars 0 now turns the context off,
like every other new number, instead of removing the cap.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): do not let discord backfill replay messages it already handled
Backfill counted a message as answered only through a visible reply, so
every restart ran the pipeline again on messages acknowledged as
aimed_elsewhere, and the thread check applied only in thread mode, so
changing replyMode answered old threads again. A message carrying a
reaction the bot added now counts as handled, and the thread and reply
checks apply in every reply mode. A non-numeric channel id is skipped on
its own instead of ending the whole backfill. With sendResponses off
nothing is ever visible, so backfill no longer runs at all rather than
replaying the window (and its outbound events) on every restart.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): list every effect of the discord sanitizeReplies switch
The sanitizeReplies description said only that it strips leaked
reasoning. The same switch retries empty and scratchpad-only answers, so
a pipeline that is silent on purpose runs 1 + nonAnswerRetries times per
message; operators see only these descriptions and the generated table.
Both descriptions and the README now list the effects, and say that
error answers are suppressed whether or not the switch is on.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: note that discord error answers are suppressed by default
The #2547 changelog line said every support behaviour is opt-in. Error
answers are now suppressed whatever the settings, so the line says so.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): regenerate the discord settings table after the review fixes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): catch discord error answers wrapped in a final answer
The error check ran only on the raw reply, before sanitize_reply took off
the agent's wrapper, so "Thought: done\nFinal Answer: Error code: 401 - key"
or a {"type":"final"} envelope around a provider error was posted. The
final-text extraction is now shared by sanitize_reply and looks_like_error,
which checks both the reply and its final text whatever the settings. A
retry's answer is checked raw and after sanitizing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): suppress only engine and provider errors in discord by default
Error suppression ran on every answer whatever sanitizeReplies said, so a
real answer that opened with "Error: ENOENT means..." or "Error code: 404
means not found." was dropped silently. The engine and provider shapes
(LLM error, traceback, chat.py:NN frame, _run failed, the API-error
sentence, and "Error code: NNN - " with its payload) are still always
suppressed; the generic openings (Error:, Exception:, <Name>Error: labels,
a bare Error code:) now count only with sanitizeReplies on. Every
suppressed answer is now also reported as a task warning naming the
message and model_error, never the answer text. README, services.json and
the CHANGELOG line for #2547 say so, and the CHANGELOG no longer calls the
error suppression opt-in.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): keep the discord team-ping cooldown per user and unbypassable
The team-ping cooldown was kept per thread or channel, so a user could
open a new thread for each ping, and the model could still ping the role
on cooldown by writing a literal <@&id> (echoed from the transcript or
asked for). The cooldown is now kept per message author, a send on
cooldown leaves the team role out of AllowedMentions (other allowed roles
and users stay), and the team role's mention is shown as the alias in the
thread transcript. The hour starts only after the chunk carrying the
mention was posted, so a failed send or sendResponses off does not use up
the user's ping.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): never treat a discord scratchpad without a final answer as a hand-off
A leaked scratchpad whose last line had no reasoning label counted as a
hand-off, even when that line was tool output: "Observation: Refunds go
to finance.\nIf unresolved, contact the RocketRide team." became the
hand-off line, pinged the team and paused the thread. A scratchpad with
no Final Answer and no final envelope is now always a non-answer; the
hand-off line is only built from a final answer that is itself
scratchpad. Two tests that encoded the old behaviour were changed, and
the README says so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): start a capped discord transcript on a whole line and strip ackEmoji
The thread transcript's tail cut could land in the middle of a message,
so the context opened with half a line from an unnamed speaker. The cut
now starts at the next speaker line (skipping indented continuation
lines), unless that would leave nothing. ackEmoji was used as typed, so
" ✅" was not a valid reaction; it is now stripped, as feedbackEmojis
entries already were.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): correct the discord timeout wording and complete the docstrings
The README said a new message with every worker busy reports timeout
rather than waiting; it waits for a worker, within the limit. It now also
says a pause set by an aimed_elsewhere skip is not rebuilt after a
restart, since only bot messages with an escalation marker are. The
functions in this change without Args or Returns (_await_pipeline,
_is_escalation, _is_bot_mentioned, _is_bot_named_in_text, _react and
others) now follow the house docstring style, and the cooldown constant's
comment says the limit is per user.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): never let a raw discord scratchpad ping or pause, sanitized or not
With sanitizeReplies off the raw scratchpad was posted as-is, the team
alias in a Thought line became a role ping, and the escalation check
could pause the thread: leaked reasoning such as "I could hand off to the
RocketRide team but I can answer this myself" handed the thread to staff.
A new pure helper, handoff_part, returns the part of a reply that may
hand off: the whole reply, or for a reply opening with a reasoning label
only its final text (after the last Final Answer:, or inside a final
envelope). The alias is injected and escalation markers are detected only
there, also when the pause is rebuilt from history, and a scratchpad
with no final text is sent with the team role withheld.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): reserve the discord team-ping cooldown before sending
The cooldown was recorded only after the send, so two messages from one
user handled concurrently both passed the check and both pinged the team
role. The cooldown is now reserved synchronously right after the check,
with no await in between, and released after the send when no posted
chunk carried the mention (a failed or raising send, or the mention's
chunk not sent), so a later hand-off still pings. sendResponses off
reserves nothing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): drop references to a bot outside this repo from the discord node
Comments, docstrings, test docstrings and the README pointed at a private
support bot and its camelCase helpers, which are not in this repository
(the base node already dropped them). Each such reference is removed and
the sentence now describes the behaviour on its own. No code changes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(nodes): regenerate the discord settings table after the review fixes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): bound how long discord shutdown waits for message handlers
_shutdown waited on every in-flight handler with no limit. With
pipelineTimeoutSeconds off (the default) one hung run kept its handler
alive, so shutdown never closed the bot or the pipeline pool, and the
Gateway client stayed connected after the node was stopped. It now waits
at most SHUTDOWN_GRACE_SECONDS and cancels the handlers still running;
their worker threads run on, but the bot is closed.
The live F08 case also stops reading the test server's own guild id for
its "other guild" run, which allowlisted the guild it expects to be
ignored; it uses a guild id that does not exist.
Raised by CodeRabbit on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): let discord answers that open by quoting an error through
The always-on error check also runs with sanitizeReplies off, so its
patterns must not match prose. "Error code: NNN - " now counts only when a
payload follows (a JSON object or list, or a proxy's HTML page), which is
how the provider SDKs format it, and "LLM error" followed by a dash needs a
space after the dash. "Error code: 401 - Unauthorized means..." and
"LLM error-handling in RocketRide..." are posted again with default
settings; the engine and agent error shapes are still suppressed.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): recognise a discord scratchpad whose Thought label was stripped
A reply counted as raw agent scratchpad only when it opened with a
reasoning label. agent_llamaindex strips the leading "Thought:", so its
scratchpad reached the node as plain text: an alias in the reasoning
became a role ping and paused the thread. A reply is now also scratchpad
when an "Action:", "Action Input:" or "Observation:" label starts any of
its lines outside a code fence. handoff_part, sanitize_reply and the
envelope unwrapping all use the same check; a label mentioned mid-sentence
or inside a code block does not count.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): ping a discord role only from the final answer, with one cooldown for all
ping_team only checked that a raw scratchpad had a final part, not where
the role mention was. With sanitizeReplies off, a "<@&id>" written in the
reasoning pinged the role and started the user's cooldown, yet the thread
was not paused because escalation detection reads only the final part.
An allowed role is now let through only when its mention sits in the part
of the reply that may hand off (handoff_part), and only then does the
cooldown start.
The per-user cooldown also covered only the alias role, so any other role
in allowedMentionRoleIds could be pinged on every answer. It now applies
to every allowed role, with or without teamMentionAlias: a delivered
mention of any of them starts it, and while it runs the send withholds
all of them. README and the setting descriptions say so.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): split the discord transcript on every line break and close clipped fences
Continuation lines of a transcript message were found with split('\n'),
so a carriage return or a Unicode line separator in a message kept a
forged "assistant: ..." line on the speaker's own line, where a model can
still read it as a new speaker. The transcript now splits with
str.splitlines(), which covers all of them.
The 1000-character clip often lands inside a code block in one of the
bot's own long answers, which left every later speaker line inside an
open fence. A clip that leaves an odd number of fences now closes it.
Raised in review on #2547.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nodes): keep a suppressed discord error answer out of the debug log
The node suppresses a provider error that arrives as the answer because
it can carry account details or internal URLs, and the task warning never
names it, but the debug line logged its first 160 characters, which reach
the engine log whenever debug tracing is on. It now logs only the message
id and the answer's length. No other debug or warning line added in this
PR logs answer or question text.
Raised in review on #2547.
Co-Authored-By: C…
mithileshgau
added a commit
that referenced
this pull request
Oct 9, 2026
…2548) * feat(nodes): add Discord bot source node (discord) Add a Discord bot source node (discord://, classType ["source"]) that connects a Discord bot to a pipeline over the Discord Gateway — the Discord counterpart to the telegram node. Routes incoming text and image/audio/ video/document attachments to the text/image/audio/video/tags lanes, and posts the pipeline's first answer back to the originating channel, message, or thread. Mirrors the Telegram endpoint architecture: scanObjects() -> WebServer (kept alive via self._server.run()) with the discord.py Gateway client launched as a background task from the async on_startup hook. Config: botToken (secure), guildIds, channelIds, ignoreBots, requireMention, replyMode (channel|reply|thread), showTyping, maxAttachmentBytes, sendResponses. Includes 2000-char reply chunking, rate-limit backoff, bot-loop prevention, and mention gating. Tests: 21 unit tests plus an env-gated live-gateway integration suite (skips unless DISCORD_BOT_TOKEN is set). Closes #1502 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(nodes): drop Discord live-gateway integration test, fix CI encoding - Remove nodes/test/discord/test_integration.py: it imports the discord.py library at module top-level, which requires the (uninstalled-in-CI) dependency and a live DISCORD_BOT_TOKEN, and the package name collides with the node's own `discord` package during collection. It cannot run in CI. The pure-logic unit tests in test_discord.py remain. - Open services.json with encoding='utf-8' in the schema tests so they pass under CI's ASCII locale. - Replace the non-ASCII em dash in services.json with a hyphen. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(nodes): address Discord node review feedback - Extract pure helpers (chunk_message, guess_media_type) into text_utils.py with no discord.py dependency, so unit tests exercise the real production code instead of reimplementing it, and can run in CI without discord.py. - chunk_message now hard-splits any single token/sentence longer than the 2000-char limit, so every emitted chunk is guaranteed to fit (previously an unbroken long token could produce an oversized chunk Discord would reject). - Stop processing further attachments once a non-empty answer is found. - Handle discord.RateLimited (which carries retry_after) instead of reading a nonexistent retry_after off discord.HTTPException. - Declare _guild_ids/_channel_ids as annotations only to avoid mutable list defaults shared at class scope; they are set per-instance in _run(). - requirements.txt: keep only discord.py (requests/fastapi/aiohttp are not imported directly and flagged unpinned/vulnerable by OSV). - README: use the UI-facing lane name "Data" for the document lane. - Rewrite test_discord.py to test the real helpers and services.json schema; drop tautological config/filtering tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(nodes): address second Discord review round - Process attachments independently of the text pipeline so captioned uploads are ingested; the first non-empty answer (text first, then attachments in order, stopping at the first that answers) becomes the reply. - _send_response now abandons the remaining chunks on an unrecoverable send failure instead of silently continuing and dropping part of the reply. - Extract the message gating (self/bot/guild/channel/mention) into a pure should_process_message() predicate in text_utils.py and unit-test it; _on_message just wires primitives and schedules the task. - README: correct the attachment note to describe in-order processing that stops at the first attachment answer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(nodes): correct Discord MIME-detection precedence in README guess_media_type() prefers Discord's reported content_type (parameters stripped) and only falls back to the file extension; the README had the order reversed. Fix the description to match, keeping the octet-stream default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(nodes): address Discord review — ingest all attachments, correct docs - Remove the early `break` in the attachment loop so a source node ingests every attachment in a multi-attachment message (up to 10); still reply with only the first non-empty answer. The break silently dropped later attachments (no download, routing, or monitorCompleted). - Revert README lane name `Data` -> `tags` to match services.json and the telegram/webhook/core convention (no `Data` lane exists). - Clarify that discord.py handles 429s internally; the RateLimited branch is defensive (only fires with max_ratelimit_timeout, which is unset). - Regenerate stale GENERATED:PARAMS em-dash back to the services.json hyphen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(nodes): Discord node — use shared WebServer + correctness fixes from deep review Critical: - _run no longer constructs its own WebServer bound to --data_port. node.py already binds a shared WebServer on that port for every subprocess, so the second bind hit EADDRINUSE (and wired the target onto the wrong server) — the node could not start under EaaS. Now mirrors the Telegram node: register on the shared server and drive _startup/_shutdown on the shared server_loop, blocking scanObjects() on a shutdown event. Correctness: - _run_with_optional_typing now runs the pipeline exactly once; a typing- indicator enter/exit failure no longer re-executes it (double ingestion). - require_mention no longer satisfied by @everyone/@here (use direct message.mentions, not mentioned_in). - guildIds/channelIds coerced via _as_str_list so a scalar string no longer becomes a per-character allowlist; non-dict 'discord' config block no longer crashes _run. - guess_media_type lowercases the reported content type (mixed-case IMAGE/PNG routed correctly) and falls back to the extension when it normalizes empty. Docs: - Attachments download via discord.py's Attachment.read(), not aiohttp directly (aiohttp is transitive); drop the misleading "concurrently" wording. Tests: - New test_process_message.py (import-guarded, py3.10+/discord.py): attachment ingest-all + first-answer, exactly-once typing, allowlist coercion, and the @everyone gating fix. New MIME normalization cases in test_discord.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(nodes): list Discord node in nodes catalog and CHANGELOG - Add a `discord` row to the Sources table in docs/README-nodes.md. - Add a Discord node line under [Unreleased] / Added in CHANGELOG.md. Addresses review feedback that both shared surfaces were missing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(nodes): Discord lifecycle failure propagation, mention safety, and develop conformance Addresses kgarg2468's review: Lifecycle: terminal Gateway failures now fail the source promptly instead of idling forever with a dead bot. A missing token raises from _startup; login failure, missing Message Content Intent, an unexpected disconnect, or a generic gateway error record a fatal error and unblock _run, which re-raises so the engine marks the source failed. Successful startup still blocks until controlled shutdown; an intentional close during _shutdown is not treated as a failure. Security: all outbound sends (reply, thread, channel) pass discord.AllowedMentions.none() so model-generated text can never ping users, roles, @here or @everyone. Tests: the behavioral suite now stubs discord in sys.modules and loads the node as a synthetic package, so it runs deterministically in clean CI without discord.py installed and without a version skip (node annotations switched to Optional[...] so it imports on 3.10-). Added lifecycle tests (missing token, login failure, privileged-intent failure, unexpected close, cancellation, shutdown-marks-closing), allowed-mentions tests for all three reply modes plus thread reuse, and an oversized-attachment regression. Retained the multi-attachment ingest-all regression. Develop conformance: - services.json: add "documentation" URL. - README.md: restructure to the current node README schema (top-level Lanes table, drop the hand-written field table, fold sections under Notes); passes scripts/validate-node-readme.py. - credentials.json: register the discord credential (nodes:credentials-generate) so nodes:credentials-check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(nodes): Discord thread reply mode handles DMs and existing threads Addresses CodeRabbit re-review: - _send_chunk thread mode no longer calls message.create_thread() unconditionally: it posts into message.channel when the message is already inside a thread, creates a thread only for TextChannel messages, and falls back to a plain reply (mentions suppressed) in DMs and other non-threadable channels — previously these raised and dropped the response. - README: document that thread reply mode also needs Create Public Threads and Send Messages in Threads, and that it falls back to a reply where a thread cannot be created. - Tests: add thread-in-TextChannel, reuse-existing-thread, post-into-own-thread, and DM-fallback cases (58 tests pass on 3.9 and 3.11). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(nodes): discord node support-bot parity features Brings the discord source node to parity with the discord.js support bot it replaces. Every new setting is opt-in; with defaults the node behaves as it did before. - Replies: thread name template, length cap and archive duration; optional (i/n) chunk labels; allowlisted user/role mentions (never @everyone/@here); a failed thread creation falls back to a reply instead of losing it. - Context: earlier thread messages as context (threadHistoryLimit). - Escalation: pause a thread after an escalating reply, rebuild the pause from thread history after a restart, resume on an @mention; a team-name alias is turned into the team role mention; per-thread serialization while the pause is on. - Aimed at someone else: optional ack emoji, and the thread pauses. - Feedback: optional reactions on the last chunk of an answer. - Reply hygiene: strip leaked agent scratchpad and final-answer envelopes, never post engine or model errors, retry empty or scratchpad answers. - Attachments: optional text-file decoding and one merged answer per message. - Gating: allowed bot ids, mention-only channels, system and empty messages dropped. - Events: reaction, no_reply and outbound events with the message metadata, broadcast as apaevt_sse; reactions are scoped by the same guild, channel and bot gates as messages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(nodes): durable event capture for the discord node With captureEvents on, the node writes every event it broadcasts (message, plus outbound, no_reply and reaction when their emit options are on) into a database tool node wired to the source by a control edge, so a support dashboard has a complete log even when nobody is watching the live stream. - One row per event in a fixed INSERT ... ON CONFLICT DO NOTHING through the database node's execute tool, keyed (message_id, event_type, event_key), so a redelivered Gateway event is ignored. - Off the answering path: a bounded queue and one writer thread; a slow or dead database costs a logged, rate-limited warning, never a Discord reply. - captureNodeId picks the database component, captureTable the table (validated, created if missing), captureSource the label written to each row's source column. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): cover the discord node parity features and capture Unit tests for every new behaviour and its default: thread replies and fallbacks, thread-history context, escalation pause and history rebuild, per-thread serialization, aimed-elsewhere acks, feedback reactions, reply hygiene and retries, error suppression, team mention alias, chunk labels, attachment merge, reaction scoping, event payloads, config coercion, and the capture writer (row shape, SQL, failures, queue, thread lifecycle). The services.json schema tests assert every field, its UI registration and its default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): document the discord node's new settings Describes each parity feature and the capture log in the node README, and regenerates the PARAMS schema table with nodes:docs-generate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): add the gated live Discord harness and test notes A live suite for the discord node against a real Discord server and engine, skipped unless DISCORD_LIVE=1 (ids and the token are read from files outside the repo). The README records each live round: what was run, what passed and the bugs it found. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): read discord live-harness files as UTF-8 Collection of the live suite opened its seed and config files with the locale's encoding, which fails under an ASCII locale before the DISCORD_LIVE gate can skip anything. Every read and write now names UTF-8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: note the discord node's support-bot features in the changelog Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): never post an engine LLM error as a discord reply With sanitizeReplies on, a provider failure still reached Discord: the engine's LLM layer turns the exception into the answer text "**LLM error** - ValueError: An error occurred with the API.", and no error signature matched it (the API pattern needs a word before "API"). Two signatures now catch it: the "**LLM error**" prefix at the start of the answer, and the bare "An error occurred with the API." sentence when it is the whole answer. Prose that only mentions API errors is not matched. The existing path then emits no_reply model_error. Found by the live engine suite (F40); verified live after the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): split fenced discord replies between lines Inside a code fence the chunker hard-split at a character count, so every boundary cut a code line in two and a copied block was broken (live F12 and F44: 2 of 2 and 10 of 10 boundaries). It now breaks at the last newline in the second half of the window. The synthetic close/reopen fence pair stands in for that newline, so the reply rebuilds exactly by replacing each boundary pair with "\n". A newline right before a fence is passed over, so a boundary never produces an empty code block, and a single line longer than a chunk is still hard-split. A numbered reply also stripped no trailing whitespace before its label, leaving blank lines between the closing fence and "*(n/n)*"; each piece is now stripped first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(nodes): add discord pipelineTimeoutSeconds and name config failures pipelineTimeoutSeconds (default 0, off) gives up on a pipeline run that has not answered within the limit and emits no_reply with reason timeout. Each run (the question, each attachment, each retry) gets the full limit; the run itself cannot be cancelled, so it finishes in the background, returns its pipe, and its late answer is dropped. Before this a hung pipeline left the question with no outcome at all. Configuration and start failures now say what to fix: - a list setting that starts with "[" but is not valid JSON is reported in the task's warnings with the setting's name, instead of silently becoming a literal id that matches nothing; in guildIds or channelIds it fails the start, since the bot would otherwise answer nothing - a botToken that is still an unresolved ${NAME} fails the start naming the variable, instead of "login failed (invalid token)" - with includeMemberMetadata on, a refused privileged intent leads with the Server Members Intent (Message Content is usually already on) Found by the live engine suite (F36, F38, F39, F41); all verified live. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): add the full live engine suite for the discord node test_engine_full.py (F01..F46) walks every discord node feature, success and failure, through real pipelines on a running engine, driven by the second bot in the support channel only. Node behaviour runs on deterministic pipes: an echo pipe (discord -> response_text keyed answers) and a fake-model pipe pointed at fake_llm.py, a scripted OpenAI-compatible endpoint for scratchpad, final-envelope, error-text, empty, retry, HTTP 429 and slow answers. One case runs the production pipe without tool_slack and capture. The suite is opt-in (DISCORD_LIVE=1 and DISCORD_E2E_FULL=1) and skips offline. The driver bot gains attachments, embeds, replies, reactions, deletes, thread creation and a typing recorder; its posts suppress mentions by default. The live README records the 44-row result matrix, the findings fixed in this branch, and the residue a run leaves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): report capture rows left unwritten when the discord writer stops stop() waits a bounded time for the capture worker. When a write is stuck on the database the worker is still busy after the join, and the rows still queued are lost when the daemon thread dies with the process, with nothing logged and nothing counted. stop() now warns with the approximate number of unwritten rows in that case. Raised by CodeRabbit on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): label the discord tile by token presence, not connection The tile only tests whether botToken is set, yet read "connected" even when login failed, an intent was missing or the ${NAME} never resolved. It now reads "Token: configured" or "Token: missing", and the README says the task status line is what reports the connection. Raised by CodeRabbit on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): list the reply patterns the discord node treats as errors The error-suppression sentence chained several clauses and did not parse. It is now a list that matches _ERROR_SIGNATURES in text_utils, with the posting, retry and sanitizeReplies-off behaviour unchanged. Raised by CodeRabbit on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): parse live discord harness config through the real _run make_endpoint kept a hand-written copy of IEndpoint._run's config block that had drifted: escalation markers and feedback emojis were split on whitespace, several newer settings (mergeAttachments, numberChunks, teamMentionAlias, nonAnswerRetries, pipelineTimeoutSeconds, capture) were never applied, and numbers skipped the production coercion. It now runs the real _run(), stopped where it would reach the engine's shared web server, as the unit tests already do. The live README also marks B1 and G6 as fixed, pointing to N7 and F03. Raised by CodeRabbit on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): look up a discord reply's target with the real discord.py API The aimed-at-others check called Message.fetch_reference, which discord.py does not have. The AttributeError was swallowed, so the node never learned who a reply was for, and with ignoreAimedAtOthers on every reply without a mention was treated as aimed at someone else, including a reply to the bot's own answer: it got the ack emoji and no answer. The target now comes from reference.resolved (sent with the reply by the Gateway), then reference.cached_message, then channel.fetch_message(reference.message_id). The unit tests delete fetch_reference from the fake message, so a call to a method discord.py does not have fails the test instead of passing; the old test mocked it. Reported in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): add a live case for a reply to the discord bot without a mention F23b replies to the bot's own answer with the ping off, so only the reply-target lookup can tell the node the message is for it. The live README records the finding as N8; the case has not been run live yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(nodes): move discord support behaviours to a follow-up PR PR #1503 is being split into three stacked PRs. This removes the support behaviours from the base Discord node: thread history as context, the escalation pause and per-thread serialization, skipping messages aimed at others, feedback reactions, the team mention alias, reply sanitizing and error detection, non-answer retries, the pipeline timeout and backfill. Their services.json fields, config parsing, helpers, text utilities, README sections and unit tests go with them; the follow-up PR re-adds them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(nodes): move discord event capture to a follow-up PR The durable Postgres capture of the node's events moves to the third PR of the split. This deletes capture.py and its tests, the captureEvents, captureNodeId, captureTable and captureSource fields, and the capture writer lifecycle and call sites in IEndpoint. The engine-warning helper _config_warning relied on now lives in IEndpoint, so the node no longer imports the capture module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): fail the discord start on an unset variable in guildIds or channelIds The engine passes an unknown ${NAME} through as literal text. In guildIds or channelIds it became an allowlist id that matches nothing, so the bot connected and answered nobody. The start now fails, on the same path as the broken-JSON check, naming the field and the variable. In allowedMentionRoleIds and allowedMentionUserIds such an entry was only dropped with a debug line; it now also produces a task warning naming the field and the variable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): keep discord thread settings within what Discord accepts Discord rejects a thread name longer than 100 characters and any auto-archive duration other than 60, 1440, 4320 or 10080 minutes, either of which cost the whole answer with a 400. threadNameMaxLength now declares minimum 1 and maximum 100 and is clamped to that range in code. threadAutoArchiveMinutes is an enum of the accepted durations plus 0 for the channel default; the code still falls back to the channel default for any other value that reaches it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): say which discord text is broadcast and kept in the run log The README called the apaevt_sse broadcast live-only. It is not: the engine writes every SSE body into the task's run log, so it is visible to every client monitoring the task and kept afterwards. The README now says so and lists exactly which message text the node puts in those bodies (the question on message events, the answer on outbound events), so operators can cover it in their privacy notice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(nodes): add comment blocks to the discord services.json Every top-level key of the Discord services.json now has a // comment block explaining its purpose, in the same form as the Telegram node's. The file is JSONC from here on, so the unit tests and the live suite drop its whole-line comments before parsing it, and a schema test checks that each top-level key keeps its comment block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): remove internal data from the discord live suite The repo is public and the live folder carried internal data. The replay seeds are now ten made-up questions about a generic pipeline tool, the README is cut down to the layers, how to run them, the variables they need, the residue policy and what they do not cover, and the test pipe's prompt no longer names an internal bot. The token and id files are read from DISCORD_LIVE_TOKEN_FILE and DISCORD_LIVE_IDS_FILE instead of fixed home-directory paths, and the layers skip with a reason when they are unset. Result files go to a temp directory or a *_RESULTS_DIR override instead of a repo-local folder. The live cases for behaviours that moved to the follow-up PRs (D11, D16..D19 and F14, F20..F26, F32..F35, F40, F41b, F45, F46) and the helpers only they used are removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): regenerate the discord README settings table Generated by nodes:docs-generate after the support behaviours and capture moved to follow-up PRs and the thread settings gained their limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(nodes): add event capture to the discord node Re-add the opt-in durable capture of the node's events into a database node connected to the source by a tool control edge: capture.py, the captureEvents, captureNodeId, captureTable and captureSource fields, the capture writer lifecycle and its call sites next to every SSE broadcast, the README section, and the unit tests. IEndpoint shares capture._engine_warning again instead of keeping its own copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): keep every part of a discord message in the capture log A message row was keyed (message_id, 'message', '') for the text pass and for every attachment, so ON CONFLICT DO NOTHING kept only the first: the attachment rows were lost, or with mergeAttachments the question text was. The event_key of a message row now names the part: 'text' for the text pass, '<lane>:<groupIndex>' for each attachment, with ':retry:<n>' appended for a retried pass. Both come from the broadcast body, so a key rebuilt from the run log matches. Keys for the other event types are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): capture discord events only into PostgreSQL, inserting before any DDL The capture SQL is PostgreSQL's, but any database node was accepted, and the writer ran CREATE TABLE before its first INSERT, so a database user that may only insert into an existing table could not capture at all. While resolving the database node the writer now asks it for its dialect once; anything other than PostgreSQL turns capture off for the run with one warning naming the dialect. Each write is an INSERT first, and only an INSERT that fails because the table does not exist runs the CREATE TABLE and CREATE INDEX statements and retries the insert once. The README and the captureEvents description say PostgreSQL only and list the rights the database user needs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): stop the discord capture worker when the node stops A capture write stuck on the database outlived stop(): the worker went on borrowing pipes and writing the rest of the queue after the node had stopped, and with a full queue the stop sentinel never got in, so the thread never ended. The writer now has a stop event. stop() still gives the worker its timeout to drain, then sets the event (at once when the queue is full). The worker checks it before every write, so once set it borrows no further pipe, drops what is still queued and ends; the unwritten-rows warning now carries the exact count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): say which database node discord capture may use Capture writes through the database node's execute tool, so it needs allow_execute, which opens raw execute, begin, commit and rollback to every caller of that node. The README now says to use a separate database node only for capture, never one an agent or any other tool caller can reach, with a database user that has only the rights capture needs. The privacy paragraph now also says that with capture on the broadcast text is stored in the capture table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): add discord capture to the live suite F32..F34 run event capture through a real engine into a disposable PostgreSQL database, gated on the DISCORD_E2E_PG_* variables: - F32: with no table yet, the first insert creates discord_events, and the question and answer land with the configured captureSource. - F33: text plus a .md attachment keeps two message rows (text, text:1), and inserting the same rows again adds nothing. - F34: with the database stopped mid-run the bot still answers, and capture resumes once it is back. The live README lists the cases and the new variables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): regenerate the discord README settings table Generated by nodes:docs-generate after the capture settings came back and their descriptions gained "PostgreSQL only". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): list decoded text attachments among the broadcast discord text With mergeAttachments off, a text-like attachment is sent through the text lane as "[attachment <name>]" plus its decoded contents, and that text is broadcast on its message event (clipped at 2000 characters), so it lands in the run log too. The privacy list now names it. Raised by CodeRabbit on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): key discord no_reply capture rows by a stable reason code The no_reply dedupe key was the raw reason, which can be exception text that differs between deliveries of the same message, so a redelivery inserted a second row. A known reason code is now its own key; any other reason shares the key "error" and keeps its text in the payload. The dropped-row counter is also updated under a lock, since events are submitted from several worker threads at once. Raised by CodeRabbit on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): handle blank discord text, partial delivery and late messages Three edge cases in the base node: - The intake gate strips a message's text to decide whether it has anything to ask, but processing read the raw text, so whitespace next to a file ran a text pass of its own (and skipped the file-only framing when merging). Processing now uses the same stripped answer. - When a later chunk failed after earlier ones were posted, the outbound event looked like a full delivery. _send_response now reports whether every chunk went out, and the outbound event carries it as "complete". - A message arriving after shutdown began could start a task the shutdown never awaited. _on_message now ignores messages once closing. Raised by CodeRabbit on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): say which discord events need emitOutbound or emitNoReply The README implied an outbound or send_failed no_reply event always follows an answer. Both need their opt-in flag, which is off by default. It also documents the new "complete" field on outbound events. Raised by CodeRabbit on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): require confirmation before the discord capture cases touch a database F32 drops the capture table and F34 stops the database container, and until now any DISCORD_E2E_PG_* values were enough to run them. They now also need DISCORD_E2E_PG_DISPOSABLE set to exactly "<container>/<database>", and they skip when that database holds any table other than the capture table. Raised by CodeRabbit on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): fail clearly when the discord capture database cannot be checked When the disposable-database check could not query the database (the container is down), its error text was read as "other tables exist" and the case skipped. It now fails with the query error. F34 also runs the check before it starts a capture pipeline, so nothing captures into a database that turns out not to be disposable. Raised by CodeRabbit on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: keep the discord changelog line to what the base node ships The entry still listed thread history, escalation pause, ack and feedback reactions, reply hygiene and retries, and event capture, which moved to follow-up PRs. List only what this node ships. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): catch every unset variable in the discord settings The engine resolves only ROCKETRIDE_* variables and replaces any other ${NAME} with the literal <REDACTED>, which the node did not recognise: a token failed as "invalid token" and list entries matched nothing. Both forms are now named, and the message says only ROCKETRIDE_* server variables are resolved. requireMentionChannelIds now fails the start like guildIds and channelIds (an unset entry made the mention gate fail open), allowedBotIds warns like the mention lists, and JSON list items get the same strip and comma/whitespace split as a bare string. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): say how to keep strangers from adding the discord bot With guildIds and channelIds empty (the default) the bot answers in every server it is added to and in DMs, and a new Discord application is a Public Bot. The setup steps now say to turn off Public Bot, the README recommends setting guildIds in production and says DMs are answered only while both lists are empty, and the node warns once at start when guildIds is empty. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(nodes): bound how many discord messages are processed at once Every message that passed the gates started its own task, and each one holds its downloaded attachments (up to maxAttachmentBytes, up to 10 files) in memory until the pipeline answers, so nothing limited memory. A new maxConcurrentMessages setting (default 4, 1 to 32) sizes a semaphore created in _startup; further messages wait their turn rather than being dropped. maxAttachmentBytes gets a 100 MB maximum, enforced in code as well. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): never let a blank discord answer win over a real one Answers were chosen by truthiness, so a whitespace-only answer such as a newline beat a real attachment answer, and with merging on a blank text pass hid the first attachment answer. Every pipeline answer is now normalised the way the question text is before it is chosen, a blank attachment answer is no longer folded into the merged question, and _send_response treats an answer with no non-blank chunks as nothing to send. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): decode discord text attachments the same way on both paths With mergeAttachments off a text file was sliced to textAttachmentMaxChars (0 sent an empty body, a negative value dropped the end) and never checked for binary content; with it on, 0 or less kept everything and a NUL byte meant binary. Both paths now share one decode helper and one cap rule: 0 means no limit, and a file with a NUL byte (such as UTF-16) is not text. textAttachmentMaxChars gets a minimum of 0 in the schema. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(nodes): drop the unexplained discord.py version floor Every discord.py API the node uses exists in 2.0 and later, and node dependencies are resolved together into one constraints lock, so an arbitrary floor could move other packages for no reason. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): document what the discord send helpers return _send_response said it returned None but returns messageIds, destination and complete; the docstring now describes each key. _emit_outbound_event and _record_sent had no docstring. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): route discord files of unlisted types by their extension The extension table knows 15 extensions; any other file without a content type became application/octet-stream and went to the tags lane, so .m4a or .flac audio reached the document lane. After the content type and the table, the extension is now looked up with Python's mimetypes before falling back to application/octet-stream. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): regenerate the discord README settings table Generated by nodes:docs-generate for maxConcurrentMessages, the new bounds on maxAttachmentBytes and textAttachmentMaxChars, and the dependency line without a version floor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): skip queued discord messages once shutdown has begun A message waiting for a maxConcurrentMessages slot is already tracked as in flight, so shutdown waits for it; once it got its slot nothing checked that shutdown had begun, and it downloaded, ran the pipeline and replied anyway. It now returns as soon as it gets the slot (with a no_reply "shutdown" event when emitNoReply is on). The README notes that the slot limit is shared across servers and what it does not buy. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): warn about an open discord bot only when no allowlist limits it With channelIds set, only those channels and their threads are answered and DMs are refused, so the start-up warning that the bot answers in any server it is added to was wrong. It now fires only when guildIds and channelIds are both empty, and the README says the same. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): fail the discord start when an allowlist resolves to nothing The engine replaces a set ${ROCKETRIDE_X} with its value, so an empty value reaches the node as "". Blank items are dropped, the list became empty, and an empty guildIds, channelIds or requireMentionChannelIds means "everywhere". A list that was given items but resolves to no ids now fails the start with the field's name; a genuinely empty list (missing, None, [], '', '[]') still means all. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(nodes): let discord list settings keep phrases whole _as_str_list always split JSON items and bare strings on commas and whitespace, which suits ids but breaks a list of phrases. It now takes split=True; with split=False items are kept whole (still stripped, blank ones still dropped). Every current caller keeps the default. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): read UTF-16 and BOM-prefixed discord text files as text Any NUL byte meant "binary", so UTF-16 files (Windows Notepad "Unicode", PowerShell 5.1 redirects) were never read as text: with merging off they went to the binary lanes, with merging on they were dropped with only a debug line. A UTF-8 BOM also survived into the text. The decode now checks for a UTF-16 BOM first, otherwise decodes as utf-8-sig, and only then treats a NUL as binary content. In merge mode a text-like file that is still binary becomes its own binary lane object (reusing the bytes already downloaded) instead of being dropped. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): guess discord file types from Python's built-in table only mimetypes.guess_type also reads the Windows registry and /etc/mime.types, so the extension fallback routed the same file differently per host: on Windows .csv became application/vnd.ms-excel and .ts a video type (it is video/mp2t on macOS too). The fallback now asks a module-level mimetypes.MimeTypes(), which CPython fills from its built-in defaults only; the node's own extension table still answers first. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): accept only numeric ids in the discord allowlists An entry in guildIds, channelIds or requireMentionChannelIds that is not plain ASCII digits (a pasted <#123>, a channel or server name) matched nothing, so the mention gate failed open or the bot answered nothing. The start now fails naming the field and the entry, and suggests the id inside a pasted mention. The mention allowlists and allowedBotIds use the same digit rule, so '123²' no longer reaches int() in every send. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): ignore discord reactions once shutdown has begun The Gateway keeps delivering reactions while _shutdown waits for in-flight messages, and each one still opened a pipe. _on_raw_reaction now returns when _closing is set, and checks again just before the emit. The reaction authorIsBot now uses the same _reactor_is_bot answer as the gate, so a removal by an allowlisted bot is reported as a bot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): split prose in fenced discord replies at a word boundary A reply with a code fence anywhere goes through the fenced chunker, which only broke at a newline in the second half of the window and otherwise cut by character count, even in prose outside the code block. Outside an open fence it now falls back to the last sentence end, then the last whitespace, in the second half before cutting by count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): bound and normalise the discord attachment settings maxAttachmentBytes of zero or below skipped every attachment with only a debug line; it is now clamped to at least 1 and the schema declares minimum 1. textAttachmentExtensions entries typed without a dot (md, json) never matched os.path.splitext; each entry is now normalised to a lowercase extension with one leading dot, and blank entries are dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): coerce discord booleans and overflowing numbers from the engine _as_int caught TypeError and ValueError but not OverflowError, so a number setting of inf or 1e400 failed _run with an unrelated traceback; it now falls back to the default. The ten boolean settings were used raw, so a string-like 'false' read as True; a new _as_bool reads real bools as-is and true/false, 1/0, yes/no, on/off strings in any case, else the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): document discord events, reasons and setup accurately The tags lane also carries the reaction, no_reply and outbound event objects, and events have their own entry URL and name; the README now says so. The no_reply reasons are listed (no_answer, send_failed, shutdown, or an error message clipped to 200 characters), and so is what happens when the question is deleted before the answer is posted. Configuration headings use the field titles, each paragraph sits under the field it is about, and the broadcast section moves under Notes. The Developer Portal steps use Reset Token, the reactions intent is described as one the node turns on itself, and the MIME fallback examples are .avi and .bmp, which the node's own table does not list. In services.json, guildIds and channelIds default to [] and the text-attachment and emitNoReply descriptions match the behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): complete the discord docstrings and drop references to a private bot Several Args sections left out parameters the functions take (_on_message wait, _process_attachment meta and attachment_index, _run_text_pipeline meta, object_name and attachment_id, _run_binary_pipeline message_id, attachment_index and meta, should_process_message parent_channel_id and allowed_bot_ids). Comments and docstrings no longer point at functions of a bot that is not in this repo; the sentences explaining the behaviour stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(nodes): make the discord live suite neutral and self-describing The live README now lists the variables L3 and L4 need on the engine and says ROCKETRIDE_DISCORD_E2E_NO_SUCH_TOKEN must stay unset, and asks for Python 3.10+. engine_min.pipe gets a fresh project_id (the SDK's use() contract carries it at the top level) and a short generic assistant prompt with no escalation line. E05 tested that escalation prompt, which belongs to a follow-up change, and E06 was a skipping placeholder; both are removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): regenerate the discord README settings table Generated by nodes:docs-generate for the new bounds and list defaults and the updated textAttachmentExtensions and emitNoReply descriptions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): wait for discord reaction emits at shutdown and hint ids by kind A reaction emit already under way was not tracked, so shutdown did not wait for it and tore the endpoint down beneath it. The emit is now a tracked task, like a message handler. The second closing check before the emit could never fire (nothing can change the flag between the two checks) and its test only passed by setting the flag from a mock; both are gone. A pasted mention in an id list suggested the digits inside it even when they were the id of something else (a role in a channel list, a channel in the server list). The hint now gives the digits only for the right kind of mention and otherwise says what was pasted. Also documents the arguments of _emit_event_pipeline and the return of _emit_no_reply_event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): never cut a discord reply inside a code-fence opener or a word When a chunk window ended inside or just after a code-fence opener line, chunk 1 ended with a split language name or an empty code block. A block whose opener line and first code line do not both fit now starts the next chunk, and the newline search never cuts right after an opener line. A reply with no fence also hard-split a line longer than the limit in the middle of a word. It is now cut at the last sentence end or whitespace in the second half of the window, by character count only when there is none. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(nodes): read discord booleans with the shared parse_bool The node had its own _as_bool, which duplicated parse_bool in ai.common.utils. Every boolean setting now goes through the shared helper. The engine passes JSON booleans as real bool values, so the tests that fed string-like proxies to a boolean are replaced with tests for real bools and plain strings. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): keep secrets out of discord id errors and bound id length A bot token or secret ${ROCKETRIDE_*} value pasted into an id list was echoed in full by the start error, the task warnings and the debug log. Every message about a list entry now goes through one helper that shows a mention whole and anything else longer than 12 characters as its first 12 characters followed by an ellipsis. An id is also bounded to a 64-bit snowflake (at most 20 digits, below 2**64), so two ids pasted together are rejected as non-numeric on the same fatal or warning path as any other bad entry. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): treat a non-positive discord attachment cap as unset maxAttachmentBytes was floored at 1, but a 1-byte cap skips every real attachment just as 0 did. A value of 0 or below now means "not set" and falls back to the 25 MB default, matching config_int in ai.common.utils; the value is still clamped to at most 100 MB. The schema keeps its minimum of 1. Raised in review on #1503. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): regenerate the discord settings table Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(discord): let the capture edge be drawn in the pipeline editor The canvas draws a tool handle only for the keys of a service's invoke block, and the Discord source had none, so the database node capture writes to could only be wired by hand-editing the .pipe. Add the invoke capability and an optional invoke.tool slot (min 0, so a pipeline without capture needs nothing connected), shaped like the webhook tools source. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(discord): do not take a missing column for a missing capture table PostgreSQL's undefined-column error (42703) reads 'column "x" of relation "<table>" does not exist', which the unanchored pattern matched. A table of another shape then took the DDL path on every row: a CREATE that an INSERT-only user may not run, logged instead of the real error. Anchor the pattern to the start of the driver message; the 42P01 and UndefinedTable checks stay. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(discord): make INSERT the only grant capture needs on an existing table seq was BIGSERIAL, whose default calls nextval() and so needs USAGE on the table's sequence: an operator who pre-created the table and granted only INSERT got "permission denied for sequence" on every row. Declare seq as BIGINT GENERATED BY DEFAULT AS IDENTITY, which needs no sequence grant (PostgreSQL 10 or later; capture already writes to PostgreSQL only). The README and the captureEvents description now state the exact grants. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(discord): warn about allow_execute in the capture field descriptions Capture writes through the database node's execute tool, so it needs Allow direct query execution, which opens raw SQL to every caller of that node. The README said so, but the editor description, the only text a pipeline builder sees, did not, so the likely fix for "execute tool is disabled" was to enable it on the node an agent already uses. Say it in the captureEvents and captureNodeId descriptions. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(discord): drain the capture queue on a normal stop _run blocked on an event only a fatal Gateway error set, so on Stop the process was killed inside the wait and the daemon writer died with whatever was queued, often the last reply's outbound row. Stop sends SIGTERM, which sets the engine's cancellation flag, and the supervisor force-kills only five seconds later. Poll isCancelled() between bounded waits, as the webhook source does, so the Gateway close and the two-second capture drain run on a normal stop too. The README states what is still lost when teardown outlasts the kill. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(discord): strip NUL characters from captured rows PostgreSQL text rejects NUL and jsonb rejects the \u0000 json.dumps writes for it, so one NUL failed the row for good. A UTF-16 .txt decoded as text with errors='ignore' keeps its NUL bytes, so the question row was lost while the bot still answered. capture_row now scrubs NUL from every string in the metadata and payload (copies, so the SSE broadcast is untouched) before building text and payload. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(discord): say that captured rows are kept until the operator deletes them Capture never prunes, and a message deleted in Discord keeps its row, with the full text in payload. Operators need that for their privacy notice and for deletion requests, so the README now says retention and deletion are theirs, with an example scheduled DELETE. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(discord): tighten the capture table name and quote it in all SQL - The table-name and source-label patterns used match with ^...$, and $ also matches before a trailing newline, so 'discord_events\n' passed. Both now use fullmatch. - A reserved word such as 'user' passed the check and then failed every INSERT. The name is now lower-cased and double-quoted in all SQL, so Discord_Events still means discord_events. - Index and constraint names share the table namespace, so the index 'events_thread' for table 'events' collided with a table of that name. The derived names now use a '$' suffix, which no accepted table name can contain, and the docstring says so. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(discord): guard the F33 live capture case like the others F33 writes into the capture table directly, but unlike F32 and F34 it did not first check that the database holds nothing but that table. Call _require_disposable_database() before _start, as F32 does. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(discord): capture one message's text and attachment as two rows Drive a whole message (text plus a PDF) through _process_message with mergeAttachments off and on, and deliver it twice. Each part gets its own message row (text, binary:1) under the same message id, the second delivery sends the same keys again, and the table keeps two rows. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: add the discord event capture changelog line #2547 has its own line for the support behaviours; #2548 had none. Add one line for what this PR adds: optional capture of the node's events to a database through a database node. Raised in review on #2548. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): regenerate the discord settings table after the review fixes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): document the discord capture connection The node now declares an invoke slot for the capture database, so the README schema requires a Connections section with one row per invoke key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): drop the invoke capability from the discord source The "invoke" capability marks a node that other nodes invoke: the canvas draws a top target handle for it that nothing can be connected to. The capture edge to a database node comes from the invoke.tool block alone, and no engine code reads the capability on the calling side (it is only parsed into a flag; control edges are wired from the .pipe control array without checking it). Keep the block, drop the capability. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): check the discord capture table exists before the first insert On a new table the first capture write was an INSERT that failed, and the database node logs every failed raw statement at error level together with its bound parameters, so the user's question landed in the error log. Before the first INSERT of a run, ask once with SELECT to_regclass($1) (the quoted, lower-cased name; no privilege needed) and run the DDL only when it answers NULL. A check that raises is a failed write and is asked again on the next row; an answer that cannot be read keeps the existing INSERT-first path, and the missing-table fallback after a failed INSERT stays for a table dropped mid-run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): key discord capture rows on the base node's no_reply reasons only The capture key builder, its tests and the README listed no_reply reasons (non_answer, model_error, paused, aimed_elsewhere, timeout) and a :retry:<n> message key that only the follow-up node changes in #2547 produce. Keep this PR consistent with the node it ships against: the fixed reasons are no_answer, send_failed and shutdown, anything else is keyed error, and a message key is text or <lane>:<groupIndex>. The removed parts come back together with #2547. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): store the whole discord message text in capture rows A message row's text was the SSE payload, which the node clips at 2000 characters, while the docs said the payload is stored unclipped. Discord allows messages of up to 4000 characters, so a long question lost its second half in the durable record. The broadcast keeps its clip; the capture row now gets the user's whole message. In merge mode it gets only the user's own text: the folded file contents and other lanes' answers are pipeline input, not what the user wrote. README and comments now say which text is clipped where. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): count discord capture rows that race the writer's stop A handler that took the capture writer just before Stop could still call submit(): either it saw the writer stopped and returned without a word, or it queued its row behind the stop marker, where the worker never reads it. Both lost a row silently. submit() now checks and queues under the same lock stop() holds while it closes the writer, so no row can land behind the marker, and a row submitted after stop() is counted in unwritten and reported in a throttled warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(nodes): share one _shown_entry and _engine_warning in discord text_utils capture.py carried its own copy of _shown_entry (without the endpoint's rule that a mention is shown whole) and owned _engine_warning, which the endpoint imported from it. Both now live once in text_utils.py, which has no engine or discord.py imports, and capture.py and IEndpoint.py import them from there; the endpoint's behaviour is kept. The bare capture loader in test_capture.py now loads the module inside a synthetic package so its relative import resolves, as the other loaders already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): correct the discord capture comments, docs and live F34 check - Comments no longer say a no_reply reason becomes the dedupe key: an exception-text reason is keyed error, and the 200-character clip keeps the event bounded. - captureEvents description: failures are logged in full once, then at most once per 30 seconds; the "follow-up" event type is gone (it is not in this PR). - CHANGELOG: outbound, no_reply and reaction rows need their emit settings on. - README: the exact DDL capture runs, for operators who create the table in advance, with the UNIQUE (message_id, event_type, event_key) constraint marked required (ON CONFLICT is refused without it); a test keeps the block equal to CREATE_TABLE_SQL. The stop paragraph states the real budget: stopping the writer takes up to about 4 s (2 s to queue the stop marker, 2 s to join) of the engine's 5 s. - Live F34 now passes only when the failed and recovered log lines are found, as the live README describes, and is skipped without DISCORD_E2E_ENGINE_LOG. F32's wording follows the table check that now runs before the first insert. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(nodes): regenerate the discord settings table after the review fixes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): key discord capture rows on the support behaviours' reasons and retries Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(nodes): one rule for discord send pings and accurate contextChars docs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): keep folded files out of a retried discord capture row A non-answer retry re-ran the text pass without its capture text, so a retried merge pass of a message that is only attachments stored the folded file contents in its message row. The text pass now carries a captureText (the user's own words) that every retry passes on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): let an INSERT-only role write discord capture rows PostgreSQL requires SELECT on the columns of a named ON CONFLICT target, even for DO NOTHING, so a database user granted only INSERT got "permission denied" for every capture row (measured on PostgreSQL 16). The insert now uses a target-less ON CONFLICT DO NOTHING: the table's UNIQUE constraint still turns a redelivered event into a conflict. The README, services.json and docstrings now say INSERT alone is enough, and that a hand-made table without the UNIQUE constraint stops deduplicating rather than failing every row. The live suite gains F32b, an INSERT-only role writing into an existing table, and F33 repeats its insert the way the node now writes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): keep each discord source's capture rows apart Two Discord sources writing to one capture table and seeing the same message produced the same (message_id, event_type, event_key), so the second source's row was dropped as a redelivery. The UNIQUE dedupe key is now (source, message_id, event_type, event_key); a redelivery to the same source is still ignored. The README DDL block (which a test keeps equal to CREATE_TABLE_SQL) and the row-key description follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): stop discord capture after a failure that cannot fix itself The database node logs every failed statement at error level together with its bound values, so a capture table that could never be written (a refused grant, a missing column) put every user's message into the server log, once per row, for the whole run. The writer now tells the two kinds of failure apart. A failure no retry can fix (permission denied, no matching unique constraint, a missing column, a missing table whose CREATE fails, the execute tool switched off, a connected node that is not a database) turns capture off for the run with one warning naming the cause, never the row's text. Any other failure, usually the database being down, starts a 60-second backoff in which rows are dropped and counted without borrowing a pipe; the next row after it is tried again, and the recovery line reports how many were dropped. The README privacy paragraph now says a failed capture write is logged by the database node with the row's values, and that capture stops after a lasting failure. The live F34 case waits out the backoff before it checks that capture resumed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): check discord captureNodeId against the connected tool nodes A configured captureNodeId was used without looking at the source's tool edge, so a typo, or an id wired to something else, failed every capture row for the whole run. The id is now checked once against getControllerNodeIds('tool'); one that is not there turns capture off with one warning, its value shown clipped like any configured entry. A node that lets the dialect call through without an answer (one that does not own the tool) is treated as not a database: capture goes off with one warning, as for a "not owned" error. A driver message that quotes the configured node id shows <captureNodeId> instead, since the field may hold a pasted secret. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): store no exception text in a discord no_reply capture row A no_reply whose reason is an exception message (a pipeline, download or unexpected failure) carried that message into the capture row's payload. Exception text can hold account details, key fragments or internal URLs, and the capture table is kept far longer than the run log. The row now stores reason "error" in its payload, matching its event_key; the SSE broadcast is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): bound the strings a queued discord capture row holds The capture payload kept the text the node handed over unclipped, and a text attachment read with textAttachmentMaxChars 0 has no limit of its own, so each of the up to 1000 queued rows could hold megabytes. Every string in the stored payload is now clipped at PAYLOAD_TEXT_MAX_CHARS (65536, ending in an ellipsis), which is far above any Discord message or reply. The text column keeps its own 8000-character clip. The README and the module docstring's "Bounded" property now say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nodes): date a discord message capture row by when it was sent occurred_at was the moment the node handled the event, so a message replayed by backfill at startup was dated to the restart rather than to when it was written. A message row now takes Discord's createdAt from its metadata, and falls back to the node's time when that is absent, unreadable, or has no time zone. Outbound, no_reply and reaction rows carry the question's metadata, so they keep the node's time. The README now defines occurred_at and captured_at. Co-Authored-By: Claude Opus 5.5 <…
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
4 participants
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.
Summary
Adds the
discordsource node: a Discord bot connected to a pipeline over the Discord Gateway, the Discord counterpart to thetelegramnode. It routes messages and attachments into the pipeline's lanes and posts the answer back as a reply, a channel message or a thread.At review this PR was split into three, to keep each review focused:
Parts 2 and 3 are stacked on this branch and will be retargeted to
developonce this merges.tool_discord(the sender tool node) lands separately fromfeat/RR-1502-tool-discord.What the node supports
{content}, or the first attachment's name for a file-only message) capped at 100 characters, and an optional archive duration (60, 1440, 4320 or 10080 minutes); long answers split at 2000 characters with balanced code fences, breaking between lines inside a code block, and optional(i/n)labels; a failed thread creation falls back to a reply.@mentiongating that ignores@everyone/@here, other bots ignored unless listed inallowedBotIds; Discord system messages and empty messages are dropped.@everyone/@here.reaction,no_replyandoutboundevent objects with the message's metadata, also broadcast asapaevt_sseevents (and therefore kept in the task's run log; the README lists which message text that includes); a reply that could not be posted reportssend_failed.${VAR}(or the engine's<REDACTED>) inguildIds/channelIds/requireMentionChannelIds, broken JSON in a list setting, and a refused privileged intent each fail the start or warn with the setting and the fix. An emptyguildIdslogs a warning, since the bot then answers in any server it is added to.Settings
discord.botTokendiscord.guildIds[]discord.channelIds[]discord.ignoreBotstruediscord.requireMentionfalsediscord.replyModereplydiscord.showTypingtruediscord.maxAttachmentBytesdiscord.sendResponsestruediscord.requireMentionChannelIds[]discord.allowedBotIds[]discord.allowedMentionRoleIds[]discord.allowedMentionUserIds[]discord.threadNamePipeline Responsediscord.threadNameMaxLength90discord.threadAutoArchiveMinutes0(channel default)discord.numberChunksfalsediscord.textAttachmentExtensions[]discord.textAttachmentMaxChars12000discord.mergeAttachmentsfalsediscord.emitReactionsfalsediscord.emitNoReplyfalsediscord.emitOutboundfalsediscord.includeMemberMetadatafalsediscord.maxConcurrentMessages4Every setting beyond the first nine is opt-in: its default keeps the earlier behaviour, except
maxConcurrentMessages, which bounds how many messages are processed at once (more wait their turn) so large uploads cannot grow memory without limit.Review fixes in this PR
From the latest review:
DISCORD_LIVE_TOKEN_FILE/DISCORD_LIVE_IDS_FILEinstead of fixed paths.${VAR}inguildIdsorchannelIdssilently blocked every messagethreadNameMaxLengthis 1 to 100 (also clamped in code);threadAutoArchiveMinutesis an enum of the valid values.services.jsonhad no//comment blockstelegram/services.json.The capture and support-behaviour points from the same review are fixed in #2548 and #2547.
From the review that approved this PR (all non-blocking):
<REDACTED>, two list settings and padded JSON itemsrequireMentionChannelIdsfails the start,allowedBotIdswarnsguildIds), and a warning whenguildIdsis emptymaxConcurrentMessages(default 4) and a 100 MB cap onmaxAttachmentBytesdiscord.py>=2.4.0floormimetypes, with common Discord audio/video types in the node's own tableFrom the second approval (all non-blocking), plus a pre-review of the whole PR before pushing:
no_replyshutdown); reaction emits are tracked so shutdown waits for themchannelIdslimited the bot_as_str_list(..., split=False)maxAttachmentByteshas a minimum; extensions work without a dot; prose next to a code block is split at a word; the README documents the event lane, everyno_replyreason and the current Developer Portal steps; docstrings are complete; the live suite is neutral and lists the engine variables it needsFrom the third approval (all non-blocking):
_as_boolduplicatedparse_boolai.common.utils.parse_boolmaxAttachmentBytesfloor skipped everythingconfig_intFixed earlier in this PR after a full live run: long code answers no longer split mid-line, numbered chunks carry no trailing blank lines, broken JSON in a list setting is reported, an unset token variable is named, and the members-intent error leads with Server Members.
Follow-ups
send_failedreason.tool_discordas its own PR.Type
feature (new pipeline source node)
Testing
nodes/test/discord/394 passed on this branch. The live suites skip unlessDISCORD_LIVE=1.nodes/test/discord/live/holds the live Discord and engine layers for the base node; a live run of the split branches against a real server is scheduled.ruff checkandruff format --checkclean;validate-node-readme.pypasses; the README settings table is regenerated withnodes:docs-generate;./builder docs:checkpasses.Checklist
Linked Issue
Closes #1502
Generated with Claude Code
Summary by CodeRabbit