fix: suppress NO_REPLY streaming with unified quiet mode#127
Merged
Conversation
Replace nudgePending Set with quietState Map that fully suppresses all streaming output (deltas, tool activity, status, permissions) until we determine if the response is NO_REPLY. Key changes: - Buffer just track hasContent flag. Use assistant.messagenothing content (authoritative) for flush, not accumulated deltas. - Skip empty assistant.message events (SDK tool-call signals) - Defer stream creation in scheduler wrapper (no Working... flash) - Auto-deny permissions during quiet mode - 60s timeout safety net per quiet state entry - Clear on: non-empty assistant.message, session.error, session.idle Applies to both startup nudges and scheduled tasks. Closes #119 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move QuietState, enterQuietMode, exitQuietMode from inline definitions in index.ts to src/core/quiet-mode.ts. Add getQuietState() and isQuiet() accessors. Wire index.ts to use the new module. Tests cover: enter/exit lifecycle, channel isolation, cleanup function idempotency, 60s timeout safety net, hasContent tracking, threadRoot preservation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hrough
Address two code review findings:
1. Permission/user-input auto-deny during quiet now calls
resolvePermission(false) / resolveUserInput('') instead of just
prevents session from blocking on unresolved promise.returning
2. session.idle events pass through quiet mode so markIdle() fires
and waitForChannelIdle() resolves normally. Other status events
remain suppressed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a unified “quiet mode” mechanism to fully suppress streaming output (especially assistant.message_delta) until the bridge can determine whether a response is NO_REPLY, preventing leaked NO_REPLY text in DM channels after restart.
Changes:
- Added a new
quiet-modecore module to track per-channel quiet state with a timeout safety net. - Updated
handleSessionEvent()to suppress streaming/tool/status output during quiet mode and flush only once real content is confirmed. - Wired quiet mode into startup nudges and scheduled tasks, replacing the old
nudgePendingSet behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/index.ts | Integrates quiet mode into scheduler + admin nudge flows and adds quiet-mode suppression/flush logic inside session event handling. |
| src/core/quiet-mode.ts | New quiet-mode state tracker with per-channel state + 60s timeout cleanup. |
| src/core/quiet-mode.test.ts | Adds Vitest coverage for quiet mode lifecycle, timeout behavior, and state tracking. |
Comments suppressed due to low confidence (2)
src/core/quiet-mode.ts:29
- The 60s quiet-mode safety timeout is not
unref()'d. Other long-lived timers in this repo (e.g., channel-idle) callunref()to avoid keeping the Node process alive unintentionally. Consider unref'ing this timeout as well for consistency and to prevent shutdown hangs in non-process.exit()scenarios (tests/CLI usage).
const timeout = setTimeout(() => {
log.warn(`Quiet mode timeout (60s) for channel ${channelId.slice(0, 8)}... — force-clearing`);
state.delete(channelId);
}, TIMEOUT_MS);
state.set(channelId, { hasContent: false, timeout });
src/index.ts:2238
- Same as scheduler:
enterQuietMode()returns a cleanup function that is only called on error. If no events are processed (orhandleSessionEventreturns early), the channel can remain quiet for up to 60s and interfere with subsequent messages. Consider invoking the cleanup in afinallyoncesendMessage()completes (or after an idle wait, if you add one).
const clearQuiet = enterQuietMode(channelId);
try {
await sessionManager.sendMessage(channelId, NUDGE_PROMPT);
} catch (err) {
clearQuiet();
throw err;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove dead QuietState fields (threadRoot, threadRoothasContent)
was never set (flush uses channelThreadRoots), hasContent was mutated
but never read for decisions
- Add finally{} cleanup in scheduler wrapper and nudge function so
quiet mode is always cleared on success path, not just on error
- Fix misleading comment: errors exit quiet and fall through (not suppressed)
- Remove unused getQuietState import from index.ts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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
Replaces the simple
nudgePendingSet with a unified quiet mode system that fully suppresses all streaming output until we determine whether the response isNO_REPLY.What it does
NO_REPLYresponsesentirelyinitialContentwhen the response is notNO_REPLYKey changes
src/core/quiet-mode.ts: NewQuietStateinterface,enterQuietMode(),exitQuietMode(),getQuietState(),isQuiet()modulesrc/core/quiet-mode.test.ts: 13 tests covering lifecycle, timeout, cleanup, state trackingsrc/index.ts: Wires quiet mode into event handler, scheduler wrapper, and nudge function; removes inlinenudgePendingcodeDesign decisions
hasContentboolean. Useassistant.messagecontent (authoritative) for flush.deltasassistant. SDK fires these as tool-call signals before tool execution.messagesession.idlepass somarkIdle()fires andwaitForChannelIdle()resolves normally.throughCloses #119