fix(streaming): keep Discord edits under 2000 chars + cold-start tolerance for waitForReady - #65
fix(streaming): keep Discord edits under 2000 chars + cold-start tolerance for waitForReady#65moyarzun wants to merge 2 commits into
Conversation
…rance
The periodic stream update and the final response edit were both building
`${contextHeader}\n📌 **Prompt**: ${prompt}\n\n${body}` and trying to
`message.edit()` it without checking length. Once the model's accumulated
output grew past ~1700 chars the full string exceeded Discord's 2000-char
per-message limit and Discord rejected the edit with
`Invalid Form Body content[BASE_TYPE_MAX_LENGTH]`. The bot then looked
unresponsive: the initial message stayed pinned at "🚀 Starting OpenCode
server..." even though OpenCode was happily running.
A second, related issue surfaced while reproducing this: `waitForReady`
calls `fetch` on the readiness probe with no per-request timeout, so a
single slow/hanging request can eat the entire 30s budget before the loop
ever iterates. The default 30s is also tight for opencode serve's cold
start (loading models, indexing).
Changes:
**src/utils/messageFormatter.ts**
- Export `splitIntoChunks` (was private) so callers and tests can reach it.
- Export `DISCORD_MAX_LENGTH` and a new `splitForDiscordTemplate({ header,
prompt, body, maxLength })` helper that returns a single `prefixBody`
guaranteed to fit in maxLength plus any `overflowChunks` that should be
sent as follow-up messages.
**src/services/executionService.ts**
- The initial `channel.send` now defensively truncates the prompt and has a
try/catch that returns a clear error to the user instead of silently
bailing.
- `updateStreamMessage` and the periodic interval no longer concatenate
header + prompt + body themselves — they delegate to
`splitForDiscordTemplate`, so every `edit()` is guaranteed under 2000.
Overflow is sent as follow-up messages.
- Final response edit (onSessionIdle) and the connection-error path use the
same helper, so a long final answer is no longer truncated silently.
**src/services/serveManager.ts**
- `waitForReady` now passes `signal: AbortSignal.timeout(5000)` to its
readiness probe so a single slow request cannot consume the entire wait
budget. `AbortSignal.timeout` failures fall through to the retry loop
like any other transient failure.
- Default timeout bumped from 30000 → 60000 ms to absorb opencode serve's
cold start (model loading). Callers that pass an explicit timeout
(commands/session, buttonHandler) are unaffected.
- Timeout error message mentions the cold-start possibility so users know
to retry.
**Tests**
- New unit tests in messageFormatter.test.ts for `splitIntoChunks`
boundaries and `splitForDiscordTemplate` budget behaviour.
- serveManager.test.ts updated to use `expect.objectContaining` for the
fetch call (the new `signal` field is non-trivial to assert exactly),
plus three new cases: signal/AbortSignal passed, AbortSignal.timeout is
treated as a retryable failure, and a 45s cold-start succeeds with the
new 60s default.
Mitigation for the Discord REST timeouts observed while streaming long responses. Two related improvements: 1. `streamEditInFlight` gate. `updateStreamMessage` now returns early (without enqueueing) if a previous edit is still pending in discord.js's SequentialHandler queue. Before, the 1-second interval would keep firing `message.edit()` calls regardless of whether the previous one had resolved, so a single slow edit could pile up to ~6 concurrent requests in the queue. If one of them stalled (Cloudflare keep-alive drop, transient network blip, etc.) they would all time out in cascade at undici's 10s default `connectTimeout` and the stream message would freeze on whatever was last successfully edited. 2. 8s hard timeout per edit. Each `streamMessage.edit()` is raced against an explicit 8s timeout that rejects if the underlying REST request hangs. discord.js's edit() does not accept AbortSignal natively, so we race against a setTimeout instead. This fails the edit fast (instead of waiting for undici's 10s default), frees the `streamEditInFlight` gate so the next tick can try again with a fresh connection, and emits a clear log line identifying the cause. The two changes together keep the REST queue bounded to at most one in-flight edit at a time and bound the wait for any single edit to 8 seconds, instead of letting 6+ requests pile up and all time out together at 10s.
RoundTable02
left a comment
There was a problem hiding this comment.
Thanks for the detailed report and test coverage. I confirmed that issue #64 is real: on v1.5.3, a 200-character prompt plus a long streamed response produces a 2,178-character edit payload, and a 1,950-character prompt produces a 2,022-character initial payload.
The general direction of this PR is sound, and the existing suite passes (183/183) along with the TypeScript build. However, I am requesting changes because targeted regression probes found several blocking problems:
- the production
runPromptpath still explicitly uses the legacy 30-second readiness timeout; - periodic streaming updates send overflow chunks as new messages on every tick, duplicating output and potentially amplifying Discord mention/notification spam;
- if the final edit fails, the beginning of a long response is never sent;
- the formatter does not uphold its advertised 2,000-character invariant for long but valid Discord prompts;
- the 8-second
Promise.racedoes not cancel the underlying Discord request, so pending edits can still accumulate.
Please keep streaming updates edit-only, deliver overflow once from the final-response path, preserve the complete response on edit failure, budget/truncate the entire prefix, and add integration-level tests around runPrompt for these cases. Since model/user text is sent through new channel.send() calls, please also disable parsed mentions (for example via a global or per-message allowedMentions policy).
| console.error(`stream edit exceeded ${STREAM_EDIT_TIMEOUT_MS}ms; abandoning wait and continuing`); | ||
| return false; | ||
| } | ||
| for (const chunk of overflowChunks) { |
There was a problem hiding this comment.
[P1] Do not send overflow chunks from the periodic updater. updateStreamMessage is called by the 1 Hz interval below, so once output exceeds the edit budget, every qualifying tick sends the current overflow again as new Discord messages. A regression probe produced the same follow-up chunk on two consecutive ticks. This can spam the channel and, because no allowedMentions restriction is configured, can also repeatedly trigger mentions contained in model output. Keep streaming updates to a capped edit-only preview and send overflow exactly once from the final-response path.
| }, STREAM_EDIT_TIMEOUT_MS); | ||
| }); | ||
| try { | ||
| await Promise.race([editPromise, timeoutPromise]); |
There was a problem hiding this comment.
[P2] Promise.race does not cancel editPromise. After the timeout rejects, the underlying discord.js request is still pending in its sequential route queue, but the finally block clears streamEditInFlight, allowing another edit to be enqueued. With a never-settling mocked edit, the call count increased from 3 to 4 after the 8-second timeout while the previous request remained unresolved. Keep the gate held until the real request settles, or implement the timeout at a transport layer that can actually abort the request.
|
|
||
| await updateStreamMessage(`${contextHeader}\n📌 **Prompt**: ${prompt}\n\n⏳ Waiting for OpenCode server...`, [buttons]); | ||
| await updateStreamMessage('⏳ Waiting for OpenCode server...', [buttons]); | ||
| await serveManager.waitForReady(port, 30000, effectivePath, preferredModel); |
There was a problem hiding this comment.
[P1] This explicit 30000 argument bypasses the new 60-second default in waitForReady, so the production path still fails at 30 seconds and the cold-start half of issue #64 remains unfixed. Remove the explicit timeout (or pass the shared 60-second value) and add a runPrompt-level test that asserts the effective timeout.
| return false; | ||
| }); | ||
|
|
||
| const remaining = overflowChunks.length > 0 |
There was a problem hiding this comment.
[P1] When overflowChunks is non-empty and the edit fails, this branch sends only the overflow. The portion placed in prefixBody is never delivered, so the beginning of the final answer is lost. I reproduced this by placing a marker at the start of a 5,000-character response and forcing the final edit to reject; the completion marker was sent, but the start marker appeared in no message. On edit failure, send all original response chunks (or re-split the full body without assuming the edited prefix was delivered).
| const overhead = prefixTemplate.length; | ||
| // Reserve a few chars for the "\n..." ellipsis when truncating. | ||
| const footerReserve = 4; | ||
| const bodyBudget = Math.max(100, maxLength - overhead - footerReserve); |
There was a problem hiding this comment.
[P1] Math.max(100, ...) breaks the function's stated maximum-length guarantee when the prefix consumes most of the budget. With the normal header, a valid 1,900-character Discord prompt, and a long body, prefixBody.length is 2,046. The prompt/header must be truncated or split as part of the same total budget; do not force a positive body budget after the prefix has exhausted maxLength. Please add a boundary test covering a near-2,000-character prompt plus a long body.
Fixes #64.
What
Two related bugs in the streaming code path that surface together as soon as the model's accumulated output grows past ~1700 chars, or when
opencode servetakes more than 30 s to come up on its first run after a bot restart:${contextHeader}\n📌 **Prompt**: ${prompt}\n\n${body}and callmessage.edit()without any length check. Discord rejects withInvalid Form Body content[BASE_TYPE_MAX_LENGTH], the bot just logs and returns, and the message stays frozen on the initial🚀 Starting OpenCode server.... The user sees no progress and no final answer.waitForReadycallsfetchon its readiness probe with no per-request timeout — a single slow probe can swallow the whole 30 s budget before the loop iterates. The 30 s default is also too tight for opencode serve's cold start (model/provider loading).How
splitForDiscordTemplate({ header, prompt, body, maxLength })insrc/utils/messageFormatter.tsreturns a singleprefixBodyguaranteed to fit inmaxLengthplus anyoverflowChunksthat should be sent as follow-up messages. (splitIntoChunksis also exported now.)message.edit()insrc/services/executionService.ts— initial send, periodic updates, final response, connection-error and OpenCode-error paths — goes through the helper, so no path can push past Discord's 2000-char limit. The initial send additionally truncates the user prompt defensively and surfaces a clean error instead of bailing silently.src/services/serveManager.ts:waitForReadynow passessignal: AbortSignal.timeout(5000)to the probe fetch, the default timeout is bumped from 30 s → 60 s, and the timeout error message now mentions the cold-start possibility. (isOrphanedServerRunningandisServerRespondingwere already usingAbortSignal.timeout—waitForReadywas just missed.)Tests
splitIntoChunks(paragraph / single-newline / hard-split / leading-newline stripping) andsplitForDiscordTemplate(budget kept under limit, overflow emitted, custommaxLength, empty body).serveManager.test.tsupdated to useexpect.objectContainingfor the fetch call (the newsignalfield isn't easy to assert exactly), plus three new cases:signal: AbortSignalis passed, anAbortSignal.timeoutrejection is treated as a retryable failure, and a 45 s cold-start succeeds under the new 60 s default.Note for the maintainer
Per
CONTRIBUTING.mdI did not bump the version or touchCHANGELOG.md— left that for you. Happy to fold any naming / structure adjustments in if you'd like the helper to live somewhere else or the error copy to read differently.