fix: bot stuck in "typing" state with no request in flight - #17
fix: bot stuck in "typing" state with no request in flight#17nerualauren wants to merge 3 commits into
Conversation
Discord's typing state expires after ~10s, so a bot that appears to type
forever is not showing a stale indicator -- something is actively re-sending
it every 8 seconds. That something is an orphaned setInterval in
DiscordConnector.startTyping.
Two ways one got orphaned. Both are reachable because callers treat typing as
fire-and-forget: agent/loop.ts calls `startTyping(...).catch(() => {})`, and
processBatch never awaits its activationPromise, so two activations on the same
channel can overlap.
1. DOUBLE START. startTyping overwrote typingIntervals[channelId] without
clearing what was already there. The first interval was evicted from the map
while still running, so stopTyping -- which looks the channel up in that map
-- could never reach it again.
2. STOP-BEFORE-START RACE. startTyping awaited channels.fetch() and
sendTyping() BEFORE registering its interval. A fast activation reached
stopTyping while the map was still empty, cleared nothing, and then the
interval registered afterwards with nobody left to stop it. This is the one
that produces the reported symptom exactly: typing forever, nothing running.
FIX: a per-channel generation counter, claimed synchronously before any await.
Every start and every stop bumps it, so a startTyping that was overtaken --
whether by a newer start or by a stop -- notices at each await boundary and
declines to register. startTyping also clears any existing interval before
installing its own, so nothing is ever dropped while running. destroy() clears
both maps.
Note the failure was invisible from the map's side: an orphaned interval is
precisely the one NOT in typingIntervals. A first version of the leak test
asserted `typingIntervals.size === 0` and passed against the bug for that
reason; it now asserts on the timer count instead, measured as a delta because
the connector opens two timers of its own at construction.
Tests: src/discord/connector.typing.test.ts, five cases including a control
that fails if the fix simply stopped typing altogether. Mutation-checked --
neutering the generation guard fails exactly three of them, restoring it passes
all five. Full suite 477 passed. tsc unchanged (3 pre-existing errors in
src/llm/membrane/adapter.ts, present at HEAD, untouched here).
Greptile SummaryThe PR adds per-channel generations and interval replacement to prevent overlapping or overtaken typing starts from orphaning refresh timers. It also adds focused fake-timer coverage for normal stopping, double starts, stop-before-start ordering, timer cleanup, and continued typing while work is active.
Confidence Score: 4/5The shutdown race should be fixed before merging because an in-flight activation can recreate a matching generation and install a typing timer after connector cleanup. Clearing the per-channel counter resets its numeric identity while fire-and-forget activations remain alive, allowing a later stop to recreate the pending start's token and defeat shutdown invalidation. Files Needing Attention: src/discord/connector.ts
|
| Filename | Overview |
|---|---|
| src/discord/connector.ts | The generation guards address the typing races, but clearing their map during shutdown permits token reuse and a post-cleanup interval leak. |
| src/discord/connector.typing.test.ts | The new tests cover active typing races and timer cleanup but do not exercise a pending start across connector shutdown. |
Sequence Diagram
sequenceDiagram
participant A as In-flight activation
participant C as DiscordConnector
participant D as Discord API
participant S as Shutdown
A->>C: "startTyping(channel), generation = 1"
C->>D: channels.fetch / sendTyping
Note over C,D: start remains pending
S->>C: close()
C->>C: clear intervals and generations
A->>C: stopTyping(channel)
C->>C: absent generation becomes 1
D-->>C: pending operation resolves
C->>C: generation 1 still matches
C->>C: install interval after cleanup
Prompt To Fix All With AI
### Issue 1
src/discord/connector.ts:1818
**Shutdown reuses generations**
When an activation has `startTyping` pending during shutdown and later calls `stopTyping`, clearing this map lets the stop recreate the pending start's generation value. The pending start then passes its guard and installs a refresh interval after cleanup, leaving a live timer that sends typing requests through a destroyed connector.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix: bot stuck in "typing" with no reque..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Bumping generations is not enough on shutdown -- drop them, so a | ||
| // startTyping still awaiting Discord cannot register an interval into a | ||
| // connector that is going away. | ||
| this.typingGenerations.clear() |
There was a problem hiding this comment.
When an activation has startTyping pending during shutdown and later calls stopTyping, clearing this map lets the stop recreate the pending start's generation value. The pending start then passes its guard and installs a refresh interval after cleanup, leaving a live timer that sends typing requests through a destroyed connector.
Knowledge Base Used: Discord integration
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/connector.ts
Line: 1818
Comment:
**Shutdown reuses generations**
When an activation has `startTyping` pending during shutdown and later calls `stopTyping`, clearing this map lets the stop recreate the pending start's generation value. The pending start then passes its guard and installs a refresh interval after cleanup, leaving a live timer that sends typing requests through a destroyed connector.
**Knowledge Base Used:** [Discord integration](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/antra-tess/chapterx/-/docs/discord-integration.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.greptile's review on PR antra-tess#17 found a real bug in my first fix, and following it down showed the fix was the wrong SHAPE. THE REPORTED BUG, confirmed by test before fixing: the per-channel generation counter was derived from its own stored value, so close() clearing the map reset its numeric identity. A stopTyping arriving afterwards recomputed (undefined ?? 0) + 1 === 1 -- exactly the generation a pending start still held. That start's guard passed and it installed a refresh interval on a destroyed connector. Classic ABA: the token was not unique, only locally fresh. My own comment had claimed clearing was SAFER than bumping; it was precisely backwards. THE REAL FIX. A generation counter makes the race survivable. It does not remove it. The race existed only because startTyping awaited channels.fetch() and sendTyping() BEFORE registering its interval, and callers are fire-and-forget. So: delete the gap rather than guard it. Nothing is awaited before the map write, the async work moved inside the tick, and the timer handle itself is the identity token -- a Timeout is created by the runtime and cannot be forged, re-derived, or re-minted the way a number can. The invariant is now checkable in one line: A TYPING TIMER EXISTS IF AND ONLY IF IT IS IN typingIntervals. Both start and stop mutate that map synchronously, so there is no window for a stop to be missed or a start to register behind one. The generation counter, the monotonic sequence and their whole surface are gone. Mutation-tested, and it found something: making registration async again fails a test, removing the replace-before-register fails a test, but dropping the post-await identity re-check survived everything. It was untested, not dead -- the other tests watch TIMERS and that one guards a stray SEND. Now pinned, with an honest comment: sending is asynchronous, so a stop can always land mid-send and you cannot un-send. The check narrows that window rather than closing it, and the residual is one indicator Discord expires by itself in ~10s. 7 typing tests, full suite 479 passed, tsc unchanged (3 pre-existing errors in src/llm/membrane/adapter.ts). The connector's typing surface is 83 lines including all of the reasoning above.
|
Good catch — confirmed by test before fixing, and it was real: clearing the generation map in Following it down showed my fix was the wrong shape, so I've redesigned rather than patched. A generation counter makes the race survivable; it doesn't remove it. The race existed only because The invariant is now one line: a typing timer exists iff it is in Mutation testing found one more thing worth reporting: making registration async again fails a test, and removing the replace-before-register fails a test, but dropping the post-await identity re-check survived everything. It was untested rather than dead — the other tests watch timers, and that one guards a stray send. Now pinned, with an honest note that sending is async, so a stop can always land mid-send and you can't un-send; the check narrows that window rather than closing it, leaving one indicator that Discord expires by itself in ~10s. 7 typing tests, full suite 479 passed, |
Lauren: clean up comment text that is only relevant to someone deciding whether to merge, and not to future readers of the files. The load-bearing content turned out to be shorter as RULES than as history. A maintainer needs 'never await before the map write' and 'compare the handle, not a counter'; they do not need what the previous version did, what a reviewer found, or which PR it happened in. Removed: the version-comparison narrative, the ABA post-mortem, the greptile/PR references, and the asides about my own earlier wrong checks. Kept: the invariant, the two rules that preserve it, the fire-and-forget caller constraint that makes them necessary, and the honest note that the post-await re-check narrows a window rather than closing it. 7 typing tests, 479 full suite, tsc unchanged.
1 participant
Bots sometimes sit in Discord's "typing" state with nothing in flight. Since Discord's indicator expires after ~10s, a stuck one isn't a stale UI state — something is actively re-sending it. That something is an orphaned
setIntervalinDiscordConnector.startTyping.Two ways it gets orphaned, both reachable because typing is fire-and-forget (
startTyping(...).catch(() => {}), andprocessBatchnever awaits itsactivationPromise, so two activations on one channel can overlap):startTypingoverwrotetypingIntervals[channelId]without clearing the old one, evicting a still-running interval from the map thatstopTypingthen couldn't reach.startTypingawaitschannels.fetch()andsendTyping()before registering its interval. A fast activation callsstopTypingwhile the map is still empty, clears nothing, and the interval registers afterwards with nobody left to stop it. This is the one that matches the symptom exactly.Fix: a per-channel generation counter, claimed synchronously before any await. Every start and every stop bumps it, so a
startTypingthat was overtaken — by a newer start or by a stop — notices at each await boundary and declines to register.startTypingalso clears any existing interval before installing its own;destroy()clears both maps.Tests: 5 cases in
connector.typing.test.ts, including a control that fails if the fix simply stopped typing altogether. Mutation-checked — neutering the generation guard fails exactly 3, restoring it passes 5. Full suite 477 passed.tscunchanged (3 pre-existing errors insrc/llm/membrane/adapter.ts, verified identical at HEAD).One note worth flagging: my first leak test asserted
typingIntervals.size === 0and passed against the buggy code — an orphaned interval is precisely the one not in that map. It now asserts on timer count, as a delta, since the connector opens two timers of its own at construction.I don't have a reproduction from a live bot, so the causal chain is read from source plus the tests above rather than observed in production — if you've seen it stick in a channel with only one active conversation, that would point at the race specifically.
Written by Loom@flowerbox.local, a Claude instance working with Lauren. Happy to adjust anything here.